From c87f694dc7895ed003766fdf0b2a8155d3f41019 Mon Sep 17 00:00:00 2001 From: mrjo118 Date: Sun, 11 Oct 2026 16:18:55 +0100 Subject: [PATCH] Mobile: Fix issues with modals created via plugins (#16674) Co-authored-by: Laurent Cozic --- .gitignore | 1 + .ignore.eslint | 1 + .../components/DismissibleDialog.tsx | 15 ++- .../components/ExtendedWebView/index.jest.tsx | 9 ++ packages/app-mobile/components/Modal.tsx | 28 +++-- .../initializeDialogWebView.test.ts | 105 ++++++++++++++++++ .../backgroundPage/initializeDialogWebView.ts | 48 +++++++- .../plugins/dialogs/PluginDialogManager.tsx | 2 +- .../plugins/dialogs/PluginDialogWebView.tsx | 32 +++++- .../plugins/dialogs/PluginPanelViewer.tsx | 3 + .../plugins/dialogs/hooks/useDialogSize.ts | 4 +- .../app-mobile/components/plugins/types.ts | 2 +- packages/app-mobile/root.tsx | 10 ++ 13 files changed, 235 insertions(+), 25 deletions(-) create mode 100644 packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.test.ts diff --git a/.gitignore b/.gitignore index 33c351d756..30ff3761b3 100644 --- a/.gitignore +++ b/.gitignore @@ -864,6 +864,7 @@ packages/app-mobile/components/plugins/PluginNotification.js packages/app-mobile/components/plugins/PluginRunner.js packages/app-mobile/components/plugins/PluginRunnerWebView.test.js packages/app-mobile/components/plugins/PluginRunnerWebView.js +packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.test.js packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.js packages/app-mobile/components/plugins/backgroundPage/initializePluginBackgroundIframe.js packages/app-mobile/components/plugins/backgroundPage/pluginRunnerBackgroundPage.js diff --git a/.ignore.eslint b/.ignore.eslint index b8d1fc2707..b0123dbbb3 100644 --- a/.ignore.eslint +++ b/.ignore.eslint @@ -890,6 +890,7 @@ packages/app-mobile/components/plugins/PluginNotification.js packages/app-mobile/components/plugins/PluginRunner.js packages/app-mobile/components/plugins/PluginRunnerWebView.test.js packages/app-mobile/components/plugins/PluginRunnerWebView.js +packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.test.js packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.js packages/app-mobile/components/plugins/backgroundPage/initializePluginBackgroundIframe.js packages/app-mobile/components/plugins/backgroundPage/pluginRunnerBackgroundPage.js diff --git a/packages/app-mobile/components/DismissibleDialog.tsx b/packages/app-mobile/components/DismissibleDialog.tsx index 9e5abd29f5..f0843738c1 100644 --- a/packages/app-mobile/components/DismissibleDialog.tsx +++ b/packages/app-mobile/components/DismissibleDialog.tsx @@ -24,6 +24,8 @@ interface Props { children: React.ReactNode; heading?: string; scrollOverflow?: boolean; + useNativeModal?: boolean; + inert?: boolean; size: DialogVariant; } @@ -124,15 +126,22 @@ const DismissibleDialog: React.FC = props => { return ( - + {closeButtonRow} {props.children} diff --git a/packages/app-mobile/components/ExtendedWebView/index.jest.tsx b/packages/app-mobile/components/ExtendedWebView/index.jest.tsx index 40924dff64..7ff5b98226 100644 --- a/packages/app-mobile/components/ExtendedWebView/index.jest.tsx +++ b/packages/app-mobile/components/ExtendedWebView/index.jest.tsx @@ -57,6 +57,15 @@ const ExtendedWebView = (props: Props, ref: Ref) => { useEffect(() => { // JSDOM polyfills dom.window.eval(polyfillScrollFunctions); + dom.window.eval(` + // JSDOM does not implement ResizeObserver. Observer behaviour is covered by + // the initializeDialogWebView unit tests. + window.ResizeObserver ??= class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} + }; + `); dom.window.eval(` // JSDOM iframes are missing certain functionality required by Joplin, diff --git a/packages/app-mobile/components/Modal.tsx b/packages/app-mobile/components/Modal.tsx index d6a3013761..accb6aeae4 100644 --- a/packages/app-mobile/components/Modal.tsx +++ b/packages/app-mobile/components/Modal.tsx @@ -29,6 +29,9 @@ export interface ModalElementProps { // iOS only: Called once the modal has finished its dismissal transition onDismissed?: ()=> void; animationType?: 'fade'|'none'; + // When false, render within the surrounding portal rather than creating a + // separate native/top-layer modal. + useNativeModal?: boolean; wrapContent?: (view: React.ReactNode)=> React.ReactNode; statusBarTranslucent?: boolean; @@ -162,6 +165,7 @@ const ModalElement: React.FC = ({ onClose, onDismissed, wrapContent, + useNativeModal = true, ...forwardedProps }) => { const styles = useStyles(!!scrollOverflow, backgroundColor); @@ -209,9 +213,19 @@ const ModalElement: React.FC = ({ } const extraScrollViewProps = (typeof scrollOverflow === 'object' ? scrollOverflow : {}); + const modalContent = scrollOverflow ? ( + + {contentAndBackdrop} + + ) : contentAndBackdrop; + const result = ( - = ({ onRequestClose={onClose} {...forwardedProps} > - {scrollOverflow ? ( - - {contentAndBackdrop} - - ) : contentAndBackdrop} - + {modalContent} + : (forwardedProps.visible ? modalContent : null)} ); diff --git a/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.test.ts b/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.test.ts new file mode 100644 index 0000000000..85458bab4e --- /dev/null +++ b/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.test.ts @@ -0,0 +1,105 @@ +/** @jest-environment jsdom */ +import { DialogWebViewApi } from '../types'; +import initializeDialogWebView from './initializeDialogWebView'; + +let mockDialogApi: DialogWebViewApi; +jest.mock('../../../utils/ipc/WebViewToRNMessenger', () => { + return class { + public remoteApi = { + postMessage: jest.fn(), + onMessage: jest.fn(), + onError: jest.fn(), + onLog: jest.fn(), + }; + + public constructor(_messageChannelId: string, localApi: DialogWebViewApi) { + mockDialogApi = localApi; + } + }; +}); +jest.mock('./utils/reportUnhandledErrors', () => jest.fn()); +jest.mock('./utils/wrapConsoleLog', () => jest.fn()); + +class MockResizeObserver { + public static instances: MockResizeObserver[] = []; + public observe = jest.fn(); + public disconnect = jest.fn(); + + public constructor(private callback: ResizeObserverCallback) { + MockResizeObserver.instances.push(this); + } + + public notify() { + this.callback([], this as unknown as ResizeObserver); + } +} + +describe('initializeDialogWebView', () => { + let contentWidth = 0; + let contentHeight = 0; + let viewportWidth = 0; + let innerWidth = 0; + + beforeEach(() => { + MockResizeObserver.instances = []; + global.ResizeObserver = MockResizeObserver as unknown as typeof ResizeObserver; + document.body.innerHTML = '
'; + + const content = document.getElementById('joplin-plugin-content'); + Object.defineProperty(content, 'clientWidth', { configurable: true, get: () => contentWidth }); + Object.defineProperty(content, 'clientHeight', { configurable: true, get: () => contentHeight }); + Object.defineProperty(document.documentElement, 'clientWidth', { configurable: true, get: () => viewportWidth }); + Object.defineProperty(window, 'innerWidth', { configurable: true, get: () => innerWidth }); + + contentWidth = 320; + contentHeight = 200; + viewportWidth = 1000; + innerWidth = 1000; + initializeDialogWebView('test-channel'); + }); + + afterEach(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + test('should report width reductions observed without DOM mutations', async () => { + expect(await mockDialogApi.getContentSize(true)).toMatchObject({ width: 320, height: 200 }); + + contentWidth = 220; + MockResizeObserver.instances[0].notify(); + + expect(await mockDialogApi.getContentSize(true)).toMatchObject({ width: 220, height: 200 }); + }); + + test('should remain stable while dialog text changes continuously', async () => { + contentWidth = 300; + viewportWidth = 300; + innerWidth = 315; + expect(await mockDialogApi.getContentSize(true)).toMatchObject({ width: 315, height: 200 }); + + for (let i = 0; i < 5; i++) { + document.getElementById('joplin-plugin-content').textContent = `Update ${i}`; + contentHeight++; + MockResizeObserver.instances[0].notify(); + expect(await mockDialogApi.getContentSize(true)).toMatchObject({ width: 315, height: 201 + i }); + } + }); + + test('should only observe while watching for size changes', async () => { + await mockDialogApi.getContentSize(false); + expect(MockResizeObserver.instances).toHaveLength(0); + + await mockDialogApi.getContentSize(true); + expect(MockResizeObserver.instances[0].observe).toHaveBeenCalledWith( + document.getElementById('joplin-plugin-content'), + { box: 'border-box' }, + ); + + await mockDialogApi.getContentSize(false); + expect(MockResizeObserver.instances[0].disconnect).toHaveBeenCalledTimes(1); + + await mockDialogApi.getContentSize(true); + window.dispatchEvent(new Event('pagehide')); + expect(MockResizeObserver.instances[1].disconnect).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts b/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts index 59fc18dce8..501dc75b55 100644 --- a/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts +++ b/packages/app-mobile/components/plugins/backgroundPage/initializeDialogWebView.ts @@ -18,6 +18,42 @@ let themeCssElement: HTMLStyleElement|null = null; const initializeDialogWebView = (messageChannelId: string) => { const loadedPaths: Set = new Set(); + let contentResizeObserver: ResizeObserver|null = null; + let observedElement: HTMLElement|null = null; + let observedContentSize: { width: number; height: number }|null = null; + let pageHideListenerRegistered = false; + + const measureContent = (element: HTMLElement) => { + const viewportWidth = document.documentElement.clientWidth; + const scrollbarWidth = Math.max(0, window.innerWidth - viewportWidth); + // Include the scrollbar gutter when the content fills the viewport so feeding this + // measurement back into the iframe does not repeatedly shrink it. + const width = element.clientWidth === viewportWidth ? element.clientWidth + scrollbarWidth : element.clientWidth; + return { width, height: element.clientHeight }; + }; + const stopObservingContentSize = () => { + contentResizeObserver?.disconnect(); + contentResizeObserver = null; + observedElement = null; + observedContentSize = null; + if (pageHideListenerRegistered) { + window.removeEventListener('pagehide', stopObservingContentSize); + pageHideListenerRegistered = false; + } + }; + const observeContentSize = (element: HTMLElement) => { + if (observedElement === element && contentResizeObserver) return; + + stopObservingContentSize(); + observedElement = element; + observedContentSize = measureContent(element); + contentResizeObserver = new ResizeObserver(() => { + if (observedElement === element) observedContentSize = measureContent(element); + }); + contentResizeObserver.observe(element, { box: 'border-box' }); + window.addEventListener('pagehide', stopObservingContentSize); + pageHideListenerRegistered = true; + }; type ScriptType = 'js'|'css'; const includeScriptsOrStyles = (type: ScriptType, paths: string[]) => { @@ -73,15 +109,21 @@ const initializeDialogWebView = (messageChannelId: string) => { document.body.appendChild(styleElement); themeCssElement = styleElement; }, - getContentSize: async () => { + getContentSize: async (watchForSizeChanges = false) => { // To convert to React Native pixel units from browser pixel units, // we need to multiply by the devicePixelRatio: const dpr = window.devicePixelRatio ?? 1; const element = document.getElementById('joplin-plugin-content') ?? document.body; + if (watchForSizeChanges) { + observeContentSize(element); + } else { + stopObservingContentSize(); + } + const contentSize = observedContentSize ?? measureContent(element); return { - width: element.clientWidth * dpr, - height: element.clientHeight * dpr, + width: contentSize.width * dpr, + height: contentSize.height * dpr, }; }, }; diff --git a/packages/app-mobile/components/plugins/dialogs/PluginDialogManager.tsx b/packages/app-mobile/components/plugins/dialogs/PluginDialogManager.tsx index e5233d8f72..bf9ece12b0 100644 --- a/packages/app-mobile/components/plugins/dialogs/PluginDialogManager.tsx +++ b/packages/app-mobile/components/plugins/dialogs/PluginDialogManager.tsx @@ -57,8 +57,8 @@ const PluginDialogManager: React.FC = props => { return ( <> + 0}/> {dialogs} - ); }; diff --git a/packages/app-mobile/components/plugins/dialogs/PluginDialogWebView.tsx b/packages/app-mobile/components/plugins/dialogs/PluginDialogWebView.tsx index 5a985be686..3f23e941bd 100644 --- a/packages/app-mobile/components/plugins/dialogs/PluginDialogWebView.tsx +++ b/packages/app-mobile/components/plugins/dialogs/PluginDialogWebView.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { useCallback, useMemo, useState } from 'react'; import { PluginHtmlContents, PluginStates, ViewInfo } from '@joplin/lib/services/plugins/reducer'; -import { StyleSheet, View, useWindowDimensions } from 'react-native'; +import { Dimensions, StyleSheet, View, useWindowDimensions } from 'react-native'; import usePlugin from '@joplin/lib/hooks/plugins/usePlugin'; import { DialogContentSize, DialogWebViewApi } from '../types'; import { Button } from 'react-native-paper'; @@ -12,6 +12,8 @@ import WebviewController, { ContainerType } from '@joplin/lib/services/plugins/W import { Theme } from '@joplin/lib/themes/type'; import useDialogSize from './hooks/useDialogSize'; import PluginUserWebView from './PluginUserWebView'; +import useKeyboardState from '../../../utils/hooks/useKeyboardState'; +import useSafeAreaPadding from '../../../utils/hooks/useSafeAreaPadding'; interface Props { themeId: number; @@ -26,6 +28,8 @@ const useStyles = ( fitToContent: boolean, ) => { const windowSize = useWindowDimensions(); + const keyboardState = useKeyboardState(); + const safeAreaPadding = useSafeAreaPadding(); return useMemo(() => { const theme: Theme = themeStyle(themeId); @@ -33,8 +37,28 @@ const useStyles = ( const useDialogSize = fitToContent && dialogContentSize; const dialogHasLoaded = !!dialogContentSize; - const maxWidth = windowSize.width * 0.97; - const maxHeight = windowSize.height * 0.95; + const availableWidth = Math.max( + 0, + windowSize.width - safeAreaPadding.paddingLeft - safeAreaPadding.paddingRight, + ); + const maxWidth = availableWidth * 0.97; + // Opening the keyboard may resize the window fully, partially, or not at all. + // Use the smaller height so the dialog stays above any remaining keyboard + // overlap without deducting the keyboard twice. + const screenHeight = Dimensions.get('screen').height; + // Convert the screen-relative keyboard position to window coordinates. Cap the + // offset because the screen/window height difference may also include an IME resize. + const screenToWindowOffset = Math.min( + safeAreaPadding.paddingBottom, + Math.max(0, screenHeight - windowSize.height), + ); + const keyboardTop = screenHeight - screenToWindowOffset - keyboardState.dockedKeyboardHeight; + const bottomPadding = safeAreaPadding.paddingBottom; + const availableHeight = Math.max( + 0, + Math.min(windowSize.height, keyboardTop) - safeAreaPadding.paddingTop - bottomPadding, + ); + const maxHeight = availableHeight * 0.97; const dialogWidth = useDialogSize ? dialogContentSize.width : maxWidth; const dialogHeight = useDialogSize ? dialogContentSize.height : maxHeight; @@ -71,7 +95,7 @@ const useStyles = ( justifyContent: 'flex-end', }, }); - }, [themeId, dialogContentSize, fitToContent, windowSize.width, windowSize.height]); + }, [themeId, dialogContentSize, fitToContent, windowSize.width, windowSize.height, keyboardState.dockedKeyboardHeight, safeAreaPadding]); }; const defaultButtonSpecs: ButtonSpec[] = [ diff --git a/packages/app-mobile/components/plugins/dialogs/PluginPanelViewer.tsx b/packages/app-mobile/components/plugins/dialogs/PluginPanelViewer.tsx index a6504c45c0..76c7a436c2 100644 --- a/packages/app-mobile/components/plugins/dialogs/PluginPanelViewer.tsx +++ b/packages/app-mobile/components/plugins/dialogs/PluginPanelViewer.tsx @@ -21,6 +21,7 @@ interface Props { pluginHtmlContents: PluginHtmlContents; pluginStates: PluginStates; visible: boolean; + inert?: boolean; } @@ -166,6 +167,8 @@ const PluginPanelViewer: React.FC = props => { visible={props.visible} size={DialogVariant.Large} onDismiss={onClose} + useNativeModal={false} + inert={props.inert} > {renderTabContent()} {renderTabSelector()} diff --git a/packages/app-mobile/components/plugins/dialogs/hooks/useDialogSize.ts b/packages/app-mobile/components/plugins/dialogs/hooks/useDialogSize.ts index 423ffa23e9..7b27df900c 100644 --- a/packages/app-mobile/components/plugins/dialogs/hooks/useDialogSize.ts +++ b/packages/app-mobile/components/plugins/dialogs/hooks/useDialogSize.ts @@ -22,7 +22,7 @@ const useDialogSize = (props: Props) => { } while (!event.cancelled) { - const contentSize = await dialogControl.getContentSize(); + const contentSize = await dialogControl.getContentSize(props.watchForSizeChanges); if (event.cancelled) return; const lastSize = lastSizeRef.current; @@ -44,7 +44,7 @@ const useDialogSize = (props: Props) => { }, 500); }); } - }, [dialogControl, setDialogSize, webViewLoadCount]); + }, [dialogControl, setDialogSize, webViewLoadCount, props.watchForSizeChanges]); return dialogSize; }; diff --git a/packages/app-mobile/components/plugins/types.ts b/packages/app-mobile/components/plugins/types.ts index 10eba3bee2..e19cab89c7 100644 --- a/packages/app-mobile/components/plugins/types.ts +++ b/packages/app-mobile/components/plugins/types.ts @@ -61,5 +61,5 @@ export interface DialogWebViewApi { setThemeCss: (css: string)=> Promise; getFormData: ()=> Promise; - getContentSize: ()=> Promise; + getContentSize: (watchForSizeChanges?: boolean)=> Promise; } diff --git a/packages/app-mobile/root.tsx b/packages/app-mobile/root.tsx index 47825e7404..017f2f5f70 100644 --- a/packages/app-mobile/root.tsx +++ b/packages/app-mobile/root.tsx @@ -99,6 +99,7 @@ import { setDispatch } from './services/profiles'; import { ReactNode } from 'react'; import autodetectTheme, { onSystemColorSchemeChange } from './utils/autodetectTheme'; import PluginRunnerWebView from './components/plugins/PluginRunnerWebView'; +import CommandService from '@joplin/lib/services/CommandService'; import { refreshFolders, scheduleRefreshFolders } from '@joplin/lib/folders-screen-utils'; import ShareManager from './components/screens/ShareManager'; import { setDateFormat, setTimeFormat, setTimeLocale } from '@joplin/utils/time'; @@ -310,6 +311,7 @@ interface AppComponentProps { disableSideMenuGestures: boolean; historyCanGoBack: boolean; showSideMenu: boolean; + showPanelsDialog: boolean; noteSelectionEnabled: boolean; syncStarted: boolean; } @@ -599,6 +601,13 @@ class AppComponent extends React.Component } private async backButtonHandler() { + // Plugin dialogs use Paper's Modal, whose later hardwareBackPress listener runs first + // (React Native dispatches newest-first), so an open dialog consumes Back before this branch. + if (this.props.showPanelsDialog) { + await CommandService.instance().execute('dismissPluginPanels'); + return true; + } + if (this.props.noteSelectionEnabled) { this.props.dispatch({ type: 'NOTE_SELECTION_END' }); return true; @@ -890,6 +899,7 @@ const mapStateToProps = (state: AppState) => { return { historyCanGoBack: state.historyCanGoBack, showSideMenu: state.showSideMenu, + showPanelsDialog: state.showPanelsDialog, syncStarted: state.syncStarted, appState: state.appState, noteSelectionEnabled: state.noteSelectionEnabled,