From 77e79d63087b3e97c6dff28be0d6ff495550de5a Mon Sep 17 00:00:00 2001 From: Sharon Zheng Date: Mon, 26 Sep 2022 11:28:56 -0700 Subject: [PATCH] Animated.ScrollView with RefreshControl applying Animated transform twice Summary: There was a bug on Android when an Animated.ScrollView had a RefreshControl while an Animated style was applied, ie `transform`: ``` } style={{ transform: [{ translateY: new Animated.Value(200, {useNativeDriver: true}) }] }} /> ``` The transform value was being incorrectly applied twice. Since the styles were applied once on RefreshControl and once on NativeScrollView, the transform style is effectively applied twice: **1. ScrollView.js** - RefreshControl gets the transform through Fabric commit - [The RefreshControl gets wrapped around ScrollView](https://fburl.com/code/k60krxbj) while on iOS there is no change in the parent/child relationship. [Outer/inner styles are split and applied to RefreshControl/ScrollView](https://fburl.com/code/b2to75er), and transform styles are applied on the parent (RefreshControl) **2. createAnimatedComponent.js** - NativeScrollView gets the transform through Animated - [ScrollView forwards its ref to NativeScrollView](https://fburl.com/code/w1whtl5f), which means AnimatedComponent is setting the transform styles on NativeScrollView and not RefreshControl as ScrollView.js did This diff fixes this bug by using the `useAnimatedProps` hook which makes both RefreshControl and ScrollView components into animated components. Otherwise, the components don't know what to do with Animated values. --- Changelog: [Internal][Fixed] - Animated transform style properties were being applied twice when used on an Animated.ScrollView with RefreshControl on Android Reviewed By: javache Differential Revision: D38815633 fbshipit-source-id: 2b76639d2237176b6aae4fb1e22cf1a1ec70a69a --- .../Animated/components/AnimatedScrollView.js | 113 ++++++++++++++++-- 1 file changed, 104 insertions(+), 9 deletions(-) diff --git a/Libraries/Animated/components/AnimatedScrollView.js b/Libraries/Animated/components/AnimatedScrollView.js index 7589a04cd7f..df2dbccd2ad 100644 --- a/Libraries/Animated/components/AnimatedScrollView.js +++ b/Libraries/Animated/components/AnimatedScrollView.js @@ -9,22 +9,117 @@ */ import * as React from 'react'; +import {useMemo} from 'react'; +import RefreshControl from '../../Components/RefreshControl/RefreshControl'; import ScrollView from '../../Components/ScrollView/ScrollView'; +import StyleSheet from '../../StyleSheet/StyleSheet'; +import flattenStyle from '../../StyleSheet/flattenStyle'; +import splitLayoutProps from '../../StyleSheet/splitLayoutProps'; +import Platform from '../../Utilities/Platform'; +import useMergeRefs from '../../Utilities/useMergeRefs'; import createAnimatedComponent from '../createAnimatedComponent'; +import useAnimatedProps from '../useAnimatedProps'; import type {AnimatedComponentType} from '../createAnimatedComponent'; +type Props = React.ElementConfig; +type Instance = React.ElementRef; + /** * @see https://github.com/facebook/react-native/commit/b8c8562 */ -const ScrollViewWithEventThrottle = React.forwardRef((props, ref) => ( - -)); +const AnimatedScrollView: AnimatedComponentType = + React.forwardRef((props, forwardedRef) => { + // (Android only) When a ScrollView has a RefreshControl and + // any `style` property set with an Animated.Value, the CSS + // gets incorrectly applied twice. This is because ScrollView + // swaps the parent/child relationship of itself and the + // RefreshControl component (see ScrollView.js for more details). + if ( + Platform.OS === 'android' && + props.refreshControl != null && + props.style != null + ) { + return ( + + ); + } else { + return ( + + ); + } + }); -export default (createAnimatedComponent( - ScrollViewWithEventThrottle, -): AnimatedComponentType< - React.ElementConfig, - React.ElementRef, ->); +const AnimatedScrollViewWithInvertedRefreshControl = React.forwardRef( + ( + props: { + ...React.ElementConfig, + // $FlowFixMe[unclear-type] Same Flow type as `refreshControl` in ScrollView + refreshControl: React.Element, + }, + forwardedRef, + ) => { + // Split `props` into the animate-able props for the parent (RefreshControl) + // and child (ScrollView). + const {intermediatePropsForRefreshControl, intermediatePropsForScrollView} = + useMemo(() => { + const {outer, inner} = splitLayoutProps(flattenStyle(props.style)); + return { + intermediatePropsForRefreshControl: {style: outer}, + intermediatePropsForScrollView: {...props, style: inner}, + }; + }, [props]); + + // Handle animated props on `refreshControl`. + const [refreshControlAnimatedProps, refreshControlRef] = useAnimatedProps( + intermediatePropsForRefreshControl, + ); + // NOTE: Assumes that refreshControl.ref` and `refreshControl.style` can be + // safely clobbered. + const refreshControl: React.Element = + React.cloneElement(props.refreshControl, { + ...refreshControlAnimatedProps, + ref: refreshControlRef, + }); + + // Handle animated props on `NativeDirectionalScrollView`. + const [scrollViewAnimatedProps, scrollViewRef] = useAnimatedProps< + Props, + Instance, + >(intermediatePropsForScrollView); + const ref = useMergeRefs(scrollViewRef, forwardedRef); + + return ( + // $FlowFixMe[incompatible-use] Investigate useAnimatedProps return value + + ); + }, +); + +const AnimatedScrollViewWithoutInvertedRefreshControl = + createAnimatedComponent(ScrollView); + +export default AnimatedScrollView;