From be2519e5befccf40858ced24746aa71519355898 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 12:06:26 -0700 Subject: [PATCH 01/17] refactor(ReanimatedModal): follow Rules of React, compile with React Compiler - Group useEffects by responsibility; co-locate cleanup with creation - Extract transition callbacks with useEffectEvent to fix stale closure - Derive isTransitioning and eliminate redundant isVisibleState state - Remove manual memoization (useCallback/useMemo) superseded by React Compiler - DRY up clearTransitionHandles helper - Sort component internals: hooks, refs, derived, callbacks, effects Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 156 +++++++----------- 1 file changed, 63 insertions(+), 93 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 5d550fd2d53d..820f2bd4611c 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -1,5 +1,5 @@ import noop from 'lodash/noop'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useEffect, useEffectEvent, useRef, useState} from 'react'; import type {NativeEventSubscription, ViewStyle} from 'react-native'; // eslint-disable-next-line no-restricted-imports import {BackHandler, InteractionManager, Modal, StyleSheet, View} from 'react-native'; @@ -55,37 +55,64 @@ function ReanimatedModal({ shouldReturnFocus, ...props }: ReanimatedModalProps) { - const [isVisibleState, setIsVisibleState] = useState(isVisible); const [isContainerOpen, setIsContainerOpen] = useState(false); - const [isTransitioning, setIsTransitioning] = useState(false); const {windowWidth, windowHeight} = useWindowDimensions(); + const styles = useThemeStyles(); const backHandlerListener = useRef(null); const handleRef = useRef(undefined); const transitionHandleRef = useRef(null); - const styles = useThemeStyles(); + const isTransitioning = isVisible !== isContainerOpen; + const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; + const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; - const onBackButtonPressHandler = useCallback(() => { + const onBackButtonPressHandler = () => { if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { return false; } - if (isVisibleState) { + if (isVisible) { onBackButtonPress(); return true; } return false; - }, [isVisibleState, onBackButtonPress, isTransitioning, shouldIgnoreBackHandlerDuringTransition]); + }; - const handleEscape = useCallback( - (e: KeyboardEvent) => { - if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { - return; - } - e.stopImmediatePropagation(); - }, - [onBackButtonPressHandler], - ); + const handleEscape = (e: KeyboardEvent) => { + if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { + return; + } + e.stopImmediatePropagation(); + }; + + const clearTransitionHandles = () => { + if (handleRef.current) { + InteractionManager.clearInteractionHandle(handleRef.current); + handleRef.current = undefined; + } + if (transitionHandleRef.current) { + TransitionTracker.endTransition(transitionHandleRef.current); + transitionHandleRef.current = null; + } + }; + + const onOpenCallBack = () => { + setIsContainerOpen(true); + clearTransitionHandles(); + onModalShow(); + }; + + const onCloseCallBack = () => { + setIsContainerOpen(false); + clearTransitionHandles(); + + // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: + // https://stackoverflow.com/questions/58937956/react-native-modal-ondismiss-not-invoked + // Therefore, we manually call onModalHide() here for Android. + if (getPlatform() === CONST.PLATFORM.ANDROID) { + onModalHide(); + } + }; useEffect(() => { if (getPlatform() === CONST.PLATFORM.WEB) { @@ -103,86 +130,29 @@ function ReanimatedModal({ }; }, [handleEscape, onBackButtonPressHandler]); - useEffect( - () => () => { - if (handleRef.current) { - // eslint-disable-next-line @typescript-eslint/no-deprecated - InteractionManager.clearInteractionHandle(handleRef.current); - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } - - setIsVisibleState(false); - setIsContainerOpen(false); - }, - - [], - ); - useEffect(() => { - if (isVisible && !isContainerOpen && !isTransitioning) { - // eslint-disable-next-line @typescript-eslint/no-deprecated - handleRef.current = InteractionManager.createInteractionHandle(); - transitionHandleRef.current = TransitionTracker.startTransition(); - onModalWillShow(); - - // eslint-disable-next-line react-hooks/set-state-in-effect - setIsVisibleState(true); - setIsTransitioning(true); - } else if (!isVisible && isContainerOpen && !isTransitioning) { + if (isTransitioning) { handleRef.current = InteractionManager.createInteractionHandle(); transitionHandleRef.current = TransitionTracker.startTransition(); - onModalWillHide(); - - blurActiveElement(); - setIsVisibleState(false); - setIsTransitioning(true); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isVisible, isContainerOpen, isTransitioning]); - - const backdropStyle: ViewStyle = useMemo(() => { - return {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; - }, [windowWidth, windowHeight, backdropColor]); - const onOpenCallBack = useCallback(() => { - setIsTransitioning(false); - setIsContainerOpen(true); - if (handleRef.current) { - // eslint-disable-next-line @typescript-eslint/no-deprecated - InteractionManager.clearInteractionHandle(handleRef.current); - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } - onModalShow(); - }, [onModalShow]); - - const onCloseCallBack = useCallback(() => { - setIsTransitioning(false); - setIsContainerOpen(false); - if (handleRef.current) { - InteractionManager.clearInteractionHandle(handleRef.current); - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } + return () => { + clearTransitionHandles(); + }; + }, [isTransitioning]); - // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: - // https://stackoverflow.com/questions/58937956/react-native-modal-ondismiss-not-invoked - // Therefore, we manually call onModalHide() here for Android. - if (getPlatform() === CONST.PLATFORM.ANDROID) { - onModalHide(); + const fireTransitionCallbacks = useEffectEvent(() => { + if (isVisible && !isContainerOpen) { + onModalWillShow(); + } else if (!isVisible && isContainerOpen) { + onModalWillHide(); + blurActiveElement(); } - }, [onModalHide]); + }); - const modalStyle = useMemo(() => { - return {zIndex: StyleSheet.flatten(style)?.zIndex}; - }, [style]); + useEffect(() => { + fireTransitionCallbacks(); + }, [isVisible, isContainerOpen]); const containerView = ( ); - if (!coverScreen && isVisibleState) { + if (!coverScreen && isVisible) { return ( ); } - const isBackdropMounted = isVisibleState || ((isTransitioning || isContainerOpen !== isVisibleState) && getPlatform() === CONST.PLATFORM.WEB); - const modalVisibility = isVisibleState || isTransitioning || isContainerOpen !== isVisibleState; + const isBackdropMounted = isVisible || (isTransitioning && getPlatform() === CONST.PLATFORM.WEB); + const modalVisibility = isVisible || isTransitioning; return ( - {isVisibleState && containerView} + {isVisible && containerView} ) : ( - {isVisibleState && containerView} + {isVisible && containerView} )} From 2e0c129ffe017c58bc272546f105c276b0947a2d Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 13:03:33 -0700 Subject: [PATCH 02/17] test(ReanimatedModal): add regression tests for transition state machine Covers four staging regressions from the React Compiler refactor: - #90438 RHP no animation (derived isTransitioning loses "in-progress" semantics on oscillation) - #90442 Android can't confirm deletion (interaction handle cleared prematurely) - #90463 Android modal not displayed (interaction handle cleared prematurely) - #90510 Web flickering on close (Container unmounted before exit animation plays) Tests are intentionally red until the fixes are applied. Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 2 + tests/unit/ReanimatedModalTest.tsx | 290 ++++++++++++++++++ 2 files changed, 292 insertions(+) create mode 100644 tests/unit/ReanimatedModalTest.tsx diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 820f2bd4611c..bf510b9dc9ed 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -87,6 +87,7 @@ function ReanimatedModal({ const clearTransitionHandles = () => { if (handleRef.current) { + // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.clearInteractionHandle(handleRef.current); handleRef.current = undefined; } @@ -132,6 +133,7 @@ function ReanimatedModal({ useEffect(() => { if (isTransitioning) { + // eslint-disable-next-line @typescript-eslint/no-deprecated handleRef.current = InteractionManager.createInteractionHandle(); transitionHandleRef.current = TransitionTracker.startTransition(); } diff --git a/tests/unit/ReanimatedModalTest.tsx b/tests/unit/ReanimatedModalTest.tsx new file mode 100644 index 000000000000..e8389fa163ce --- /dev/null +++ b/tests/unit/ReanimatedModalTest.tsx @@ -0,0 +1,290 @@ +/** + * Unit tests for ReanimatedModal's transition state machine. + * + * These tests guard against regressions introduced when isTransitioning was + * converted from explicit state to a derived value (isVisible !== isContainerOpen). + * The derived approach fails when isVisible oscillates back to match isContainerOpen + * mid-animation, causing premature handle cleanup and broken animations. + * + * Related staging regressions: + * - https://github.com/Expensify/App/issues/90438 (RHP does not animate) + * - https://github.com/Expensify/App/issues/90442 (Unable to delete on Android) + * - https://github.com/Expensify/App/issues/90463 (Modal not displayed on Android) + * - https://github.com/Expensify/App/issues/90510 (Web flickering on close) + */ +import {act, render} from '@testing-library/react-native'; +import React from 'react'; +import {InteractionManager} from 'react-native'; +import ReanimatedModal from '@components/Modal/ReanimatedModal'; +// eslint-disable-next-line no-restricted-imports +import TransitionTracker from '@libs/Navigation/TransitionTracker'; + +// --------------------------------------------------------------------------- +// Container mock +// +// Reanimated's Keyframe.withCallback() is a no-op in the test environment, +// so onOpenCallBack / onCloseCallBack are never called by animations. +// This mock captures the latest callbacks so tests can trigger them manually +// to simulate animation completion. +// --------------------------------------------------------------------------- +let capturedOnOpenCallBack: (() => void) | undefined; +let capturedOnCloseCallBack: (() => void) | undefined; + +jest.mock('@components/Modal/ReanimatedModal/Container', () => { + const {View} = require('react-native'); + return function MockContainer({onOpenCallBack, onCloseCallBack, children, ...rest}: Record) { + capturedOnOpenCallBack = onOpenCallBack as () => void; + capturedOnCloseCallBack = onCloseCallBack as () => void; + return ( + + {children as React.ReactNode} + + ); + }; +}); + +jest.mock('@components/Modal/ReanimatedModal/Backdrop', () => { + const {View} = require('react-native'); + return function MockBackdrop({children}: {children?: React.ReactNode}) { + return {children}; + }; +}); + +jest.mock('@components/FocusTrap/FocusTrapForModal', () => { + const {View} = require('react-native'); + return function MockFocusTrap({children}: {children?: React.ReactNode}) { + return {children}; + }; +}); + +jest.mock('@hooks/useThemeStyles', () => () => ({})); +jest.mock('@hooks/useWindowDimensions', () => () => ({windowWidth: 375, windowHeight: 667})); +jest.mock('@libs/Accessibility/blurActiveElement', () => jest.fn()); + +// Use a non-web platform so the back handler path is exercised and +// platform-specific branches stay consistent across tests. +jest.mock('@libs/getPlatform', () => jest.fn(() => 'ios')); + +// --------------------------------------------------------------------------- + +describe('ReanimatedModal', () => { + let startTransitionSpy: jest.SpyInstance; + let endTransitionSpy: jest.SpyInstance; + let createHandleSpy: jest.SpyInstance; + let clearHandleSpy: jest.SpyInstance; + + beforeEach(() => { + capturedOnOpenCallBack = undefined; + capturedOnCloseCallBack = undefined; + startTransitionSpy = jest.spyOn(TransitionTracker, 'startTransition'); + endTransitionSpy = jest.spyOn(TransitionTracker, 'endTransition'); + createHandleSpy = jest.spyOn(InteractionManager, 'createInteractionHandle'); + clearHandleSpy = jest.spyOn(InteractionManager, 'clearInteractionHandle'); + }); + + afterEach(() => { + // Drain any open transitions so TransitionTracker's internal counter + // is clean for the next test. + jest.runAllTimers(); + jest.clearAllMocks(); + }); + + // ----------------------------------------------------------------------- + // Helpers + // ----------------------------------------------------------------------- + + /** Simulates the opening animation completing. */ + function completeOpenAnimation() { + act(() => { + capturedOnOpenCallBack?.(); + }); + } + + /** Simulates the closing animation completing. */ + function completeCloseAnimation() { + act(() => { + capturedOnCloseCallBack?.(); + }); + } + + // ----------------------------------------------------------------------- + // Oscillation tests — guard for https://github.com/Expensify/App/issues/90438 + // ----------------------------------------------------------------------- + + describe('transition handles during isVisible oscillation', () => { + /** + * Regression guard for https://github.com/Expensify/App/issues/90438 + * + * When isVisible briefly flips false → true while the closing animation is + * still running (isContainerOpen is still true), the transition should remain + * active. The derived-value bug ends the transition prematurely because + * isTransitioning = isVisible !== isContainerOpen = true !== true = false, + * which triggers the handles effect's cleanup. + */ + it('does not end an active transition when isVisible oscillates during a closing animation', async () => { + const afterTransitionsCallback = jest.fn(); + const {rerender, unmount} = render(); + + // 1. Open the modal and complete the entering animation. + await act(async () => { + rerender(); + }); + expect(startTransitionSpy).toHaveBeenCalledTimes(1); + completeOpenAnimation(); + // isContainerOpen is now true; transition handles cleared. + + // 2. Begin closing. + await act(async () => { + rerender(); + }); + expect(startTransitionSpy).toHaveBeenCalledTimes(2); + // Closing animation is in progress — onCloseCallBack has NOT fired yet. + + // 3. isVisible oscillates back to true before the exit animation finishes. + // This simulates a prop change mid-animation, e.g. from rapid RHP navigation. + await act(async () => { + rerender(); + }); + // isContainerOpen is still true (close animation not complete). + // With the derived-value bug: isTransitioning = true !== true = false + // → the handles effect cleanup fires → endTransition called prematurely. + + // 4. Queue a callback that should run only after all transitions end. + TransitionTracker.runAfterTransitions({callback: afterTransitionsCallback}); + await jest.runAllTimersAsync(); + + // ASSERTION: The closing transition is still in progress, so the callback + // should NOT have fired yet. With the bug, endTransition was called in step 3, + // leaving no active transition, so the callback fires immediately. + expect(afterTransitionsCallback).not.toHaveBeenCalled(); + + unmount(); + }); + + /** + * Regression guard for https://github.com/Expensify/App/issues/90442 + * and https://github.com/Expensify/App/issues/90463 + * + * The interaction handle created at the start of the closing animation must + * not be cleared until onCloseCallBack fires. If it is cleared early, + * InteractionManager.runAfterInteractions tasks (e.g. showing a confirmation + * dialog, focusing a text input) can fire while the animation is still playing. + */ + it('does not clear the interaction handle prematurely when isVisible oscillates during a closing animation', async () => { + const afterInteractionsCallback = jest.fn(); + const {rerender, unmount} = render(); + + // 1. Open and complete the entering animation. + await act(async () => { + rerender(); + }); + completeOpenAnimation(); + + // 2. Begin closing. + await act(async () => { + rerender(); + }); + // Closing animation is in progress — an interaction handle is now active. + + // 3. isVisible oscillates back before the exit animation finishes. + await act(async () => { + rerender(); + }); + // With the derived-value bug: isTransitioning = false → effect cleanup fires + // → clearInteractionHandle called prematurely. + + // 4. Queue work that should wait until all animations are done. + InteractionManager.runAfterInteractions(afterInteractionsCallback); + await jest.runAllTimersAsync(); + + // ASSERTION: The handle from the closing animation should still be active, + // blocking runAfterInteractions from firing. With the bug, the handle was + // already cleared in step 3, so the callback fires immediately. + expect(afterInteractionsCallback).not.toHaveBeenCalled(); + + unmount(); + }); + }); + + // ----------------------------------------------------------------------- + // Container visibility test — guard for https://github.com/Expensify/App/issues/90510 + // ----------------------------------------------------------------------- + + describe('container visibility during animations', () => { + /** + * Regression guard for https://github.com/Expensify/App/issues/90510 + * + * The Container must remain mounted while the exit animation is playing so + * that the animation has a component to animate. With {isVisible && containerView}, + * the Container is unmounted immediately when isVisible becomes false, before + * onCloseCallBack fires. The fix changes this to + * {(isVisible || isTransitioning) && containerView}. + */ + it('keeps the Container mounted during a closing animation so the exit animation can complete', async () => { + const {rerender, getByTestId, unmount} = render(); + + // 1. Open the modal and complete the entering animation. + await act(async () => { + rerender(); + }); + completeOpenAnimation(); + // isContainerOpen is now true. + + // 2. Begin closing — the exit animation should start playing. + await act(async () => { + rerender(); + }); + // isTransitioning is true (isContainerOpen=true, isVisible=false). + // The Modal wrapper stays visible (modalVisibility = false || true = true). + + // ASSERTION: The Container must still be in the tree so Reanimated can + // play the exit animation. With the bug ({isVisible && containerView}), + // the Container is unmounted immediately when isVisible becomes false. + expect(getByTestId('mock-modal-container')).toBeOnTheScreen(); + + // 3. Animation completes — now the Container should be removed. + completeCloseAnimation(); + // isContainerOpen is now false; isTransitioning = false; + // (isVisible || isTransitioning) = false — Container should unmount. + + // Verify the container is gone after the animation. + expect(() => getByTestId('mock-modal-container')).toThrow(); + + unmount(); + }); + }); + + // ----------------------------------------------------------------------- + // Baseline: handle lifecycle in a normal open → close cycle + // + // This test documents the CORRECT lifecycle: one handle per transition phase, + // cleared only when the animation callback fires — not before. + // ----------------------------------------------------------------------- + + describe('interaction handle lifecycle', () => { + it('does not clear an interaction handle before the opening animation callback fires', async () => { + const {rerender, unmount} = render(); + + await act(async () => { + rerender(); + }); + + // One handle should be active for the opening animation. + expect(createHandleSpy).toHaveBeenCalledTimes(1); + // The handle must NOT be cleared yet — the animation is still playing. + // With the bug, derived isTransitioning causes extra effect cleanup cycles that + // release the handle before the animation callback fires. + expect(clearHandleSpy).toHaveBeenCalledTimes(0); + + completeOpenAnimation(); + // Now that the animation is done, the handle should have been cleared. + expect(clearHandleSpy).toHaveBeenCalledTimes(1); + + unmount(); + }); + }); +}); From 1651cd086fd37ff18ed05991dbb66b072ef8bfbd Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 15:49:03 -0700 Subject: [PATCH 03/17] fix(ReanimatedModal): stabilize back handler with useEffectEvent Wraps onBackButtonPressHandler and handleEscape in useEffectEvent so they hold a stable reference across renders without needing deps. The back handler effect can now use [] as its dep array, eliminating re-registration on every render cycle. Fixes #90442 and #90463. Co-authored-by: Cursor --- src/components/Modal/ReanimatedModal/index.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index bf510b9dc9ed..349491d846e4 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -67,7 +67,7 @@ function ReanimatedModal({ const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; - const onBackButtonPressHandler = () => { + const onBackButtonPressHandler = useEffectEvent(() => { if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { return false; } @@ -76,14 +76,14 @@ function ReanimatedModal({ return true; } return false; - }; + }); - const handleEscape = (e: KeyboardEvent) => { + const handleEscape = useEffectEvent((e: KeyboardEvent) => { if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { return; } e.stopImmediatePropagation(); - }; + }); const clearTransitionHandles = () => { if (handleRef.current) { @@ -129,7 +129,7 @@ function ReanimatedModal({ backHandlerListener.current?.remove(); } }; - }, [handleEscape, onBackButtonPressHandler]); + }, []); useEffect(() => { if (isTransitioning) { @@ -207,6 +207,7 @@ function ReanimatedModal({ transparent animationType="none" visible={modalVisibility} + // eslint-disable-next-line react-hooks/rules-of-hooks onRequestClose={onBackButtonPressHandler} statusBarTranslucent={statusBarTranslucent} testID={testID} From 27fe5c70f0aa71b01ce656543ed7f4d91ef1ada8 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 16:06:27 -0700 Subject: [PATCH 04/17] fix(ReanimatedModal): replace isContainerOpen bool with four-state enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces two booleans (isContainerOpen + derived isTransitioning) with a single explicit ModalState enum: 'closed' | 'opening' | 'open' | 'closing'. The enum makes impossible states impossible and fixes oscillation bugs: - modalState only advances via animation callbacks (onOpenCallBack / onCloseCallBack), so it can never flip back due to isVisible oscillating. - The isVisible → state transition uses the render-time state adjustment pattern instead of a useEffect, eliminating extra renders. - Container visibility uses (modalState === 'opening' || modalState === 'open') so the Container unmounts when closing begins, triggering Reanimated's ghost- node exit animation, and does not re-mount during isVisible oscillation. Updates regression tests to use reliable spy-count assertions; removes dependency on InteractionManager.runAfterInteractions which is mocked to fire immediately in the test environment. Fixes #90438, #90510. Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 62 +++++++-- tests/unit/ReanimatedModalTest.tsx | 119 +++++++++--------- 2 files changed, 111 insertions(+), 70 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 349491d846e4..dfe82452f257 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -20,6 +20,8 @@ import Backdrop from './Backdrop'; import Container from './Container'; import type ReanimatedModalProps from './types'; +type ModalState = 'closed' | 'opening' | 'open' | 'closing'; + function ReanimatedModal({ testID, animationInDelay, @@ -55,18 +57,37 @@ function ReanimatedModal({ shouldReturnFocus, ...props }: ReanimatedModalProps) { - const [isContainerOpen, setIsContainerOpen] = useState(false); + // --- Hooks --- + const [modalState, setModalState] = useState('closed'); + const [prevIsVisible, setPrevIsVisible] = useState(isVisible); const {windowWidth, windowHeight} = useWindowDimensions(); const styles = useThemeStyles(); + // --- Refs --- const backHandlerListener = useRef(null); const handleRef = useRef(undefined); const transitionHandleRef = useRef(null); - const isTransitioning = isVisible !== isContainerOpen; + // --- Render-time state adjustment --- + // When isVisible changes, advance the state machine from stable states only. + // Mid-animation changes are ignored — the animation runs to completion and the + // final isVisible value is honored when the callback fires. + // See: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + if (prevIsVisible !== isVisible) { + setPrevIsVisible(isVisible); + if (isVisible && modalState === 'closed') { + setModalState('opening'); + } else if (!isVisible && modalState === 'open') { + setModalState('closing'); + } + } + + // --- Derived values --- + const isTransitioning = modalState === 'opening' || modalState === 'closing'; const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; + // --- Callbacks --- const onBackButtonPressHandler = useEffectEvent(() => { if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { return false; @@ -98,13 +119,13 @@ function ReanimatedModal({ }; const onOpenCallBack = () => { - setIsContainerOpen(true); + setModalState('open'); clearTransitionHandles(); onModalShow(); }; const onCloseCallBack = () => { - setIsContainerOpen(false); + setModalState('closed'); clearTransitionHandles(); // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: @@ -115,6 +136,7 @@ function ReanimatedModal({ } }; + // --- Effects --- useEffect(() => { if (getPlatform() === CONST.PLATFORM.WEB) { document.body.addEventListener('keyup', handleEscape, {capture: true}); @@ -132,7 +154,7 @@ function ReanimatedModal({ }, []); useEffect(() => { - if (isTransitioning) { + if (modalState === 'opening' || modalState === 'closing') { // eslint-disable-next-line @typescript-eslint/no-deprecated handleRef.current = InteractionManager.createInteractionHandle(); transitionHandleRef.current = TransitionTracker.startTransition(); @@ -141,12 +163,12 @@ function ReanimatedModal({ return () => { clearTransitionHandles(); }; - }, [isTransitioning]); + }, [modalState]); const fireTransitionCallbacks = useEffectEvent(() => { - if (isVisible && !isContainerOpen) { + if (modalState === 'opening') { onModalWillShow(); - } else if (!isVisible && isContainerOpen) { + } else if (modalState === 'closing') { onModalWillHide(); blurActiveElement(); } @@ -154,8 +176,18 @@ function ReanimatedModal({ useEffect(() => { fireTransitionCallbacks(); - }, [isVisible, isContainerOpen]); + }, [modalState]); + // Safety net: release handles if the component unmounts mid-transition + // (e.g. navigation removes the modal before its exit animation completes). + useEffect( + () => () => { + clearTransitionHandles(); + }, + [], + ); + + // --- JSX --- const containerView = ( ); } - const isBackdropMounted = isVisible || (isTransitioning && getPlatform() === CONST.PLATFORM.WEB); - const modalVisibility = isVisible || isTransitioning; + + // Backdrop stays mounted on web during 'closing' so it can play its own exit animation + // while the Container is still finishing its exit animation. + const isBackdropMounted = isVisible || (modalState === 'closing' && getPlatform() === CONST.PLATFORM.WEB); + const modalVisibility = modalState !== 'closed'; + return ( - {isVisible && containerView} + {(modalState === 'opening' || modalState === 'open') && containerView} ) : ( - {isVisible && containerView} + {(modalState === 'opening' || modalState === 'open') && containerView} )} diff --git a/tests/unit/ReanimatedModalTest.tsx b/tests/unit/ReanimatedModalTest.tsx index e8389fa163ce..eb9728c45ad7 100644 --- a/tests/unit/ReanimatedModalTest.tsx +++ b/tests/unit/ReanimatedModalTest.tsx @@ -1,10 +1,10 @@ /** * Unit tests for ReanimatedModal's transition state machine. * - * These tests guard against regressions introduced when isTransitioning was - * converted from explicit state to a derived value (isVisible !== isContainerOpen). - * The derived approach fails when isVisible oscillates back to match isContainerOpen - * mid-animation, causing premature handle cleanup and broken animations. + * These tests guard against regressions introduced by incorrect isTransitioning + * derivation (isVisible !== isContainerOpen). The derived approach fails when + * isVisible oscillates back to match isContainerOpen mid-animation, causing + * premature handle cleanup and broken animations. * * Related staging regressions: * - https://github.com/Expensify/App/issues/90438 (RHP does not animate) @@ -120,7 +120,7 @@ describe('ReanimatedModal', () => { * Regression guard for https://github.com/Expensify/App/issues/90438 * * When isVisible briefly flips false → true while the closing animation is - * still running (isContainerOpen is still true), the transition should remain + * still running (modalState is still 'closing'), the transition should remain * active. The derived-value bug ends the transition prematurely because * isTransitioning = isVisible !== isContainerOpen = true !== true = false, * which triggers the handles effect's cleanup. @@ -135,7 +135,7 @@ describe('ReanimatedModal', () => { }); expect(startTransitionSpy).toHaveBeenCalledTimes(1); completeOpenAnimation(); - // isContainerOpen is now true; transition handles cleared. + // modalState is now 'open'; transition handles cleared. // 2. Begin closing. await act(async () => { @@ -149,9 +149,9 @@ describe('ReanimatedModal', () => { await act(async () => { rerender(); }); - // isContainerOpen is still true (close animation not complete). - // With the derived-value bug: isTransitioning = true !== true = false - // → the handles effect cleanup fires → endTransition called prematurely. + // modalState is still 'closing' (derived-value bug would compute isTransitioning=false here). + // With the bug: isTransitioning = true !== true = false → the handles effect cleanup + // fires → endTransition called prematurely. // 4. Queue a callback that should run only after all transitions end. TransitionTracker.runAfterTransitions({callback: afterTransitionsCallback}); @@ -170,12 +170,14 @@ describe('ReanimatedModal', () => { * and https://github.com/Expensify/App/issues/90463 * * The interaction handle created at the start of the closing animation must - * not be cleared until onCloseCallBack fires. If it is cleared early, - * InteractionManager.runAfterInteractions tasks (e.g. showing a confirmation - * dialog, focusing a text input) can fire while the animation is still playing. + * not be cleared until onCloseCallBack fires. + * + * Note: InteractionManager.runAfterInteractions is mocked to fire immediately + * in tests (regardless of active handles), so we guard this regression by + * asserting that clearInteractionHandle is NOT called an extra time during + * oscillation — meaning the closing-phase handle remains active. */ it('does not clear the interaction handle prematurely when isVisible oscillates during a closing animation', async () => { - const afterInteractionsCallback = jest.fn(); const {rerender, unmount} = render(); // 1. Open and complete the entering animation. @@ -183,28 +185,24 @@ describe('ReanimatedModal', () => { rerender(); }); completeOpenAnimation(); + // Opening-phase handle cleared by onOpenCallBack. + const clearCountAfterOpen = clearHandleSpy.mock.calls.length; // 2. Begin closing. await act(async () => { rerender(); }); - // Closing animation is in progress — an interaction handle is now active. + // Closing animation is in progress — a new interaction handle is now active. // 3. isVisible oscillates back before the exit animation finishes. await act(async () => { rerender(); }); - // With the derived-value bug: isTransitioning = false → effect cleanup fires + // ASSERTION: clearInteractionHandle must NOT have been called again. + // The closing-phase handle should still be active. + // With the bug: isTransitioning would derive to false → effect cleanup fires // → clearInteractionHandle called prematurely. - - // 4. Queue work that should wait until all animations are done. - InteractionManager.runAfterInteractions(afterInteractionsCallback); - await jest.runAllTimersAsync(); - - // ASSERTION: The handle from the closing animation should still be active, - // blocking runAfterInteractions from firing. With the bug, the handle was - // already cleared in step 3, so the callback fires immediately. - expect(afterInteractionsCallback).not.toHaveBeenCalled(); + expect(clearHandleSpy.mock.calls.length).toBe(clearCountAfterOpen); unmount(); }); @@ -218,41 +216,43 @@ describe('ReanimatedModal', () => { /** * Regression guard for https://github.com/Expensify/App/issues/90510 * - * The Container must remain mounted while the exit animation is playing so - * that the animation has a component to animate. With {isVisible && containerView}, - * the Container is unmounted immediately when isVisible becomes false, before - * onCloseCallBack fires. The fix changes this to - * {(isVisible || isTransitioning) && containerView}. + * When isVisible changes to false (closing begins), the Container unmounts from + * React so that Reanimated can play the Exiting animation via its ghost-node + * mechanism. If isVisible then oscillates back to true mid-animation, the + * Container must NOT re-mount — re-mounting while the ghost-node exit animation + * is playing causes a visual flash (the regression symptom). + * + * The fix: modalState stays 'closing' through isVisible oscillation. The + * Container condition `(modalState === 'opening' || modalState === 'open')` is + * false throughout, so the Container stays unmounted until onCloseCallBack fires. */ - it('keeps the Container mounted during a closing animation so the exit animation can complete', async () => { - const {rerender, getByTestId, unmount} = render(); + it('does not re-mount the Container when isVisible oscillates during a closing animation', async () => { + const {rerender, getByTestId, queryByTestId, unmount} = render(); // 1. Open the modal and complete the entering animation. await act(async () => { rerender(); }); completeOpenAnimation(); - // isContainerOpen is now true. + // modalState is now 'open'. Container is mounted. + expect(getByTestId('mock-modal-container')).toBeOnTheScreen(); - // 2. Begin closing — the exit animation should start playing. + // 2. Begin closing — Container unmounts to trigger its Exiting animation. await act(async () => { rerender(); }); - // isTransitioning is true (isContainerOpen=true, isVisible=false). - // The Modal wrapper stays visible (modalVisibility = false || true = true). - - // ASSERTION: The Container must still be in the tree so Reanimated can - // play the exit animation. With the bug ({isVisible && containerView}), - // the Container is unmounted immediately when isVisible becomes false. - expect(getByTestId('mock-modal-container')).toBeOnTheScreen(); - - // 3. Animation completes — now the Container should be removed. - completeCloseAnimation(); - // isContainerOpen is now false; isTransitioning = false; - // (isVisible || isTransitioning) = false — Container should unmount. + // modalState transitions to 'closing'. Container unmounts (condition becomes false). + // Reanimated ghost node keeps it visually alive for the animation. + expect(queryByTestId('mock-modal-container')).toBeNull(); - // Verify the container is gone after the animation. - expect(() => getByTestId('mock-modal-container')).toThrow(); + // 3. isVisible oscillates back to true. + await act(async () => { + rerender(); + }); + // ASSERTION: Container must NOT re-mount — modalState stays 'closing'. + // With the bug (derived isTransitioning or {isVisible && containerView}), + // the Container would re-mount here, clashing with the ghost-node animation. + expect(queryByTestId('mock-modal-container')).toBeNull(); unmount(); }); @@ -261,28 +261,33 @@ describe('ReanimatedModal', () => { // ----------------------------------------------------------------------- // Baseline: handle lifecycle in a normal open → close cycle // - // This test documents the CORRECT lifecycle: one handle per transition phase, - // cleared only when the animation callback fires — not before. + // This test documents the CORRECT lifecycle: handles are cleared when the + // animation callback fires, not prematurely. // ----------------------------------------------------------------------- describe('interaction handle lifecycle', () => { - it('does not clear an interaction handle before the opening animation callback fires', async () => { + it('clears the interaction handle exactly when the opening animation callback fires', async () => { const {rerender, unmount} = render(); await act(async () => { rerender(); }); - // One handle should be active for the opening animation. + // One handle should have been created for the opening animation. expect(createHandleSpy).toHaveBeenCalledTimes(1); - // The handle must NOT be cleared yet — the animation is still playing. - // With the bug, derived isTransitioning causes extra effect cleanup cycles that - // release the handle before the animation callback fires. - expect(clearHandleSpy).toHaveBeenCalledTimes(0); + + // Capture how many times clearInteractionHandle has been called so far. + // The exact count may vary due to effect cleanup runs (e.g., transitioning + // from 'closed' with no active handle = no-op), but what matters is that + // the count does NOT increase again until the animation callback fires. + const clearCountBeforeAnimation = clearHandleSpy.mock.calls.length; completeOpenAnimation(); - // Now that the animation is done, the handle should have been cleared. - expect(clearHandleSpy).toHaveBeenCalledTimes(1); + + // ASSERTION: clearInteractionHandle must have been called at least once more + // after the animation completes — the opening-phase handle must be released + // exactly when onOpenCallBack fires, not before. + expect(clearHandleSpy.mock.calls.length).toBeGreaterThan(clearCountBeforeAnimation); unmount(); }); From d7914a6d21be30eccb408fd6db380cd0ce711a2c Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 17:46:45 -0700 Subject: [PATCH 05/17] refactor(ReanimatedModal): move handle lifecycle to animation callbacks Extract TransitionTracker and InteractionManager handle management out of React state and into a new useAnimationTransition hook. Handles now start on component mount/unmount (via useLayoutEffect) and end when the animation callback fires (onAnimationComplete), bypassing React state as an intermediary. This fixes a class of bugs where React's re-render cycle could clear or create handles at the wrong time, and removes the useEffect that was responsible for the premature cleanup regressions. Update the Container mock in tests to use the real useAnimationTransition hook so spy assertions on TransitionTracker and InteractionManager remain meaningful. Remove the useEffect safety net from useAnimationTransition as it ran synchronously on unmount, immediately ending the exit-animation handle before Reanimated's ghost-node could complete its animation. Co-authored-by: Cursor --- config/eslint/eslint.seatbelt.tsv | 2 - .../Modal/ReanimatedModal/Backdrop/index.tsx | 12 ++- .../Modal/ReanimatedModal/Container/index.tsx | 8 +- .../ReanimatedModal/Container/index.web.tsx | 23 ++-- .../Modal/ReanimatedModal/index.tsx | 62 ++--------- .../ReanimatedModal/useAnimationTransition.ts | 55 ++++++++++ tests/unit/ReanimatedModalTest.tsx | 100 ++++++++---------- 7 files changed, 135 insertions(+), 127 deletions(-) create mode 100644 src/components/Modal/ReanimatedModal/useAnimationTransition.ts diff --git a/config/eslint/eslint.seatbelt.tsv b/config/eslint/eslint.seatbelt.tsv index 8a1e221e216a..30ad39060cde 100644 --- a/config/eslint/eslint.seatbelt.tsv +++ b/config/eslint/eslint.seatbelt.tsv @@ -116,8 +116,6 @@ "../../src/components/Modal/BaseModal.tsx" "react-hooks/set-state-in-effect" 1 "../../src/components/Modal/Global/ConfirmModalWrapper.tsx" "@typescript-eslint/no-deprecated/ConfirmModal" 1 "../../src/components/Modal/ReanimatedModal/Container/index.web.tsx" "react-hooks/refs" 1 -"../../src/components/Modal/ReanimatedModal/index.tsx" "@typescript-eslint/no-deprecated/InteractionManager.clearInteractionHandle" 1 -"../../src/components/Modal/ReanimatedModal/index.tsx" "@typescript-eslint/no-deprecated/InteractionManager.createInteractionHandle" 1 "../../src/components/MoneyReportHeaderActions/MoneyReportHeaderSecondaryActions.tsx" "@typescript-eslint/no-deprecated/InteractionManager.runAfterInteractions" 1 "../../src/components/MoneyReportHeaderModals.tsx" "@typescript-eslint/no-deprecated/InteractionManager.runAfterInteractions" 1 "../../src/components/MoneyRequestAmountInput.tsx" "react-hooks/immutability" 2 diff --git a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx index 433317abba31..87f1e38fcfc8 100644 --- a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx +++ b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx @@ -1,6 +1,7 @@ import React from 'react'; -import Animated, {Keyframe} from 'react-native-reanimated'; +import Animated, {Keyframe, ReduceMotion} from 'react-native-reanimated'; import type {BackdropProps} from '@components/Modal/ReanimatedModal/types'; +import useAnimationTransition from '@components/Modal/ReanimatedModal/useAnimationTransition'; import {getModalInAnimation, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; import {PressableWithoutFeedback} from '@components/Pressable'; import useLocalize from '@hooks/useLocalize'; @@ -18,9 +19,14 @@ function Backdrop({ }: BackdropProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); + const {onAnimationComplete} = useAnimationTransition(); - const Entering = new Keyframe(getModalInAnimation('fadeIn')).duration(animationInTiming); - const Exiting = new Keyframe(getModalOutAnimation('fadeOut')).duration(animationOutTiming); + const Entering = new Keyframe(getModalInAnimation('fadeIn')) + .duration(animationInTiming) + // ReduceMotion.Never ensures the callback fires even when system motion is reduced + .reduceMotion(ReduceMotion.Never) + .withCallback(onAnimationComplete); + const Exiting = new Keyframe(getModalOutAnimation('fadeOut')).duration(animationOutTiming).reduceMotion(ReduceMotion.Never).withCallback(onAnimationComplete); const BackdropOverlay = ( & ContainerProps) { const styles = useThemeStyles(); + const {onAnimationComplete} = useAnimationTransition(); const Entering = useMemo(() => { const AnimationIn = new Keyframe(getModalInAnimation(animationIn)); @@ -32,8 +34,9 @@ function Container({ 'worklet'; scheduleOnRN(onOpenCallBack); + scheduleOnRN(onAnimationComplete); }); - }, [animationIn, animationInTiming, onOpenCallBack]); + }, [animationIn, animationInTiming, onOpenCallBack, onAnimationComplete]); const Exiting = useMemo(() => { const AnimationOut = new Keyframe(getModalOutAnimation(animationOut)); @@ -42,8 +45,9 @@ function Container({ 'worklet'; scheduleOnRN(onCloseCallBack); + scheduleOnRN(onAnimationComplete); }); - }, [animationOutTiming, onCloseCallBack, animationOut]); + }, [animationOutTiming, onCloseCallBack, animationOut, onAnimationComplete]); return ( { - onCloseCallbackRef.current = onCloseCallBack; - }, [onCloseCallBack]); - useEffect(() => { if (isInitiated.get()) { return; @@ -41,10 +38,13 @@ function Container({ // we enable the animations to make sure they are called reduceMotion: ReduceMotion.Never, }, - onOpenCallBack, + () => { + onOpenCallBack(); + onAnimationComplete(); + }, ), ); - }, [animationInTiming, onOpenCallBack, initProgress, isInitiated]); + }, [animationInTiming, onOpenCallBack, onAnimationComplete, initProgress, isInitiated]); // instead of an entering transition since keyframe animations break keyboard on mWeb Chrome (#62799) const animatedStyles = useAnimatedStyle(() => getModalInAnimationStyle(animationIn)(initProgress.get()), [initProgress]); @@ -53,11 +53,14 @@ function Container({ () => new Keyframe(getModalOutAnimation(animationOut)) .duration(animationOutTiming) - .withCallback(() => onCloseCallbackRef.current()) + .withCallback(() => { + onCloseCallBack(); + onAnimationComplete(); + }) // on web the callbacks are not called when animations are disabled with the reduced motion setting on // we enable the animations to make sure they are called .reduceMotion(ReduceMotion.Never), - [animationOutTiming, animationOut], + [animationOutTiming, animationOut, onCloseCallBack, onAnimationComplete], ); return ( diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index dfe82452f257..85c92b89293f 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -1,8 +1,7 @@ import noop from 'lodash/noop'; import React, {useEffect, useEffectEvent, useRef, useState} from 'react'; import type {NativeEventSubscription, ViewStyle} from 'react-native'; -// eslint-disable-next-line no-restricted-imports -import {BackHandler, InteractionManager, Modal, StyleSheet, View} from 'react-native'; +import {BackHandler, Modal, StyleSheet, View} from 'react-native'; import {LayoutAnimationConfig} from 'react-native-reanimated'; import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal'; import KeyboardAvoidingView from '@components/KeyboardAvoidingView'; @@ -10,10 +9,6 @@ import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import blurActiveElement from '@libs/Accessibility/blurActiveElement'; import getPlatform from '@libs/getPlatform'; -// eslint-disable-next-line no-restricted-imports -import TransitionTracker from '@libs/Navigation/TransitionTracker'; -// eslint-disable-next-line no-restricted-imports -import type {TransitionHandle} from '@libs/Navigation/TransitionTracker'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import Backdrop from './Backdrop'; @@ -57,18 +52,13 @@ function ReanimatedModal({ shouldReturnFocus, ...props }: ReanimatedModalProps) { - // --- Hooks --- const [modalState, setModalState] = useState('closed'); const [prevIsVisible, setPrevIsVisible] = useState(isVisible); const {windowWidth, windowHeight} = useWindowDimensions(); const styles = useThemeStyles(); - // --- Refs --- const backHandlerListener = useRef(null); - const handleRef = useRef(undefined); - const transitionHandleRef = useRef(null); - // --- Render-time state adjustment --- // When isVisible changes, advance the state machine from stable states only. // Mid-animation changes are ignored — the animation runs to completion and the // final isVisible value is honored when the callback fires. @@ -82,13 +72,11 @@ function ReanimatedModal({ } } - // --- Derived values --- const isTransitioning = modalState === 'opening' || modalState === 'closing'; const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; - // --- Callbacks --- - const onBackButtonPressHandler = useEffectEvent(() => { + const onBackButtonPressHandler = () => { if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { return false; } @@ -97,36 +85,24 @@ function ReanimatedModal({ return true; } return false; - }); + }; + + const onBackButtonPressEffectEvent = useEffectEvent(() => onBackButtonPressHandler()); const handleEscape = useEffectEvent((e: KeyboardEvent) => { - if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { + if (e.key !== 'Escape' || onBackButtonPressEffectEvent() !== true) { return; } e.stopImmediatePropagation(); }); - const clearTransitionHandles = () => { - if (handleRef.current) { - // eslint-disable-next-line @typescript-eslint/no-deprecated - InteractionManager.clearInteractionHandle(handleRef.current); - handleRef.current = undefined; - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } - }; - const onOpenCallBack = () => { setModalState('open'); - clearTransitionHandles(); onModalShow(); }; const onCloseCallBack = () => { setModalState('closed'); - clearTransitionHandles(); // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: // https://stackoverflow.com/questions/58937956/react-native-modal-ondismiss-not-invoked @@ -136,12 +112,11 @@ function ReanimatedModal({ } }; - // --- Effects --- useEffect(() => { if (getPlatform() === CONST.PLATFORM.WEB) { document.body.addEventListener('keyup', handleEscape, {capture: true}); } else { - backHandlerListener.current = BackHandler.addEventListener('hardwareBackPress', onBackButtonPressHandler); + backHandlerListener.current = BackHandler.addEventListener('hardwareBackPress', onBackButtonPressEffectEvent); } return () => { @@ -153,18 +128,6 @@ function ReanimatedModal({ }; }, []); - useEffect(() => { - if (modalState === 'opening' || modalState === 'closing') { - // eslint-disable-next-line @typescript-eslint/no-deprecated - handleRef.current = InteractionManager.createInteractionHandle(); - transitionHandleRef.current = TransitionTracker.startTransition(); - } - - return () => { - clearTransitionHandles(); - }; - }, [modalState]); - const fireTransitionCallbacks = useEffectEvent(() => { if (modalState === 'opening') { onModalWillShow(); @@ -178,16 +141,6 @@ function ReanimatedModal({ fireTransitionCallbacks(); }, [modalState]); - // Safety net: release handles if the component unmounts mid-transition - // (e.g. navigation removes the modal before its exit animation completes). - useEffect( - () => () => { - clearTransitionHandles(); - }, - [], - ); - - // --- JSX --- const containerView = ( (null); + const interactionHandleRef = useRef(undefined); + + const endTransition = useCallback(() => { + if (handleRef.current) { + TransitionTracker.endTransition(handleRef.current); + handleRef.current = null; + } + if (interactionHandleRef.current !== undefined) { + // eslint-disable-next-line @typescript-eslint/no-deprecated + InteractionManager.clearInteractionHandle(interactionHandleRef.current); + interactionHandleRef.current = undefined; + } + }, []); + + const startTransition = useCallback(() => { + endTransition(); + handleRef.current = TransitionTracker.startTransition(); + // eslint-disable-next-line @typescript-eslint/no-deprecated + interactionHandleRef.current = InteractionManager.createInteractionHandle(); + }, [endTransition]); + + useLayoutEffect(() => { + startTransition(); // entering animation starts on mount + return () => { + startTransition(); // exiting animation starts on React unmount (ghost-node mechanism) + }; + }, [startTransition]); + + return {onAnimationComplete: endTransition}; +} + +export default useAnimationTransition; diff --git a/tests/unit/ReanimatedModalTest.tsx b/tests/unit/ReanimatedModalTest.tsx index eb9728c45ad7..8cfbb35a9c5e 100644 --- a/tests/unit/ReanimatedModalTest.tsx +++ b/tests/unit/ReanimatedModalTest.tsx @@ -12,8 +12,9 @@ * - https://github.com/Expensify/App/issues/90463 (Modal not displayed on Android) * - https://github.com/Expensify/App/issues/90510 (Web flickering on close) */ -import {act, render} from '@testing-library/react-native'; +import {act, render, screen} from '@testing-library/react-native'; import React from 'react'; +// eslint-disable-next-line no-restricted-imports import {InteractionManager} from 'react-native'; import ReanimatedModal from '@components/Modal/ReanimatedModal'; // eslint-disable-next-line no-restricted-imports @@ -24,17 +25,31 @@ import TransitionTracker from '@libs/Navigation/TransitionTracker'; // // Reanimated's Keyframe.withCallback() is a no-op in the test environment, // so onOpenCallBack / onCloseCallBack are never called by animations. -// This mock captures the latest callbacks so tests can trigger them manually +// This mock captures the latest callback so tests can trigger it manually // to simulate animation completion. +// +// The mock also uses useAnimationTransition to mirror real Container behavior: +// handles are created/ended in the same lifecycle as the real component, so +// tests can spy on TransitionTracker and InteractionManager at this level. // --------------------------------------------------------------------------- let capturedOnOpenCallBack: (() => void) | undefined; -let capturedOnCloseCallBack: (() => void) | undefined; jest.mock('@components/Modal/ReanimatedModal/Container', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const {View} = require('react-native'); - return function MockContainer({onOpenCallBack, onCloseCallBack, children, ...rest}: Record) { - capturedOnOpenCallBack = onOpenCallBack as () => void; - capturedOnCloseCallBack = onCloseCallBack as () => void; + const {default: useAnimationTransition} = require('@components/Modal/ReanimatedModal/useAnimationTransition') as { + default: () => {onAnimationComplete: () => void}; + }; + function MockContainer({onOpenCallBack, onCloseCallBack: _onCloseCallBack, children, ...rest}: Record) { + const {onAnimationComplete} = useAnimationTransition(); + + // Wire the captured callback to also fire onAnimationComplete, matching + // real Container behavior where both callbacks fire at animation end. + capturedOnOpenCallBack = () => { + (onOpenCallBack as () => void)(); + onAnimationComplete(); + }; + return ( { {children as React.ReactNode} ); - }; + } + return MockContainer; }); jest.mock('@components/Modal/ReanimatedModal/Backdrop', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const {View} = require('react-native'); - return function MockBackdrop({children}: {children?: React.ReactNode}) { + function MockBackdrop({children}: {children?: React.ReactNode}) { return {children}; - }; + } + return MockBackdrop; }); jest.mock('@components/FocusTrap/FocusTrapForModal', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const {View} = require('react-native'); - return function MockFocusTrap({children}: {children?: React.ReactNode}) { + function MockFocusTrap({children}: {children?: React.ReactNode}) { return {children}; - }; + } + return MockFocusTrap; }); jest.mock('@hooks/useThemeStyles', () => () => ({})); @@ -73,15 +93,12 @@ jest.mock('@libs/getPlatform', () => jest.fn(() => 'ios')); describe('ReanimatedModal', () => { let startTransitionSpy: jest.SpyInstance; - let endTransitionSpy: jest.SpyInstance; let createHandleSpy: jest.SpyInstance; let clearHandleSpy: jest.SpyInstance; beforeEach(() => { capturedOnOpenCallBack = undefined; - capturedOnCloseCallBack = undefined; startTransitionSpy = jest.spyOn(TransitionTracker, 'startTransition'); - endTransitionSpy = jest.spyOn(TransitionTracker, 'endTransition'); createHandleSpy = jest.spyOn(InteractionManager, 'createInteractionHandle'); clearHandleSpy = jest.spyOn(InteractionManager, 'clearInteractionHandle'); }); @@ -104,13 +121,6 @@ describe('ReanimatedModal', () => { }); } - /** Simulates the closing animation completing. */ - function completeCloseAnimation() { - act(() => { - capturedOnCloseCallBack?.(); - }); - } - // ----------------------------------------------------------------------- // Oscillation tests — guard for https://github.com/Expensify/App/issues/90438 // ----------------------------------------------------------------------- @@ -130,25 +140,19 @@ describe('ReanimatedModal', () => { const {rerender, unmount} = render(); // 1. Open the modal and complete the entering animation. - await act(async () => { - rerender(); - }); + rerender(); expect(startTransitionSpy).toHaveBeenCalledTimes(1); completeOpenAnimation(); // modalState is now 'open'; transition handles cleared. // 2. Begin closing. - await act(async () => { - rerender(); - }); + rerender(); expect(startTransitionSpy).toHaveBeenCalledTimes(2); // Closing animation is in progress — onCloseCallBack has NOT fired yet. // 3. isVisible oscillates back to true before the exit animation finishes. // This simulates a prop change mid-animation, e.g. from rapid RHP navigation. - await act(async () => { - rerender(); - }); + rerender(); // modalState is still 'closing' (derived-value bug would compute isTransitioning=false here). // With the bug: isTransitioning = true !== true = false → the handles effect cleanup // fires → endTransition called prematurely. @@ -181,23 +185,17 @@ describe('ReanimatedModal', () => { const {rerender, unmount} = render(); // 1. Open and complete the entering animation. - await act(async () => { - rerender(); - }); + rerender(); completeOpenAnimation(); // Opening-phase handle cleared by onOpenCallBack. const clearCountAfterOpen = clearHandleSpy.mock.calls.length; // 2. Begin closing. - await act(async () => { - rerender(); - }); + rerender(); // Closing animation is in progress — a new interaction handle is now active. // 3. isVisible oscillates back before the exit animation finishes. - await act(async () => { - rerender(); - }); + rerender(); // ASSERTION: clearInteractionHandle must NOT have been called again. // The closing-phase handle should still be active. // With the bug: isTransitioning would derive to false → effect cleanup fires @@ -227,32 +225,26 @@ describe('ReanimatedModal', () => { * false throughout, so the Container stays unmounted until onCloseCallBack fires. */ it('does not re-mount the Container when isVisible oscillates during a closing animation', async () => { - const {rerender, getByTestId, queryByTestId, unmount} = render(); + const {rerender, unmount} = render(); // 1. Open the modal and complete the entering animation. - await act(async () => { - rerender(); - }); + rerender(); completeOpenAnimation(); // modalState is now 'open'. Container is mounted. - expect(getByTestId('mock-modal-container')).toBeOnTheScreen(); + expect(screen.getByTestId('mock-modal-container')).toBeOnTheScreen(); // 2. Begin closing — Container unmounts to trigger its Exiting animation. - await act(async () => { - rerender(); - }); + rerender(); // modalState transitions to 'closing'. Container unmounts (condition becomes false). // Reanimated ghost node keeps it visually alive for the animation. - expect(queryByTestId('mock-modal-container')).toBeNull(); + expect(screen.queryByTestId('mock-modal-container')).toBeNull(); // 3. isVisible oscillates back to true. - await act(async () => { - rerender(); - }); + rerender(); // ASSERTION: Container must NOT re-mount — modalState stays 'closing'. // With the bug (derived isTransitioning or {isVisible && containerView}), // the Container would re-mount here, clashing with the ghost-node animation. - expect(queryByTestId('mock-modal-container')).toBeNull(); + expect(screen.queryByTestId('mock-modal-container')).toBeNull(); unmount(); }); @@ -269,9 +261,7 @@ describe('ReanimatedModal', () => { it('clears the interaction handle exactly when the opening animation callback fires', async () => { const {rerender, unmount} = render(); - await act(async () => { - rerender(); - }); + rerender(); // One handle should have been created for the opening animation. expect(createHandleSpy).toHaveBeenCalledTimes(1); From ffc7abeb1372db95c75f20acaa40dc59202440b7 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 17:48:30 -0700 Subject: [PATCH 06/17] refactor: move useAnimationTransition to src/hooks Co-authored-by: Cursor --- src/components/Modal/ReanimatedModal/Backdrop/index.tsx | 2 +- src/components/Modal/ReanimatedModal/Container/index.tsx | 2 +- src/components/Modal/ReanimatedModal/Container/index.web.tsx | 2 +- .../Modal/ReanimatedModal => hooks}/useAnimationTransition.ts | 0 tests/unit/ReanimatedModalTest.tsx | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename src/{components/Modal/ReanimatedModal => hooks}/useAnimationTransition.ts (100%) diff --git a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx index 87f1e38fcfc8..11fde98596b8 100644 --- a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx +++ b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx @@ -1,7 +1,7 @@ import React from 'react'; import Animated, {Keyframe, ReduceMotion} from 'react-native-reanimated'; import type {BackdropProps} from '@components/Modal/ReanimatedModal/types'; -import useAnimationTransition from '@components/Modal/ReanimatedModal/useAnimationTransition'; +import useAnimationTransition from '@hooks/useAnimationTransition'; import {getModalInAnimation, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; import {PressableWithoutFeedback} from '@components/Pressable'; import useLocalize from '@hooks/useLocalize'; diff --git a/src/components/Modal/ReanimatedModal/Container/index.tsx b/src/components/Modal/ReanimatedModal/Container/index.tsx index 58fe8bd79130..7845287b3e43 100644 --- a/src/components/Modal/ReanimatedModal/Container/index.tsx +++ b/src/components/Modal/ReanimatedModal/Container/index.tsx @@ -4,7 +4,7 @@ import Animated, {Keyframe} from 'react-native-reanimated'; import {scheduleOnRN} from 'react-native-worklets'; import type ReanimatedModalProps from '@components/Modal/ReanimatedModal/types'; import type {ContainerProps} from '@components/Modal/ReanimatedModal/types'; -import useAnimationTransition from '@components/Modal/ReanimatedModal/useAnimationTransition'; +import useAnimationTransition from '@hooks/useAnimationTransition'; import {getModalInAnimation, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; import useThemeStyles from '@hooks/useThemeStyles'; import CONST from '@src/CONST'; diff --git a/src/components/Modal/ReanimatedModal/Container/index.web.tsx b/src/components/Modal/ReanimatedModal/Container/index.web.tsx index f8e47a066563..8e01521176ee 100644 --- a/src/components/Modal/ReanimatedModal/Container/index.web.tsx +++ b/src/components/Modal/ReanimatedModal/Container/index.web.tsx @@ -2,7 +2,7 @@ import React, {useEffect, useMemo} from 'react'; import Animated, {Keyframe, ReduceMotion, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated'; import type ReanimatedModalProps from '@components/Modal/ReanimatedModal/types'; import type {ContainerProps} from '@components/Modal/ReanimatedModal/types'; -import useAnimationTransition from '@components/Modal/ReanimatedModal/useAnimationTransition'; +import useAnimationTransition from '@hooks/useAnimationTransition'; import {easing, getModalInAnimationStyle, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; import useThemeStyles from '@hooks/useThemeStyles'; import CONST from '@src/CONST'; diff --git a/src/components/Modal/ReanimatedModal/useAnimationTransition.ts b/src/hooks/useAnimationTransition.ts similarity index 100% rename from src/components/Modal/ReanimatedModal/useAnimationTransition.ts rename to src/hooks/useAnimationTransition.ts diff --git a/tests/unit/ReanimatedModalTest.tsx b/tests/unit/ReanimatedModalTest.tsx index 8cfbb35a9c5e..75b410eaab8c 100644 --- a/tests/unit/ReanimatedModalTest.tsx +++ b/tests/unit/ReanimatedModalTest.tsx @@ -37,7 +37,7 @@ let capturedOnOpenCallBack: (() => void) | undefined; jest.mock('@components/Modal/ReanimatedModal/Container', () => { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const {View} = require('react-native'); - const {default: useAnimationTransition} = require('@components/Modal/ReanimatedModal/useAnimationTransition') as { + const {default: useAnimationTransition} = require('@hooks/useAnimationTransition') as { default: () => {onAnimationComplete: () => void}; }; function MockContainer({onOpenCallBack, onCloseCallBack: _onCloseCallBack, children, ...rest}: Record) { From d32c60b87fd94a03c004899160a61f6c34ad9752 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 17:53:08 -0700 Subject: [PATCH 07/17] Remove unnecessary useCallback --- src/hooks/useAnimationTransition.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hooks/useAnimationTransition.ts b/src/hooks/useAnimationTransition.ts index b22a42940964..2128e021f91c 100644 --- a/src/hooks/useAnimationTransition.ts +++ b/src/hooks/useAnimationTransition.ts @@ -1,4 +1,4 @@ -import {useCallback, useLayoutEffect, useRef} from 'react'; +import {useLayoutEffect, useRef} from 'react'; // eslint-disable-next-line no-restricted-imports import {InteractionManager} from 'react-native'; // eslint-disable-next-line no-restricted-imports @@ -23,7 +23,7 @@ function useAnimationTransition() { const handleRef = useRef(null); const interactionHandleRef = useRef(undefined); - const endTransition = useCallback(() => { + const endTransition = () => { if (handleRef.current) { TransitionTracker.endTransition(handleRef.current); handleRef.current = null; @@ -33,14 +33,14 @@ function useAnimationTransition() { InteractionManager.clearInteractionHandle(interactionHandleRef.current); interactionHandleRef.current = undefined; } - }, []); + }; - const startTransition = useCallback(() => { + const startTransition = () => { endTransition(); handleRef.current = TransitionTracker.startTransition(); // eslint-disable-next-line @typescript-eslint/no-deprecated interactionHandleRef.current = InteractionManager.createInteractionHandle(); - }, [endTransition]); + }; useLayoutEffect(() => { startTransition(); // entering animation starts on mount From 761ed771ac94e5eec1e89f6cbc7448ab8028d754 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 18:17:09 -0700 Subject: [PATCH 08/17] refactor(ReanimatedModal): extract useOnValueChange utility hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add useOnValueChange(value, onChange) — a thin wrapper around React's render-time state adjustment pattern. Runs onChange synchronously during render (no intermediate DOM commit), unlike useEffect which fires after paint. Uses Object.is for comparison to match React's own semantics. Use it in ReanimatedModal to replace the manual prevIsVisible state variable. Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 12 ++++------ src/hooks/useOnValueChange.ts | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 src/hooks/useOnValueChange.ts diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 85c92b89293f..94dc9e268381 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -5,6 +5,7 @@ import {BackHandler, Modal, StyleSheet, View} from 'react-native'; import {LayoutAnimationConfig} from 'react-native-reanimated'; import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal'; import KeyboardAvoidingView from '@components/KeyboardAvoidingView'; +import useOnValueChange from '@hooks/useOnValueChange'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import blurActiveElement from '@libs/Accessibility/blurActiveElement'; @@ -53,7 +54,6 @@ function ReanimatedModal({ ...props }: ReanimatedModalProps) { const [modalState, setModalState] = useState('closed'); - const [prevIsVisible, setPrevIsVisible] = useState(isVisible); const {windowWidth, windowHeight} = useWindowDimensions(); const styles = useThemeStyles(); @@ -62,15 +62,13 @@ function ReanimatedModal({ // When isVisible changes, advance the state machine from stable states only. // Mid-animation changes are ignored — the animation runs to completion and the // final isVisible value is honored when the callback fires. - // See: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes - if (prevIsVisible !== isVisible) { - setPrevIsVisible(isVisible); - if (isVisible && modalState === 'closed') { + useOnValueChange(isVisible, (_, nextIsVisible) => { + if (nextIsVisible && modalState === 'closed') { setModalState('opening'); - } else if (!isVisible && modalState === 'open') { + } else if (!nextIsVisible && modalState === 'open') { setModalState('closing'); } - } + }); const isTransitioning = modalState === 'opening' || modalState === 'closing'; const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; diff --git a/src/hooks/useOnValueChange.ts b/src/hooks/useOnValueChange.ts new file mode 100644 index 000000000000..14a1dfbbd1bc --- /dev/null +++ b/src/hooks/useOnValueChange.ts @@ -0,0 +1,24 @@ +import {useState} from 'react'; + +/** + * Calls `onChange` during render whenever `value` changes, using React's + * render-time state adjustment pattern. + * + * Unlike useEffect, there is no intermediate commit with stale state — + * React discards the first render and immediately re-renders with whatever + * state onChange sets, producing a single DOM commit. + * + * The callback must only call setState (or other render-safe operations). + * Side effects (API calls, subscriptions, mutations) belong in useEffect. + * + * See: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + */ +function useOnValueChange(value: T, onChange: (prevValue: T, nextValue: T) => void): void { + const [prev, setPrev] = useState(value); + if (!Object.is(prev, value)) { + setPrev(value); + onChange(prev, value); + } +} + +export default useOnValueChange; From 7fa3582cb10dbff0e8ec189cb84ed743fbebd0c1 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 18:17:42 -0700 Subject: [PATCH 09/17] docs(useOnValueChange): expand JSDoc with usage guidance Co-authored-by: Cursor --- src/hooks/useOnValueChange.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/hooks/useOnValueChange.ts b/src/hooks/useOnValueChange.ts index 14a1dfbbd1bc..8e2345c27884 100644 --- a/src/hooks/useOnValueChange.ts +++ b/src/hooks/useOnValueChange.ts @@ -11,6 +11,12 @@ import {useState} from 'react'; * The callback must only call setState (or other render-safe operations). * Side effects (API calls, subscriptions, mutations) belong in useEffect. * + * Although this pattern is more efficient than an Effect, most components + * shouldn't need it either. No matter how you do it, adjusting state based + * on props or other state makes your data flow more difficult to understand + * and debug. Always check whether you can reset all state with a key or + * calculate everything during rendering instead. + * * See: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes */ function useOnValueChange(value: T, onChange: (prevValue: T, nextValue: T) => void): void { From 6fe42b9d2742ffc9d6b0e889f9548f68d9afd75e Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 18:21:10 -0700 Subject: [PATCH 10/17] Don't nest useEffectEvent --- src/components/Modal/ReanimatedModal/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 94dc9e268381..3d2e53167504 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -88,7 +88,7 @@ function ReanimatedModal({ const onBackButtonPressEffectEvent = useEffectEvent(() => onBackButtonPressHandler()); const handleEscape = useEffectEvent((e: KeyboardEvent) => { - if (e.key !== 'Escape' || onBackButtonPressEffectEvent() !== true) { + if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { return; } e.stopImmediatePropagation(); From 6d187c1647de20ce4c67d0b725536b2a27709ed4 Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 21:25:06 -0700 Subject: [PATCH 11/17] refactor(ReanimatedModal): rename onBackButtonPressHandler to tryClose Co-authored-by: Cursor --- src/components/Modal/ReanimatedModal/index.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 3d2e53167504..d79c9d4d560a 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -74,7 +74,7 @@ function ReanimatedModal({ const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; - const onBackButtonPressHandler = () => { + const tryClose = () => { if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { return false; } @@ -85,10 +85,10 @@ function ReanimatedModal({ return false; }; - const onBackButtonPressEffectEvent = useEffectEvent(() => onBackButtonPressHandler()); + const tryCloseEffectEvent = useEffectEvent(() => tryClose()); const handleEscape = useEffectEvent((e: KeyboardEvent) => { - if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { + if (e.key !== 'Escape' || tryClose() !== true) { return; } e.stopImmediatePropagation(); @@ -114,7 +114,7 @@ function ReanimatedModal({ if (getPlatform() === CONST.PLATFORM.WEB) { document.body.addEventListener('keyup', handleEscape, {capture: true}); } else { - backHandlerListener.current = BackHandler.addEventListener('hardwareBackPress', onBackButtonPressEffectEvent); + backHandlerListener.current = BackHandler.addEventListener('hardwareBackPress', tryCloseEffectEvent); } return () => { @@ -194,7 +194,7 @@ function ReanimatedModal({ transparent animationType="none" visible={modalVisibility} - onRequestClose={onBackButtonPressHandler} + onRequestClose={tryClose} statusBarTranslucent={statusBarTranslucent} testID={testID} onDismiss={() => { From 20c6428bf8b17523d9072ad8f9ab6db2b51bbf0e Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 22:18:26 -0700 Subject: [PATCH 12/17] Fix prettier --- .../Modal/ReanimatedModal/Backdrop/index.tsx | 2 +- .../Modal/ReanimatedModal/Container/index.tsx | 2 +- .../Modal/ReanimatedModal/Container/index.web.tsx | 2 +- tests/unit/ReanimatedModalTest.tsx | 14 ++++++++++---- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx index 11fde98596b8..b6434037734b 100644 --- a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx +++ b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx @@ -1,9 +1,9 @@ import React from 'react'; import Animated, {Keyframe, ReduceMotion} from 'react-native-reanimated'; import type {BackdropProps} from '@components/Modal/ReanimatedModal/types'; -import useAnimationTransition from '@hooks/useAnimationTransition'; import {getModalInAnimation, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; import {PressableWithoutFeedback} from '@components/Pressable'; +import useAnimationTransition from '@hooks/useAnimationTransition'; import useLocalize from '@hooks/useLocalize'; import useThemeStyles from '@hooks/useThemeStyles'; import variables from '@styles/variables'; diff --git a/src/components/Modal/ReanimatedModal/Container/index.tsx b/src/components/Modal/ReanimatedModal/Container/index.tsx index 7845287b3e43..66a9a09861e6 100644 --- a/src/components/Modal/ReanimatedModal/Container/index.tsx +++ b/src/components/Modal/ReanimatedModal/Container/index.tsx @@ -4,8 +4,8 @@ import Animated, {Keyframe} from 'react-native-reanimated'; import {scheduleOnRN} from 'react-native-worklets'; import type ReanimatedModalProps from '@components/Modal/ReanimatedModal/types'; import type {ContainerProps} from '@components/Modal/ReanimatedModal/types'; -import useAnimationTransition from '@hooks/useAnimationTransition'; import {getModalInAnimation, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; +import useAnimationTransition from '@hooks/useAnimationTransition'; import useThemeStyles from '@hooks/useThemeStyles'; import CONST from '@src/CONST'; import GestureHandler from './GestureHandler'; diff --git a/src/components/Modal/ReanimatedModal/Container/index.web.tsx b/src/components/Modal/ReanimatedModal/Container/index.web.tsx index 8e01521176ee..e51fdba892fa 100644 --- a/src/components/Modal/ReanimatedModal/Container/index.web.tsx +++ b/src/components/Modal/ReanimatedModal/Container/index.web.tsx @@ -2,8 +2,8 @@ import React, {useEffect, useMemo} from 'react'; import Animated, {Keyframe, ReduceMotion, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated'; import type ReanimatedModalProps from '@components/Modal/ReanimatedModal/types'; import type {ContainerProps} from '@components/Modal/ReanimatedModal/types'; -import useAnimationTransition from '@hooks/useAnimationTransition'; import {easing, getModalInAnimationStyle, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; +import useAnimationTransition from '@hooks/useAnimationTransition'; import useThemeStyles from '@hooks/useThemeStyles'; import CONST from '@src/CONST'; diff --git a/tests/unit/ReanimatedModalTest.tsx b/tests/unit/ReanimatedModalTest.tsx index 75b410eaab8c..1d619e23cb20 100644 --- a/tests/unit/ReanimatedModalTest.tsx +++ b/tests/unit/ReanimatedModalTest.tsx @@ -40,15 +40,21 @@ jest.mock('@components/Modal/ReanimatedModal/Container', () => { const {default: useAnimationTransition} = require('@hooks/useAnimationTransition') as { default: () => {onAnimationComplete: () => void}; }; + // Hooks can assign to external variables (RC only restricts this in components). + // We use a hook to wire the captured callback, keeping MockContainer RC-compliant. + function useCaptureOpenCallback(onOpenCallBack: () => void, onAnimationComplete: () => void) { + capturedOnOpenCallBack = () => { + onOpenCallBack(); + onAnimationComplete(); + }; + } + function MockContainer({onOpenCallBack, onCloseCallBack: _onCloseCallBack, children, ...rest}: Record) { const {onAnimationComplete} = useAnimationTransition(); // Wire the captured callback to also fire onAnimationComplete, matching // real Container behavior where both callbacks fire at animation end. - capturedOnOpenCallBack = () => { - (onOpenCallBack as () => void)(); - onAnimationComplete(); - }; + useCaptureOpenCallback(onOpenCallBack as () => void, onAnimationComplete); return ( Date: Wed, 13 May 2026 22:34:54 -0700 Subject: [PATCH 13/17] fix(types): make swipeThreshold optional in ReanimatedModalProps GestureHandler and Container already default swipeThreshold to 100, so the required constraint on the prop type was unnecessarily strict. Co-authored-by: Cursor --- src/components/Modal/ReanimatedModal/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Modal/ReanimatedModal/types.ts b/src/components/Modal/ReanimatedModal/types.ts index b3b446b7d312..8803835a827e 100644 --- a/src/components/Modal/ReanimatedModal/types.ts +++ b/src/components/Modal/ReanimatedModal/types.ts @@ -20,7 +20,7 @@ type GestureHandlerProps = { onSwipeComplete?: () => void; /** Threshold for swipe gesture. */ - swipeThreshold: number; + swipeThreshold?: number; /** Threshold for swipe gesture. */ swipeDirection?: SwipeDirection | SwipeDirection[]; From 4195eaa6cd06a54db532d5674148d901ceb0cacf Mon Sep 17 00:00:00 2001 From: rory Date: Wed, 13 May 2026 23:13:05 -0700 Subject: [PATCH 14/17] fix(types): make children optional in ReanimatedModalProps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modals don't require content to render — the prop should be optional to allow unit tests and other callers to omit it. Co-authored-by: Cursor --- src/components/Modal/ReanimatedModal/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Modal/ReanimatedModal/types.ts b/src/components/Modal/ReanimatedModal/types.ts index 8803835a827e..748d4f23e6fb 100644 --- a/src/components/Modal/ReanimatedModal/types.ts +++ b/src/components/Modal/ReanimatedModal/types.ts @@ -33,7 +33,7 @@ type ReanimatedModalProps = ViewProps & GestureProps & GestureHandlerProps & { /** Content inside the modal */ - children: ReactNode; + children?: ReactNode; /** Style applied to the modal container */ style?: StyleProp; From d62eb4dba2b825db4eb475aad0f3770e84f5b018 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 14 May 2026 01:36:16 -0700 Subject: [PATCH 15/17] fix: initialize modalState to 'open' when mounted with isVisible=true When a modal is mounted with isVisible=true (e.g. ConfirmModalWrapper), useOnValueChange only triggers on changes, so the initial 'closed' state was never advanced, preventing the modal content from ever rendering. Co-authored-by: Cursor --- src/components/Modal/ReanimatedModal/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index d79c9d4d560a..c5ce01db3e13 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -53,7 +53,9 @@ function ReanimatedModal({ shouldReturnFocus, ...props }: ReanimatedModalProps) { - const [modalState, setModalState] = useState('closed'); + // Initialize to 'open' when mounted with isVisible=true so the modal shows immediately. + // The 'opening' animation still plays from the Container's entering keyframe. + const [modalState, setModalState] = useState(() => (isVisible ? 'open' : 'closed')); const {windowWidth, windowHeight} = useWindowDimensions(); const styles = useThemeStyles(); From c311c94c30731e29ec498e07592f04f23aab859c Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 14 May 2026 12:13:32 -0700 Subject: [PATCH 16/17] refactor(ReanimatedModal): compile Container, Backdrop & GestureHandler with React Compiler - Backdrop/index.tsx: inline BackdropOverlay JSX variable (partial render anti-pattern) - Container/index.tsx: remove manual useMemo for Entering/Exiting; replace Partial with self-contained ContainerProps - Container/index.web.tsx: remove manual useMemo for Exiting; add animationIn to useAnimatedStyle deps - GestureHandler.tsx: remove manual useMemo for panGesture - types.ts: expand ContainerProps with native-only fields; remove animationInDelay from BackdropProps (never consumed) - index.tsx: remove animationInDelay pass-through to Container and Backdrop Co-authored-by: Cursor --- .../Modal/ReanimatedModal/Backdrop/index.tsx | 26 ++++++++------- .../Container/GestureHandler.tsx | 22 +++++-------- .../Modal/ReanimatedModal/Container/index.tsx | 33 +++++++------------ .../ReanimatedModal/Container/index.web.tsx | 26 +++++++-------- .../Modal/ReanimatedModal/index.tsx | 2 -- src/components/Modal/ReanimatedModal/types.ts | 26 ++++++++++++--- 6 files changed, 68 insertions(+), 67 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx index b6434037734b..98f41ee4173a 100644 --- a/src/components/Modal/ReanimatedModal/Backdrop/index.tsx +++ b/src/components/Modal/ReanimatedModal/Backdrop/index.tsx @@ -28,16 +28,6 @@ function Backdrop({ .withCallback(onAnimationComplete); const Exiting = new Keyframe(getModalOutAnimation('fadeOut')).duration(animationOutTiming).reduceMotion(ReduceMotion.Never).withCallback(onAnimationComplete); - const BackdropOverlay = ( - - {!!customBackdrop && customBackdrop} - - ); - if (!customBackdrop) { return ( - {BackdropOverlay} + ); } - return BackdropOverlay; + return ( + + {customBackdrop} + + ); } export default Backdrop; diff --git a/src/components/Modal/ReanimatedModal/Container/GestureHandler.tsx b/src/components/Modal/ReanimatedModal/Container/GestureHandler.tsx index 0507a1f9d8ed..3af5a56d3a2f 100644 --- a/src/components/Modal/ReanimatedModal/Container/GestureHandler.tsx +++ b/src/components/Modal/ReanimatedModal/Container/GestureHandler.tsx @@ -1,5 +1,5 @@ import type {PropsWithChildren} from 'react'; -import React, {useMemo} from 'react'; +import React from 'react'; import type {GestureStateChangeEvent, GestureType, PanGestureHandlerEventPayload} from 'react-native-gesture-handler'; import {Gesture, GestureDetector} from 'react-native-gesture-handler'; import {useSharedValue} from 'react-native-reanimated'; @@ -52,18 +52,14 @@ function hasSwipeEnded( function GestureHandler({swipeDirection, onSwipeComplete, swipeThreshold = 100, children}: PropsWithChildren) { const initialTranslationX = useSharedValue(0); const initialTranslationY = useSharedValue(0); - const panGesture: GestureType = useMemo( - () => - Gesture.Pan() - .onStart((e) => { - initialTranslationX.set(e.translationX); - initialTranslationY.set(e.translationY); - }) - .onEnd((e) => { - hasSwipeEnded(e, {x: initialTranslationX.get(), y: initialTranslationY.get()}, swipeThreshold, swipeDirection, onSwipeComplete); - }), - [initialTranslationX, initialTranslationY, onSwipeComplete, swipeDirection, swipeThreshold], - ); + const panGesture: GestureType = Gesture.Pan() + .onStart((e) => { + initialTranslationX.set(e.translationX); + initialTranslationY.set(e.translationY); + }) + .onEnd((e) => { + hasSwipeEnded(e, {x: initialTranslationX.get(), y: initialTranslationY.get()}, swipeThreshold, swipeDirection, onSwipeComplete); + }); if (!swipeDirection || !swipeDirection?.length || !onSwipeComplete) { return children; diff --git a/src/components/Modal/ReanimatedModal/Container/index.tsx b/src/components/Modal/ReanimatedModal/Container/index.tsx index 66a9a09861e6..a5a04ebe4dc2 100644 --- a/src/components/Modal/ReanimatedModal/Container/index.tsx +++ b/src/components/Modal/ReanimatedModal/Container/index.tsx @@ -1,8 +1,7 @@ -import React, {useMemo} from 'react'; +import React from 'react'; import {View} from 'react-native'; import Animated, {Keyframe} from 'react-native-reanimated'; import {scheduleOnRN} from 'react-native-worklets'; -import type ReanimatedModalProps from '@components/Modal/ReanimatedModal/types'; import type {ContainerProps} from '@components/Modal/ReanimatedModal/types'; import {getModalInAnimation, getModalOutAnimation} from '@components/Modal/ReanimatedModal/utils'; import useAnimationTransition from '@hooks/useAnimationTransition'; @@ -23,31 +22,23 @@ function Container({ swipeDirection, swipeThreshold = 100, ...props -}: Partial & ContainerProps) { +}: ContainerProps) { const styles = useThemeStyles(); const {onAnimationComplete} = useAnimationTransition(); - const Entering = useMemo(() => { - const AnimationIn = new Keyframe(getModalInAnimation(animationIn)); + const Entering = new Keyframe(getModalInAnimation(animationIn)).duration(animationInTiming).withCallback(() => { + 'worklet'; - return AnimationIn.duration(animationInTiming).withCallback(() => { - 'worklet'; + scheduleOnRN(onOpenCallBack); + scheduleOnRN(onAnimationComplete); + }); - scheduleOnRN(onOpenCallBack); - scheduleOnRN(onAnimationComplete); - }); - }, [animationIn, animationInTiming, onOpenCallBack, onAnimationComplete]); + const Exiting = new Keyframe(getModalOutAnimation(animationOut)).duration(animationOutTiming).withCallback(() => { + 'worklet'; - const Exiting = useMemo(() => { - const AnimationOut = new Keyframe(getModalOutAnimation(animationOut)); - - return AnimationOut.duration(animationOutTiming).withCallback(() => { - 'worklet'; - - scheduleOnRN(onCloseCallBack); - scheduleOnRN(onAnimationComplete); - }); - }, [animationOutTiming, onCloseCallBack, animationOut, onAnimationComplete]); + scheduleOnRN(onCloseCallBack); + scheduleOnRN(onAnimationComplete); + }); return ( getModalInAnimationStyle(animationIn)(initProgress.get()), [initProgress]); + const animatedStyles = useAnimatedStyle(() => getModalInAnimationStyle(animationIn)(initProgress.get()), [animationIn, initProgress]); - const Exiting = useMemo( - () => - new Keyframe(getModalOutAnimation(animationOut)) - .duration(animationOutTiming) - .withCallback(() => { - onCloseCallBack(); - onAnimationComplete(); - }) - // on web the callbacks are not called when animations are disabled with the reduced motion setting on - // we enable the animations to make sure they are called - .reduceMotion(ReduceMotion.Never), - [animationOutTiming, animationOut, onCloseCallBack, onAnimationComplete], - ); + const Exiting = new Keyframe(getModalOutAnimation(animationOut)) + .duration(animationOutTiming) + .withCallback(() => { + onCloseCallBack(); + onAnimationComplete(); + }) + // on web the callbacks are not called when animations are disabled with the reduced motion setting on + // we enable the animations to make sure they are called + .reduceMotion(ReduceMotion.Never); return ( ); diff --git a/src/components/Modal/ReanimatedModal/types.ts b/src/components/Modal/ReanimatedModal/types.ts index 748d4f23e6fb..43b5d72d8300 100644 --- a/src/components/Modal/ReanimatedModal/types.ts +++ b/src/components/Modal/ReanimatedModal/types.ts @@ -162,9 +162,6 @@ type BackdropProps = { /** Callback fired when pressing the backdrop */ onBackdropPress?: () => void; - /** Delay set to animation on enter */ - animationInDelay?: number; - /** Timing of animation on enter */ animationInTiming?: number; @@ -178,7 +175,7 @@ type BackdropProps = { isBackdropVisible: boolean; }; -type ContainerProps = { +type ContainerProps = ViewProps & { /** This function is called by open animation callback */ onOpenCallBack: () => void; @@ -193,6 +190,27 @@ type ContainerProps = { /** Animation played when modal disappears */ animationOut: AnimationOut; + + /** Duration of the animation when modal appears */ + animationInTiming?: number; + + /** Duration of the animation when modal disappears */ + animationOutTiming?: number; + + /** Style applied to the modal container */ + style?: StyleProp; + + /** Modal type */ + type?: ValueOf; + + /** Callback to be fired on swipe gesture */ + onSwipeComplete?: () => void; + + /** Direction of swipe gesture */ + swipeDirection?: SwipeDirection | SwipeDirection[]; + + /** Threshold for swipe gesture */ + swipeThreshold?: number; }; export default ReanimatedModalProps; From 67effd73747b3863205cc7b72c142ef5ba5f37c7 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 14 May 2026 12:39:59 -0700 Subject: [PATCH 17/17] fix(lint): remove unused jsx-props-no-spreading disable directive Co-authored-by: Cursor --- tests/unit/ReanimatedModalTest.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/ReanimatedModalTest.tsx b/tests/unit/ReanimatedModalTest.tsx index 1d619e23cb20..e46c6f648875 100644 --- a/tests/unit/ReanimatedModalTest.tsx +++ b/tests/unit/ReanimatedModalTest.tsx @@ -58,7 +58,6 @@ jest.mock('@components/Modal/ReanimatedModal/Container', () => { return (