From 49d0b3ea0df427c4ab9e28584829a5f74f3d540c Mon Sep 17 00:00:00 2001 From: Pedro Ladaria Date: Mon, 4 Mar 2024 13:32:16 +0100 Subject: [PATCH 1/4] WEB-1761 remove history hacks in dialog --- src/__tests__/dialog-test.tsx | 58 ----------------------------------- src/dialog.tsx | 25 +-------------- 2 files changed, 1 insertion(+), 82 deletions(-) diff --git a/src/__tests__/dialog-test.tsx b/src/__tests__/dialog-test.tsx index cb8f0236c3..309a527695 100644 --- a/src/__tests__/dialog-test.tsx +++ b/src/__tests__/dialog-test.tsx @@ -50,11 +50,6 @@ const TestComponent = () => { ); }; -beforeEach(() => { - // The history object is not cleared between tests. This way we put the history position at the end - window.history.pushState({}, '', '/'); -}); - test('does not render anything initially', () => { const {asFragment} = render(); expect(asFragment()).toMatchInlineSnapshot(``); @@ -281,56 +276,3 @@ test('when webview bridge is available nativeConfirm is shown', async () => { }); }); }); - -test('history restored after closing a dialog using back', async () => { - const pushStateSpy = jest.spyOn(window.history, 'pushState'); - const backSpy = jest.spyOn(window.history, 'back'); - const initialHistoryLength = window.history.length; - - render( - - - - ); - - const alertButton = await screen.findByRole('button', {name: 'Alert'}); - await userEvent.click(alertButton); - - expect(await screen.findByRole('dialog')).toBeInTheDocument(); - expect(window.history.length).toBe(initialHistoryLength + 1); - - act(() => { - window.history.back(); - }); - - await waitForElementToBeRemoved(() => screen.queryByRole('dialog', {hidden: true})); - - expect(pushStateSpy).toHaveBeenCalledTimes(1); - expect(backSpy).toHaveBeenCalledTimes(1); -}); - -test('history restored after closing a dialog using a button', async () => { - const pushStateSpy = jest.spyOn(window.history, 'pushState'); - const backSpy = jest.spyOn(window.history, 'back'); - const initialHistoryLength = window.history.length; - - render( - - - - ); - - const alertButton = await screen.findByRole('button', {name: 'Alert'}); - await userEvent.click(alertButton); - - expect(await screen.findByRole('dialog')).toBeInTheDocument(); - expect(window.history.length).toBe(initialHistoryLength + 1); - - const acceptButton = await screen.findByRole('button', {name: 'Yay!'}); - await userEvent.click(acceptButton); - - await waitForElementToBeRemoved(() => screen.queryByRole('dialog', {hidden: true})); - - expect(pushStateSpy).toHaveBeenCalledTimes(1); - expect(backSpy).toHaveBeenCalledTimes(1); -}); diff --git a/src/dialog.tsx b/src/dialog.tsx index 0a7878ed12..7f01f49d0a 100644 --- a/src/dialog.tsx +++ b/src/dialog.tsx @@ -220,9 +220,7 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { /** this flag is used to disable user interactions while the component is animating */ const [isReady, setIsReady] = React.useState(false); const dialogWasAcceptedRef = React.useRef(false); - const historyWasPushedRef = React.useRef(false); const animationDurationRef = React.useRef(shouldAnimate() ? styles.ANIMATION_DURATION_MS : 0); - const handleBackNavigationRef = React.useRef(() => {}); const shouldRenderNative = props.type !== 'dialog' && isWebViewBridgeAvailable(); const shouldDismissOnPressOverlay = props.type === 'dialog'; @@ -254,11 +252,6 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { }, [onAccept, onCancel, onDestroy]); const startClosing = React.useCallback(() => { - window.removeEventListener('popstate', handleBackNavigationRef.current); - if (historyWasPushedRef.current) { - historyWasPushedRef.current = false; - window.history.back(); - } let timeout: NodeJS.Timeout; if (!isClosingRef.current && isReady) { isClosingRef.current = true; @@ -294,27 +287,11 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { } }, [handleAccept, handleCancel, shouldAcceptOnDismiss]); - const handleBackNavigation = React.useCallback(() => { - if (historyWasPushedRef.current) { - historyWasPushedRef.current = false; - dismiss(); - } - }, [dismiss]); - React.useEffect(() => { if (shouldRenderNative) { return; } - if (!historyWasPushedRef.current && !isClosingRef.current) { - historyWasPushedRef.current = true; - window.history.pushState(null, document.title, window.location.href); - } - handleBackNavigationRef.current = handleBackNavigation; - window.addEventListener('popstate', handleBackNavigationRef.current); - return () => { - window.removeEventListener('popstate', handleBackNavigationRef.current); - }; - }, [handleBackNavigation, shouldRenderNative]); + }, [shouldRenderNative]); const handleKeyDown = React.useCallback( (event: KeyboardEvent) => { From 24517efa239130a4644eda42faac520e624dc76b Mon Sep 17 00:00:00 2001 From: Pedro Ladaria Date: Mon, 11 Mar 2024 12:29:36 +0100 Subject: [PATCH 2/4] WEB-1761 dismiss on popstate --- src/dialog.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/dialog.tsx b/src/dialog.tsx index 7f01f49d0a..cea5d8b3d9 100644 --- a/src/dialog.tsx +++ b/src/dialog.tsx @@ -314,6 +314,13 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { }; }, [handleKeyDown, shouldRenderNative]); + React.useEffect(() => { + window.addEventListener('popstate', dismiss); + return () => { + window.removeEventListener('popstate', dismiss); + }; + }, [dismiss]); + const handleOverlayPress = React.useCallback( (event: React.MouseEvent) => { event.stopPropagation(); From f1ac27dcfebfd9dc48b3dabc4084ae7a90198dfd Mon Sep 17 00:00:00 2001 From: Pedro Ladaria Date: Mon, 11 Mar 2024 13:20:59 +0100 Subject: [PATCH 3/4] WEB-1761 close dialog on navigation --- src/dialog.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dialog.tsx b/src/dialog.tsx index cea5d8b3d9..8a5f1d2965 100644 --- a/src/dialog.tsx +++ b/src/dialog.tsx @@ -315,6 +315,7 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { }, [handleKeyDown, shouldRenderNative]); React.useEffect(() => { + // Consider any page navigation as a dismiss window.addEventListener('popstate', dismiss); return () => { window.removeEventListener('popstate', dismiss); From a7fb3c6f9fdecff5dddf39b7c44557449bb567a5 Mon Sep 17 00:00:00 2001 From: Pedro Ladaria Date: Mon, 11 Mar 2024 15:34:54 +0100 Subject: [PATCH 4/4] WEB-1761 cleanup --- src/dialog.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/dialog.tsx b/src/dialog.tsx index 8a5f1d2965..f581397b7f 100644 --- a/src/dialog.tsx +++ b/src/dialog.tsx @@ -287,12 +287,6 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { } }, [handleAccept, handleCancel, shouldAcceptOnDismiss]); - React.useEffect(() => { - if (shouldRenderNative) { - return; - } - }, [shouldRenderNative]); - const handleKeyDown = React.useCallback( (event: KeyboardEvent) => { if (event.key === ESC) { @@ -315,12 +309,15 @@ const ModalDialog = (props: ModalDialogProps): JSX.Element => { }, [handleKeyDown, shouldRenderNative]); React.useEffect(() => { + if (shouldRenderNative) { + return; + } // Consider any page navigation as a dismiss window.addEventListener('popstate', dismiss); return () => { window.removeEventListener('popstate', dismiss); }; - }, [dismiss]); + }, [dismiss, shouldRenderNative]); const handleOverlayPress = React.useCallback( (event: React.MouseEvent) => {