-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Summary
This pull request allows opening Joplin's editor in a new window using React's portal API. While working, it currently has several major issues. As such, it's currently 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.useFormNote
was adjusted to refresh notes when changed in the database. Does this cause the note to reload unnecessarily when writing its content to the database?