-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Desktop: Multiple window support #11181
base: dev
Are you sure you want to change the base?
Desktop: Multiple window support #11181
Conversation
Related to laurent22#10459, but needs more testing -- this change could cause regressions.
const parentNode = loaded ? doc?.body : null; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Needed to allow adding the portal to the DOM | ||
const contentPortal = parentNode && createPortal(props.children, parentNode) as any; |
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.
Related documentation/explanation:
- React Portals documentation.
- StackOverflow thread with discussion on how to render React components to a new window.
- A blog post about using
createPortal
with new windows.
…me' into test/editor-in-iframe
…en and into separate directory This pull request previously moved the main window command registration logic from MainScreen.tsx to a different component that could manage showing dialogs, etc for different windows. This, however, could cause errors when closing settings (attempts to use commands with unregistered runtimes). This pull request ensures that command runtimes are registered by loading them in Root.tsx, rather than in MainScreen.tsx.
styled-components only works in the main window. See the related styled-components/styled-components#2620.
return <> | ||
<div ref={setDependencyStyleContainer}></div> | ||
<StyleSheetManager target={dependencyStyleContainer}> | ||
<CacheProvider value={cache}> |
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.
This code uses CacheProvider
from @emotion/react
to cause react-select
to insert its styles into the new window (see comment). @emotion/react
is a dependency of react-select
and, at present, is not a direct dependency of Joplin.
Adding @emotion/react
to app-desktop/package.json
could be problematic if react-select
is updated without also updating the @emotion/react
version — having multiple versions of @emotion/react
may cause styling issues (or perhaps break styling just in new windows).
See this unmerged upstream pull request for a possible future solution that doesn't involve import
ing transitive dependencies.
This commit converts the toggleEditors command to an editor command.
…me' into test/editor-in-iframe
to be restored By registering the toggleEditors runtime sooner, the `runtimeMustBeRegistered: true` assertion can be restored.
container window and fix tag display
This commit attempts to address what may be a race condition -- if commands aren't loaded by the time certain parts of the UI attempt to render, a "crashed" screen can be shown. This may be what is happening in CI.
background windows Previously, these buttons were enabled/disabled based on the state of whichever window had focus.
Previously, only the focused window's note history was updated.
Creates titles for background windows based on the current note ID.
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.
NoteEditor.tsx
is a new file. The old NoteEditor.tsx
was renamed to NoteEditorContent.tsx
.
- To-do: Use a different name for the note editor wrapper so that it's marked as a rename by
git
?
Summary
This pull request allows opening Joplin's editor in a new window using React's portal API. It still has a few issues and is thus a draft.
Known issues
selectedNoteId
always act on the note displayed in the main window.selectedNoteId
for their enabled condition can be incorrectly enabled/disabled in new windows.state.notes
is refreshed unnecessarily when a window gains focus and it isn't a just-created window.all-windows-same-layout.webm
Screen recording
multi-window-demo-oct-15-2024.webm
Notes
document
andwindow
variables may no longer point to the current component's DOM.useDom
hook has been added to get theDocument
for a component.react-select
to be styled in new windows, it seems necessary to wrap the window's content in theCacheProvider
component from one of its dependencies.