-
Notifications
You must be signed in to change notification settings - Fork 2
Desktop: Resolves #41: Multi-window support in the desktop app #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Desktop: Resolves #41: Multi-window support in the desktop app #42
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm highlighting a few of the major API changes.
await joplin.data.put(['notes', messageNote.id], null, { body: messageNote.body }); | ||
await joplin.views.editors.saveNote(editorHandle, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change: data.put
is replaced with editors.saveNote
to allow Joplin to determine which editor made the change. This prevents changes from the editor from emitting update
events.
src/index.ts
Outdated
joplin.plugins.register({ | ||
onStart: async function() { | ||
const versionInfo = await joplin.versionInfo(); | ||
const registerEditorPlugin = async (windowId: string|undefined) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change: One editor plugin is now registered for each window.
It's not ideal to have to pass around this windowId variable. Isn't it possible to manage this application side? i.e. every time the plugin calls some function, we complete it with whatever the window ID was when the panel was created? |
At present,
For item 1 above, it may be possible to default to the focused window ID (the window ID when the panel was created). However, this could lead to race conditions. If for example,
Then, with auto-setting the window ID, both editors are registered for the window created in step 3. With the current editorview-per-window design, this results in a window that lacks an editor plugin view. As a result, the editor plugin can't be shown for the window in step 1. |
src/index.ts
Outdated
logger.info('onUpdate'); | ||
await updateFromSelectedNote(); | ||
await registerEditorPlugin(undefined); | ||
joplin.workspace.onWindowOpen(async ({ windowId }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API redesign suggestion (v1)
Possible API redesign:
- Deprecate
joplin.editors.create
. - New function: Add a new
joplin.editors.register
.joplin.editors.register
accepts an argument of typeEditorSpec
:Above,type EditorSpec = { onCreate: (handle: ViewHandle)=>Promise<void>; onActivationCheck: (handle: ViewHandle, event: ActivationCheckEvent)=>Promise<boolean>; onUpdate: (handle: ViewHandle, event: UpdateEvent)=>Promise<void>; };
onCreate
is called when Joplin creates a new instance of the editor plugin. This will be done when the user creates a new window. Joplin automatically destroys the editor when the user closes the window. - New function:
joplin.editors.save(handle: ViewHandle, note: Note)
to save the content of the editor, without triggeringonUpdate
.- On desktop,
.editors.save
connects to the existinguseFormNote.ts
logic for better integration with external editors and secondary windows.
- On desktop,
Example:
joplin.editors.register({
onCreate: async (handle: ViewHandle) => { // Handles creating the editor plugin per editor.
// Setup
// Use e.g. joplin.data.get(['notes', cachedNoteId]); to get the note
// Use editors.save(handle, ...) to save it. (Allows proper sync with external editors, etc.)
},
onActivationCheck: async (handle, event) => {
return true; // Always allow users to enable
},
onUpdate: async (handle, event) => {
joplin.editors.postMessage(handle, { type: 'update', content: event.newBody });
},
});
API redesign suggestion (v2) (not feasible due to desktop plugin IPC restrictions)
Possible API redesign:
- Deprecate
joplin.editors.create
. - New function: Add a new
joplin.editors.register
.joplin.editors.register
accepts an argument of typeOnCreateEditor
:Above,interface EditorCallbacks { onActivationCheck: (event: ActivationCheckEvent)=>Promise<boolean>; onUpdate: (event: UpdateEvent)=>Promise<void>; }; type OnCreateEditor = (handle: ViewHandle)=>Promise<EditorCallbacks>; // Type of joplin.editors.register joplin.editors.register(viewId: string, callback: OnCreateEditor): Promise<void>;
callback
is called when Joplin creates a new instance of the editor plugin. This will be done, for example, when the user creates a new window. - New function:
joplin.editors.save(handle: ViewHandle, note: NoteProperties)
to save the content of the editor, without triggeringonUpdate
.- On desktop,
.editors.save
connects to the existinguseFormNote.ts
logic for better integration with external editors and secondary windows.
- On desktop,
Example:
joplin.editors.register('test-plugin', async (handle) => {
await joplin.editors.setHtml(handle, `...`);
await joplin.editors.addScript(handle, './path/to/script.js');
await joplin.editors.onMessage(handle, message => {
if (message.kind === 'save') {
const editingId = message.id;
const bodyToSave = message.body;
joplin.editors.save(handle, { id: editingId, body: bodyToSave });
}
});
return {
onActivationCheck: async (event) => {
return true; // Always allow users to enable
},
onUpdate: async (event) => {
joplin.editors.postMessage(handle, { type: 'update', content: event.newBody });
},
};
});
API redesign suggestion (v3)
Possible API redesign:
- Deprecate
joplin.editors.create
. - New function: Add a new
joplin.editors.register
:Above,interface EditorOptions { onActivationCheck: (event: ActivationCheckEvent)=>Promise<boolean>; onSetup: (handle: ViewHandle)=>Promise<void>; }; // Type of joplin.editors.register joplin.editors.register(id: string, callback: EditorOptions): Promise<void>;
onSetup
is called when Joplin creates a new instance of the editor plugin. This will be done, for example, when the user creates a new window.
Note thatonUpdate
is not included inEditorOptions
. This is because it's often convenient to store information fromonUpdate
in per-editor local variables inonSetup
. Forcing users to provideonUpdate
inEditorOptions
makes it more difficult to share this state with other parts of the editor. - New function:
joplin.editors.saveNote(handle: ViewHandle, note: NoteProperties)
to save the content of the editor, without triggeringonUpdate
.- On desktop,
.editors.save
connects to the existinguseFormNote.ts
logic for better integration with external editors and secondary windows.
- On desktop,
Example:
joplin.editors.register('test-plugin', {
onSetup: async (handle) => {
await joplin.editors.setHtml(handle, `...`);
await joplin.editors.addScript(handle, './path/to/script.js');
let noteId;
await joplin.editors.onUpdate(handle, event => {
noteId = event.noteId;
joplin.editors.postMessage(handle, { type: 'update', content: event.newBody });
});
await joplin.editors.onMessage(handle, message => {
if (message.kind === 'save') {
const editingId = message.id;
const bodyToSave = message.body;
joplin.editors.save(handle, { id: editingId, body: bodyToSave });
}
});
},
onActivationCheck: async (event) => {
return true; // Always allow users to enable
},
});
To-do:
- Clean this up, convert to a specification.
Summary
This pull request updates the YesYouKan plugin to support secondary windows in Joplin desktop.
Resolves #41.
To-do
Notes