Skip to content

Mobile: Fix issues with modals created via plugins - #16674

Merged
laurent22 merged 20 commits into
laurent22:devfrom
mrjo118:fix-plugin-modals-behind-plugin-panel2
Oct 11, 2026
Merged

laurent22 merged 20 commits into
laurent22:devfrom
mrjo118:fix-plugin-modals-behind-plugin-panel2

Conversation

@mrjo118

@mrjo118 mrjo118 commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

There are several issues with plugin-launched modals on mobile, resulting in a poor experience for plugins that use them. The most significant is that, on both web and native platforms, a modal launched from the plugin panel (accessible through the jigsaw-piece icon) opens behind the panel. Users must close the plugin panel to see the modal and may assume that their action did nothing.

This PR adds a useNativeModal prop to Modal.tsx. When set to false, the modal content is rendered without wrapping it in a native ModalComponent. The plugin panel disables the native modal and instead renders within a Portal, placing it in the same modal hierarchy as plugin-launched dialogs. This allows those dialogs to appear above the panel, receive input and accessibility focus, and block interaction with the panel until closed.

Other issues addressed:

  • On the web app, modals which have long height and are scrollable will gradually shrink as soon as they are rendered
  • On the native app, modals which have long height and are scrollable may not be able to be accessible at the top of the content when the on screen keyboard is open. Additionally addressed when the keyboard is not open, the modal may overlap the safe area insets, and when in landscape mode with 3 button navigation of a mobile sized screen, prevent the modal expanding beyond the insets there
  • The previous 0.95 max height cap provided additional room for safe-area insets. Since the applicable insets are now included explicitly in the available-height calculation, that additional allowance is no longer needed, so the height cap was changed to match the 0.97 width cap.

Testing

I tested these changes using the Agenda plugin, where these issues are clearly visible.

Testing on Android:

Before change:

Screen_Recording_20260927_003205_Joplin.mp4

After change:

Screen_Recording_20260927_003248_Joplin2.mp4

Testing on web:

Before change:

wYjx9SobYs.before.mp4

After change:

v5ys6P51Ar.after.mp4

@coderabbitai coderabbitai Bot added accessibility Related to accessibility bug It's a bug mobile All mobile platforms labels Sep 26, 2026

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Comment thread packages/app-mobile/components/DismissibleDialog.tsx
Comment thread packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts Outdated
@joplin-coderabbit-cleanup joplin-coderabbit-cleanup Bot deleted a comment from coderabbitai Bot Sep 27, 2026
@laurent22

Copy link
Copy Markdown
Owner

Please see below:

  1. Restore Android Back handling for plugin panels — required. PluginPanelViewer.tsx:170. With useNativeModal={false} the native onRequestClose is gone and root.tsx:601 has no showPanelsDialog case, so Back navigates away or exits the app with the panel still overlaid. Either add a showPanelsDialog branch to backButtonHandler() dispatching dismissPluginPanels, or register a BackHandler listener in the component while visible.
  2. Don't measure dialog width from the viewport — required. initializeDialogWebView.ts:83. getContentSize should measure #joplin-plugin-content; window.innerWidth makes fit-to-content dialogs full-viewport-wide while height still measures the element. Fix the scrollbar feedback loop at the source instead — e.g. scrollbar-gutter: stable or overflow: hidden on the measured element, or Math.max against the previous width.
  3. Remove the double safe-area subtraction — required. PluginDialogWebView.tsx:48. Modal's modalBackground already applies safeAreaPadding (Modal.tsx:72), so subtracting insets again costs ~90px of height on notched devices.
  4. Use consistent coordinate spaces for the keyboard calc — required. PluginDialogWebView.tsx:44. Dimensions.get('screen').height includes the Android nav bar; windowSize.height does not. Mixing them under-deducts the keyboard by the nav-bar height. Derive keyboardTop from window height, or compare both in screen space.
  5. Explain or revert the 0.95 → 0.97 max-height change — minor. Unrelated to the stated fixes and undocumented in the diff.
  6. Hide the panel backdrop when inert — minor/optional. DismissibleDialog.tsx:129. Consider making modalBackground transparent (or not rendering the panel at all) when inert, so there aren't two stacked dim layers.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Comment thread packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts Outdated

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Comment thread packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts Outdated
@mrjo118

