Repository navigation
Replace modal window.history guard with state.history sentinel (#90776) #92492
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
Changes from all commits
e9f76de
5c853ae
3041426
9c777cb
b9d575b
6352f24
3a1fe01
824f3c0
2df430a
0c78b04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,2 @@ | ||||||||||||||||||||||||||||||||||||||
| // Modal back-guard synchronization with the browser history is only supported for web. | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: is this to limit scope, or because the problem doesn't exist on native? Asking because I know Android has a native back button
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this PR, we changed the approach for the web only, since previously a workaround for the back button on the web was used only in modal windows—because that approach relies on Here is the current code snippet: App/src/components/Modal/index.tsx Lines 111 to 128 in 83bf7f3
A different approach is used on native devices, and this PR does not change anything for native devices |
||||||||||||||||||||||||||||||||||||||
| export default function useSyncModalWithHistory() {} | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| import {useEffect, useEffectEvent, useId, useRef, useSyncExternalStore} from 'react'; | ||
| import subscribeToRootNavigation from '@libs/Navigation/helpers/subscribeToRootNavigation'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import navigationRef from '@libs/Navigation/navigationRef'; | ||
| import CONST from '@src/CONST'; | ||
| import {EMPTY_MODAL_GUARD_SNAPSHOT_KEY, getModalGuardSnapshotKey, parseModalGuardSnapshotKey} from './modalGuardSnapshot'; | ||
| import reduceModalGuardState, {getModalGuardEventFromSnapshotChange, MODAL_GUARD_EFFECT, MODAL_GUARD_STATE} from './modalGuardState'; | ||
| import type {ModalGuardState} from './modalGuardState'; | ||
|
|
||
| type UseSyncModalWithHistoryParams = { | ||
| /** Whether the modal is currently visible */ | ||
| isVisible: boolean; | ||
|
|
||
| /** Whether this modal participates in browser-history back handling */ | ||
| shouldHandleNavigationBack?: boolean; | ||
|
|
||
| /** Called when a browser Back press removes this modal's history entry */ | ||
| onClose?: () => void; | ||
|
|
||
| /** Called when browser Forward navigation restores this modal's history entry while the modal is closed */ | ||
| onOpen?: () => void; | ||
| }; | ||
|
|
||
| /** | ||
| * Web: represents a `shouldHandleNavigationBack` modal's back-guard as a uniquely-tagged sentinel in the | ||
| * root navigator's `state.history` (dispatched via `TOGGLE_MODAL_WITH_HISTORY`). React Navigation's | ||
| * `useLinking` mirrors the history length delta into the browser, so opening the modal pushes a browser | ||
| * entry and browser Back removes it. The router consumes the sentinel on forward navigation | ||
| * (history length unchanged → `replaceState`) so no orphaned entry is left behind. | ||
| * | ||
| * The per-instance tag lets nested modals add/remove their own guard independently (LIFO). | ||
| */ | ||
| export default function useSyncModalWithHistory({isVisible, shouldHandleNavigationBack, onClose, onOpen}: UseSyncModalWithHistoryParams) { | ||
| const modalId = useId(); | ||
| const sentinel = `${CONST.NAVIGATION.CUSTOM_HISTORY_ENTRY_MODAL}:${modalId}`; | ||
|
|
||
| const guardStateRef = useRef<ModalGuardState>(MODAL_GUARD_STATE.CLOSED); | ||
|
|
||
| const onCloseEvent = useEffectEvent(() => { | ||
| onClose?.(); | ||
| }); | ||
|
|
||
| const onOpenEvent = useEffectEvent(() => { | ||
| onOpen?.(); | ||
| }); | ||
|
|
||
| const snapshotKey = useSyncExternalStore( | ||
| subscribeToRootNavigation, | ||
| () => getModalGuardSnapshotKey(sentinel), | ||
| () => EMPTY_MODAL_GUARD_SNAPSHOT_KEY, | ||
| ); | ||
| // We can't use usePrevious here because we need to imperatively reset this ref mid-effect | ||
| // (to sync up when shouldHandleNavigationBack is false, so no stale transition fires when | ||
| // the flag toggles back on). usePrevious only updates after render and is read-only to callers. | ||
| const prevSnapshotKeyRef = useRef(snapshotKey); | ||
|
rlinoz marked this conversation as resolved.
|
||
|
|
||
| // Add the guard entry on open / remove it on close. | ||
| useEffect(() => { | ||
| if (!shouldHandleNavigationBack) { | ||
| return; | ||
| } | ||
|
|
||
| if (isVisible) { | ||
| guardStateRef.current = MODAL_GUARD_STATE.OPEN; | ||
| Navigation.isNavigationReady().then(() => { | ||
| navigationRef.dispatch({ | ||
| type: CONST.NAVIGATION.ACTION_TYPE.TOGGLE_MODAL_WITH_HISTORY, | ||
| payload: {isVisible: true, modalId}, | ||
| }); | ||
| }); | ||
|
rlinoz marked this conversation as resolved.
|
||
| return () => { | ||
| if (guardStateRef.current !== MODAL_GUARD_STATE.OPEN) { | ||
| return; | ||
| } | ||
| guardStateRef.current = MODAL_GUARD_STATE.CLOSED; | ||
| navigationRef.dispatch({ | ||
| type: CONST.NAVIGATION.ACTION_TYPE.TOGGLE_MODAL_WITH_HISTORY, | ||
| payload: {isVisible: false, modalId}, | ||
| }); | ||
| }; | ||
| } | ||
|
|
||
| if (guardStateRef.current !== MODAL_GUARD_STATE.OPEN) { | ||
| return; | ||
| } | ||
| guardStateRef.current = MODAL_GUARD_STATE.CLOSING_BY_DISPATCH; | ||
| // Defer (microtask via isNavigationReady) so any forward navigation fired from the same close | ||
| // handler is dispatched first; the router then consumes our sentinel during that push and this | ||
| // toggle(false) becomes a no-op, avoiding an extra browser back(). | ||
| Navigation.isNavigationReady().then(() => { | ||
| navigationRef.dispatch({ | ||
| type: CONST.NAVIGATION.ACTION_TYPE.TOGGLE_MODAL_WITH_HISTORY, | ||
| payload: {isVisible: false, modalId}, | ||
| }); | ||
| }); | ||
| }, [isVisible, shouldHandleNavigationBack, modalId]); | ||
|
|
||
| // Browser Back/Forward changes the guard sentinel in root history — react via snapshot transitions. | ||
| useEffect(() => { | ||
| if (!shouldHandleNavigationBack) { | ||
| prevSnapshotKeyRef.current = snapshotKey; | ||
| return; | ||
| } | ||
|
|
||
| if (prevSnapshotKeyRef.current === snapshotKey) { | ||
| return; | ||
| } | ||
|
|
||
| const prevSnapshot = parseModalGuardSnapshotKey(prevSnapshotKeyRef.current); | ||
| const nextSnapshot = parseModalGuardSnapshotKey(snapshotKey); | ||
| prevSnapshotKeyRef.current = snapshotKey; | ||
|
|
||
| const event = getModalGuardEventFromSnapshotChange(prevSnapshot, nextSnapshot, isVisible); | ||
| if (!event) { | ||
| return; | ||
| } | ||
|
|
||
| const {state, effect} = reduceModalGuardState(guardStateRef.current, event, isVisible); | ||
| guardStateRef.current = state; | ||
|
|
||
| if (effect === MODAL_GUARD_EFFECT.ON_CLOSE) { | ||
| onCloseEvent(); | ||
| } else if (effect === MODAL_GUARD_EFFECT.ON_OPEN) { | ||
| onOpenEvent(); | ||
| } | ||
| }, [snapshotKey, isVisible, shouldHandleNavigationBack]); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import navigationRef from '@libs/Navigation/navigationRef'; | ||
|
|
||
| // Slice of navigation state consumed by useSyncExternalStore to detect guard presence and route-count changes. | ||
| type ModalGuardSnapshot = { | ||
| guardPresent: boolean; | ||
| routesLength: number; | ||
| }; | ||
|
Comment on lines
+4
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add comment here |
||
|
|
||
| const EMPTY_MODAL_GUARD_SNAPSHOT: ModalGuardSnapshot = { | ||
| guardPresent: false, | ||
| routesLength: 0, | ||
| }; | ||
|
|
||
| const EMPTY_MODAL_GUARD_SNAPSHOT_KEY = 'false:0'; | ||
|
|
||
| function getModalGuardSnapshot(sentinel: string): ModalGuardSnapshot { | ||
| if (!navigationRef.isReady()) { | ||
| return EMPTY_MODAL_GUARD_SNAPSHOT; | ||
| } | ||
|
|
||
| const state = navigationRef.getRootState(); | ||
| return { | ||
| guardPresent: !!state?.history?.includes(sentinel), | ||
| routesLength: state?.routes?.length ?? 0, | ||
| }; | ||
| } | ||
|
|
||
| function serializeModalGuardSnapshot(snapshot: ModalGuardSnapshot): string { | ||
| return `${snapshot.guardPresent}:${snapshot.routesLength}`; | ||
| } | ||
|
|
||
| function getModalGuardSnapshotKey(sentinel: string): string { | ||
| return serializeModalGuardSnapshot(getModalGuardSnapshot(sentinel)); | ||
| } | ||
|
|
||
| function parseModalGuardSnapshotKey(snapshotKey: string): ModalGuardSnapshot { | ||
| const [guardPresent, routesLength] = snapshotKey.split(':'); | ||
| return { | ||
| guardPresent: guardPresent === 'true', | ||
| routesLength: Number(routesLength), | ||
| }; | ||
| } | ||
|
|
||
| export type {ModalGuardSnapshot}; | ||
| export {EMPTY_MODAL_GUARD_SNAPSHOT_KEY, getModalGuardSnapshotKey, parseModalGuardSnapshotKey}; | ||
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.
When a
shouldHandleNavigationBackmodal is the discard-confirmation dialog,useDiscardChangesConfirmation.markNextBeforeRemoveAsModalCleanup()still checkswindow.history.state.shouldGoBackbefore ignoring the modal's synthetic cleanup back. This new sentinel path no longer writes that flag, so on web pages with unsaved changes, dismissing or confirming the discard dialog can let the modal-history cleanup be treated as a real back navigation and re-trigger/block the originalbeforeRemoveflow. Please either keep an equivalent marker in history state or update that hook before routing all modals through this path.Useful? React with 👍 / 👎.