From 0411df13e1e8b832d8011e4fafe34436cd8fc773 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Fri, 14 Jul 2023 16:44:57 +0200 Subject: [PATCH] [RNMobile] Use Reanimated in bottom sheet height animation (#52563) * Expose max height properties in `BottomSheetProvider` * Animate bottom sheet's height with Reanimated Pass `currentHeight` in bottom sheet navigation context * Use pixel value when setting fullscreen height We need to pass pixel values in order to animate the height with Reanimated. * Rename `heightRef` to `maxHeight` * Re-enable `exhaustive-deps` lint rule in `BottomSheetNavigationContainer` * Avoid setting height using debounce * Add test ID to navigation container component * Mock Reanimated's `now` function * Update test cases related to bottom sheet height animation * Update test snapshots --- .../bottom-sheet-navigation-context.native.js | 2 +- .../navigation-container.native.js | 115 +++---- .../navigation-screen.native.js | 36 +-- .../test/navigation-container.native.js | 284 ++++++++++-------- .../src/mobile/bottom-sheet/index.native.js | 2 + .../test/__snapshots__/modal.native.js.snap | 15 +- .../with-reanimated-timer.js | 8 + 7 files changed, 265 insertions(+), 197 deletions(-) diff --git a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/bottom-sheet-navigation-context.native.js b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/bottom-sheet-navigation-context.native.js index 9e89c285b2534..5995490239d5d 100644 --- a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/bottom-sheet-navigation-context.native.js +++ b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/bottom-sheet-navigation-context.native.js @@ -6,7 +6,7 @@ import { createContext } from '@wordpress/element'; // Navigation context in BottomSheet is necessary for controlling the // height of navigation container. export const BottomSheetNavigationContext = createContext( { - currentHeight: 1, + currentHeight: { value: 0 }, setHeight: () => {}, } ); diff --git a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-container.native.js b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-container.native.js index 5f26f459182b9..3fe7ec003ceb8 100644 --- a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-container.native.js +++ b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-container.native.js @@ -1,15 +1,19 @@ /** * External dependencies */ -import { View, Easing } from 'react-native'; import { NavigationContainer, DefaultTheme } from '@react-navigation/native'; import { createStackNavigator } from '@react-navigation/stack'; +import Animated, { + Easing, + useAnimatedStyle, + useSharedValue, + withTiming, +} from 'react-native-reanimated'; /** * WordPress dependencies */ import { - useState, useContext, useMemo, useCallback, @@ -23,11 +27,11 @@ import { usePreferredColorSchemeStyle } from '@wordpress/compose'; /** * Internal dependencies */ -import { performLayoutAnimation } from '../../layout-animation'; import { BottomSheetNavigationContext, BottomSheetNavigationProvider, } from './bottom-sheet-navigation-context'; +import { BottomSheetContext } from '../bottom-sheet-context'; import styles from './styles.scss'; @@ -57,7 +61,8 @@ const options = { cardStyleInterpolator: fadeConfig, }; -const ANIMATION_DURATION = 190; +const HEIGHT_ANIMATION_DURATION = 300; +const DEFAULT_HEIGHT = 1; function BottomSheetNavigationContainer( { children, @@ -65,11 +70,14 @@ function BottomSheetNavigationContainer( { main, theme, style, + testID, } ) { const Stack = useRef( createStackNavigator() ).current; - const context = useContext( BottomSheetNavigationContext ); - const [ currentHeight, setCurrentHeight ] = useState( - context.currentHeight || 1 + const navigationContext = useContext( BottomSheetNavigationContext ); + const { maxHeight: sheetMaxHeight, isMaxHeightSet: isSheetMaxHeightSet } = + useContext( BottomSheetContext ); + const currentHeight = useSharedValue( + navigationContext.currentHeight?.value || DEFAULT_HEIGHT ); const backgroundStyle = usePreferredColorSchemeStyle( @@ -77,47 +85,49 @@ function BottomSheetNavigationContainer( { styles.backgroundDark ); - const _theme = theme || { - ...DefaultTheme, - colors: { - ...DefaultTheme.colors, - background: backgroundStyle.backgroundColor, - }, - }; + const defaultTheme = useMemo( + () => ( { + ...DefaultTheme, + colors: { + ...DefaultTheme.colors, + background: backgroundStyle.backgroundColor, + }, + } ), + [ backgroundStyle.backgroundColor ] + ); + const _theme = theme || defaultTheme; const setHeight = useCallback( ( height ) => { - // The screen is fullHeight. if ( - typeof height === 'string' && - typeof height !== typeof currentHeight + height > DEFAULT_HEIGHT && + Math.round( height ) !== Math.round( currentHeight.value ) ) { - performLayoutAnimation( ANIMATION_DURATION ); - setCurrentHeight( height ); - - return; - } - - if ( - height > 1 && - Math.round( height ) !== Math.round( currentHeight ) - ) { - if ( currentHeight === 1 ) { - setCurrentHeight( height ); - } else if ( animate ) { - performLayoutAnimation( ANIMATION_DURATION ); - setCurrentHeight( height ); + // If max height is set in the bottom sheet, we clamp + // the new height using that value. + const newHeight = isSheetMaxHeightSet + ? Math.min( sheetMaxHeight, height ) + : height; + const shouldAnimate = + animate && currentHeight.value !== DEFAULT_HEIGHT; + + if ( shouldAnimate ) { + currentHeight.value = withTiming( newHeight, { + duration: HEIGHT_ANIMATION_DURATION, + easing: Easing.out( Easing.cubic ), + } ); } else { - setCurrentHeight( height ); + currentHeight.value = newHeight; } } }, - // Disable reason: deferring this refactor to the native team. - // see https://github.com/WordPress/gutenberg/pull/41166 - // eslint-disable-next-line react-hooks/exhaustive-deps - [ currentHeight ] + [ animate, currentHeight, isSheetMaxHeightSet, sheetMaxHeight ] ); + const animatedStyles = useAnimatedStyle( () => ( { + height: currentHeight.value, + } ) ); + const screens = useMemo( () => { return Children.map( children, ( child ) => { let screen = child; @@ -136,19 +146,16 @@ function BottomSheetNavigationContainer( { /> ); } ); - // Disable reason: deferring this refactor to the native team. - // see https://github.com/WordPress/gutenberg/pull/41166 - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ children ] ); + }, [ children, main ] ); return useMemo( () => { return ( - + { main ? ( @@ -168,12 +175,18 @@ function BottomSheetNavigationContainer( { ) } - + ); - // Disable reason: deferring this refactor to the native team. - // see https://github.com/WordPress/gutenberg/pull/41166 - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ currentHeight, _theme ] ); + }, [ + _theme, + animatedStyles, + currentHeight, + main, + screens, + setHeight, + style, + testID, + ] ); } export default BottomSheetNavigationContainer; diff --git a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js index 059adb42c7089..bde7c2c67ee52 100644 --- a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js +++ b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-screen.native.js @@ -6,13 +6,17 @@ import { useNavigation, useFocusEffect, } from '@react-navigation/native'; -import { View, ScrollView, TouchableHighlight } from 'react-native'; +import { + ScrollView, + TouchableHighlight, + useWindowDimensions, + View, +} from 'react-native'; /** * WordPress dependencies */ import { BottomSheetContext } from '@wordpress/components'; -import { debounce } from '@wordpress/compose'; import { useRef, useCallback, useContext, useMemo } from '@wordpress/element'; /** @@ -29,7 +33,7 @@ const BottomSheetNavigationScreen = ( { name, } ) => { const navigation = useNavigation(); - const heightRef = useRef( { maxHeight: 0 } ); + const maxHeight = useRef( 0 ); const isFocused = useIsFocused(); const { onHandleHardwareButtonPress, @@ -38,16 +42,10 @@ const BottomSheetNavigationScreen = ( { listProps, safeAreaBottomInset, } = useContext( BottomSheetContext ); + const { height: windowHeight } = useWindowDimensions(); const { setHeight } = useContext( BottomSheetNavigationContext ); - // Disable reason: deferring this refactor to the native team. - // see https://github.com/WordPress/gutenberg/pull/41166 - // eslint-disable-next-line react-hooks/exhaustive-deps - const setHeightDebounce = useCallback( debounce( setHeight, 10 ), [ - setHeight, - ] ); - useFocusEffect( useCallback( () => { onHandleHardwareButtonPress( () => { @@ -81,17 +79,14 @@ const BottomSheetNavigationScreen = ( { useFocusEffect( useCallback( () => { if ( fullScreen ) { - setHeight( '100%' ); + setHeight( windowHeight ); setIsFullScreen( true ); - } else if ( heightRef.current.maxHeight !== 0 ) { + } else if ( maxHeight.current !== 0 ) { setIsFullScreen( false ); - setHeight( heightRef.current.maxHeight ); + setHeight( maxHeight.current ); } return () => {}; - // Disable reason: deferring this refactor to the native team. - // see https://github.com/WordPress/gutenberg/pull/41166 - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ setHeight ] ) + }, [ fullScreen, setHeight, setIsFullScreen, windowHeight ] ) ); const onLayout = ( { nativeEvent } ) => { @@ -99,10 +94,9 @@ const BottomSheetNavigationScreen = ( { return; } const { height } = nativeEvent.layout; - - if ( heightRef.current.maxHeight !== height && isFocused ) { - heightRef.current.maxHeight = height; - setHeightDebounce( height ); + if ( maxHeight.current !== height && isFocused ) { + maxHeight.current = height; + setHeight( height ); } }; diff --git a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/test/navigation-container.native.js b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/test/navigation-container.native.js index de12428fed02e..9ff6a115f0a6f 100644 --- a/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/test/navigation-container.native.js +++ b/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/test/navigation-container.native.js @@ -2,7 +2,12 @@ * External dependencies */ import { Text } from 'react-native'; -import { render, fireEvent, act } from 'test/helpers'; +import { + render, + fireEvent, + withReanimatedTimer, + advanceAnimationByTime, +} from 'test/helpers'; import { useNavigation } from '@react-navigation/native'; /** @@ -10,11 +15,8 @@ import { useNavigation } from '@react-navigation/native'; */ import NavigationContainer from '../navigation-container'; import NavigationScreen from '../navigation-screen'; -import { performLayoutAnimation } from '../../../layout-animation'; -jest.mock( '../../../layout-animation', () => ( { - performLayoutAnimation: jest.fn(), -} ) ); +const WINDOW_HEIGHT = 1000; const TestScreen = ( { fullScreen, name, navigateTo } ) => { const navigation = useNavigation(); @@ -27,130 +29,174 @@ const TestScreen = ( { fullScreen, name, navigateTo } ) => { ); }; +const fireLayoutEvent = ( element, layout ) => + fireEvent( element, 'layout', { + nativeEvent: { layout }, + } ); + beforeAll( () => { - jest.useFakeTimers( { legacyFakeTimers: true } ); + jest.spyOn( + require( 'react-native' ), + 'useWindowDimensions' + ).mockReturnValue( { width: 900, height: WINDOW_HEIGHT } ); } ); -afterAll( () => { - jest.runOnlyPendingTimers(); - jest.useRealTimers(); -} ); +it( 'animates height transitioning from non-full-screen to non-full-screen', async () => + withReanimatedTimer( async () => { + const screen = render( + + + + + ); -it( 'animates height transitioning from non-full-screen to full-screen', async () => { - const screen = render( - - - - - ); + const navigationContainer = await screen.findByTestId( + 'navigation-container' + ); - // Await navigation screen to allow async state updates to complete - const navigationScreen = await screen.findByTestId( - 'navigation-screen-test-screen-1' - ); - // Trigger non-full-screen layout event - act( () => { - fireEvent( navigationScreen, 'layout', { - nativeEvent: { - layout: { - height: 123, - }, - }, + expect( navigationContainer ).toHaveAnimatedStyle( { height: 1 } ); + + // First height value should be set without animation, but we need + // to wait for a frame to let animated styles be updated. + const screen1Layout = { height: 100 }; + fireLayoutEvent( + screen.getByTestId( 'navigation-screen-test-screen-1' ), + screen1Layout + ); + advanceAnimationByTime( 1 ); + expect( navigationContainer ).toHaveAnimatedStyle( screen1Layout ); + + // Navigate to screen 2 + fireEvent.press( screen.getByText( /test-screen-1/ ) ); + const screen2Layout = { height: 200 }; + fireLayoutEvent( + screen.getByTestId( 'navigation-screen-test-screen-2' ), + screen2Layout + ); + // The animation takes 300 ms, so we wait that time plus 1 ms + // to the completion. + advanceAnimationByTime( 301 ); + expect( navigationContainer ).toHaveAnimatedStyle( screen2Layout ); + } ) ); + +it( 'animates height transitioning from non-full-screen to full-screen', async () => + withReanimatedTimer( async () => { + const screen = render( + + + + + ); + + const navigationContainer = await screen.findByTestId( + 'navigation-container' + ); + + expect( navigationContainer ).toHaveAnimatedStyle( { height: 1 } ); + + // First height value should be set without animation, but we need + // to wait for a frame to let animated styles be updated. + const screen1Layout = { height: 100 }; + fireLayoutEvent( + screen.getByTestId( 'navigation-screen-test-screen-1' ), + screen1Layout + ); + advanceAnimationByTime( 1 ); + expect( navigationContainer ).toHaveAnimatedStyle( screen1Layout ); + + // Navigate to screen 2 + fireEvent.press( screen.getByText( /test-screen-1/ ) ); + // The animation takes 300 ms, so we wait that time plus 1 ms + // to the completion. + advanceAnimationByTime( 301 ); + expect( navigationContainer ).toHaveAnimatedStyle( { + height: WINDOW_HEIGHT, } ); - // Trigger debounced setting of height after layout event - jest.advanceTimersByTime( 10 ); - } ); - // Navigate to screen 2 - fireEvent.press( await screen.findByText( /test-screen-1/ ) ); - // Await navigation screen to allow async state updates to complete - await screen.findByText( /test-screen-2/ ); + } ) ); - expect( performLayoutAnimation ).toHaveBeenCalledTimes( 1 ); -} ); +it( 'animates height transitioning from full-screen to non-full-screen', async () => + withReanimatedTimer( async () => { + const screen = render( + + + + + ); -it( 'animates height transitioning from full-screen to non-full-screen', async () => { - const screen = render( - - - - - ); + const navigationContainer = await screen.findByTestId( + 'navigation-container' + ); - // Await navigation screen to allow async state updates to complete - const navigationScreen = await screen.findByTestId( - 'navigation-screen-test-screen-1' - ); - // Trigger non-full-screen layout event - act( () => { - fireEvent( navigationScreen, 'layout', { - nativeEvent: { - layout: { - height: 123, - }, - }, + expect( navigationContainer ).toHaveAnimatedStyle( { height: 1 } ); + + // First height value should be set without animation, but we need + // to wait for a frame to let animated styles be updated. + const screen1Layout = { height: 100 }; + fireLayoutEvent( + screen.getByTestId( 'navigation-screen-test-screen-1' ), + screen1Layout + ); + advanceAnimationByTime( 1 ); + expect( navigationContainer ).toHaveAnimatedStyle( screen1Layout ); + + // Navigate to screen 2 + fireEvent.press( screen.getByText( /test-screen-1/ ) ); + // The animation takes 300 ms, so we wait that time plus 1 ms + // to the completion. + advanceAnimationByTime( 301 ); + expect( navigationContainer ).toHaveAnimatedStyle( { + height: WINDOW_HEIGHT, } ); - // Trigger debounced setting of height after layout event - jest.advanceTimersByTime( 10 ); - } ); - // Navigate to screen 2 - fireEvent.press( await screen.findByText( /test-screen-1/ ) ); - // Navigate to screen 1 - fireEvent.press( await screen.findByText( /test-screen-2/ ) ); - // Await navigation screen to allow async state updates to complete - await screen.findByText( /test-screen-1/ ); - - expect( performLayoutAnimation ).toHaveBeenCalledTimes( 2 ); -} ); -it( 'does not animate height transitioning from full-screen to full-screen', async () => { - const screen = render( - - - - - - ); + // Navigate to screen 1 + fireEvent.press( await screen.findByText( /test-screen-2/ ) ); + // The animation takes 300 ms, so we wait that time plus 1 ms + // to the completion. + advanceAnimationByTime( 301 ); + expect( navigationContainer ).toHaveAnimatedStyle( screen1Layout ); + } ) ); - // Await navigation screen to allow async state updates to complete - const navigationScreen = await screen.findByTestId( - 'navigation-screen-test-screen-1' - ); - // Trigger non-full-screen layout event - act( () => { - fireEvent( navigationScreen, 'layout', { - nativeEvent: { - layout: { - height: 123, - }, - }, +it( 'does not animate height transitioning from full-screen to full-screen', async () => + withReanimatedTimer( async () => { + const screen = render( + + + + + ); + + const navigationContainer = await screen.findByTestId( + 'navigation-container' + ); + + // First height value should be set without animation, but we need + // to wait for a frame to let animated styles be updated. + advanceAnimationByTime( 1 ); + expect( navigationContainer ).toHaveAnimatedStyle( { + height: WINDOW_HEIGHT, } ); - // Trigger debounced setting of height after layout event - jest.advanceTimersByTime( 10 ); - } ); - // Navigate to screen 2 - fireEvent.press( await screen.findByText( /test-screen-1/ ) ); - // Navigate to screen 3 - fireEvent.press( await screen.findByText( /test-screen-2/ ) ); - // Navigate to screen 2 - fireEvent.press( await screen.findByText( /test-screen-3/ ) ); - // Await navigation screen to allow async state updates to complete - await screen.findByText( /test-screen-2/ ); - - expect( performLayoutAnimation ).toHaveBeenCalledTimes( 1 ); -} ); + + // Navigate to screen 2 + fireEvent.press( screen.getByText( /test-screen-1/ ) ); + // We wait some milliseconds to check if height has changed. + advanceAnimationByTime( 10 ); + expect( navigationContainer ).toHaveAnimatedStyle( { + height: WINDOW_HEIGHT, + } ); + } ) ); diff --git a/packages/components/src/mobile/bottom-sheet/index.native.js b/packages/components/src/mobile/bottom-sheet/index.native.js index 27eaf1b462aca..4ef8614279404 100644 --- a/packages/components/src/mobile/bottom-sheet/index.native.js +++ b/packages/components/src/mobile/bottom-sheet/index.native.js @@ -575,6 +575,8 @@ class BottomSheet extends Component { listProps, setIsFullScreen: this.setIsFullScreen, safeAreaBottomInset, + maxHeight, + isMaxHeightSet, } } > { hasNavigation ? ( diff --git a/packages/format-library/src/link/test/__snapshots__/modal.native.js.snap b/packages/format-library/src/link/test/__snapshots__/modal.native.js.snap index 9cf40dd21bf45..12d7ac6d30136 100644 --- a/packages/format-library/src/link/test/__snapshots__/modal.native.js.snap +++ b/packages/format-library/src/link/test/__snapshots__/modal.native.js.snap @@ -44,13 +44,18 @@ exports[`LinksUI LinksUI renders 1`] = ` } > setTimeout( callback, FRAME_TIME ); + // Reanimated uses a custom `now` function to advance animations. In order to be able to use + // Jest timer functions to advance animations we need to set the fake timers' internal clock. + // Reference: https://t.ly/0S__f + const reanimatedNowMockCopy = global.ReanimatedDataMock.now; + global.ReanimatedDataMock.now = jest.now; + const result = await fn(); // As part of the clean up, we run all pending timers that might have been derived from animations. act( () => jest.runOnlyPendingTimers() ); + global.ReanimatedDataMock.now = reanimatedNowMockCopy; + return result; } ); }