mrjo118 commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai resume

@mrjo118

mrjo118 commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Points 1, 2, 4, and 6 have been addressed. The results from the changes look exactly the same when tested on Android and web. I also verified the back handler works correctly on Android.

For point 4, the screen-relative keyboard boundary is now converted to window coordinates using the bottom system-UI offset. The offset is capped at the safe-area inset to avoid deducting an IME-driven window resize twice.

Regarding the remaining points:

  • 3: Remove the double safe-area subtraction:
    I did not remove the explicit safe-area adjustment because PluginDialogWebView is wrapped by React Native Paper’s Modal, not the modal in Modal.tsx. Paper’s modal adds margins around its wrapper, but those margins do not change the windowSize used to calculate the dialog’s numeric maxHeight. Without the explicit adjustment, device testing showed the dialog extending into the status-bar area.
  • 5: Explain or revert the 0.95 → 0.97 max-height change:
    The previous 0.95 height cap provided additional room for safe-area insets. Since the applicable insets are now included explicitly in the available-height calculation, that additional allowance is no longer needed, so the height cap can match the 0.97 width cap.

In addition I also fixed plugin modals not catering with horizontal insets:

Before:

Screenshot_20261004_001723_Joplin2

After:

Screenshot_20261004_003317_Joplin2

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Comment thread packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts Outdated
@joplin-coderabbit-cleanup joplin-coderabbit-cleanup Bot deleted a comment from coderabbitai Bot Oct 4, 2026
@joplin-coderabbit-cleanup joplin-coderabbit-cleanup Bot deleted a comment from coderabbitai Bot Oct 4, 2026

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Comment thread packages/app-mobile/components/Modal.tsx
@joplin-coderabbit-cleanup joplin-coderabbit-cleanup Bot deleted a comment from coderabbitai Bot Oct 4, 2026
@laurent22

Copy link
Copy Markdown
Owner

Should be fixed

  1. Width ratchet can stick permanently too wide — initializeDialogWebView.ts:21-28. maximumContentWidth only resets on a DOM mutation, stylesheet load, or viewport change. A width reduction from a non-mutation cause (web font loading, image intrinsic size resolving, CSS transition settling) leaves the dialog locked at the wider measurement for its lifetime. A ResizeObserver on the measured element expresses the actual intent and removes the need for the high-water-mark latch entirely.
  2. The fix is defeated by dialogs with live content — same file. subtree: true + characterData: true on document.body fires on any text change anywhere, so a dialog with a clock, countdown, progress indicator, or framework render loop sets resetContentWidthBaseline = true before nearly every 500ms poll, collapsing the latch and restoring the original shrinking behaviour. Worth testing against a continuously-updating dialog.
  3. Observer is never disconnected — created unconditionally, observing body and head subtrees for the webview's lifetime, including for fitToContent: false dialogs where getContentSize is never polled.

Should be explained, not necessarily changed

  1. maxHeight margin changed from 5% to 3% — windowSize.height * 0.95 → availableHeight * 0.97 in PluginDialogWebView.tsx. A deliberate-looking behaviour change on a different base, not mentioned in the PR description.
  2. New back-button branch relies on third-party listener ordering — root.tsx:601-605 fires whenever showPanelsDialog is true, including while a plugin dialog is open above the panel. It works only because Paper's Modal registers its own hardwareBackPress listener later and RN dispatches most-recent-first. A guard on open-dialog count, or a comment recording why it's safe, would make this less fragile.

@mrjo118

mrjo118 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Done

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

Comment thread packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts Outdated
@joplin-coderabbit-cleanup joplin-coderabbit-cleanup Bot deleted a comment from coderabbitai Bot Oct 7, 2026
@joplin-coderabbit-cleanup joplin-coderabbit-cleanup Bot deleted a comment from coderabbitai Bot Oct 7, 2026
@laurent22
laurent22 merged commit c87f694 into laurent22:dev Oct 11, 2026
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility Related to accessibility bug It's a bug mobile All mobile platforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants