From 65def371659c3b64481199b2585a40a1affd9ec2 Mon Sep 17 00:00:00 2001 From: dan Date: Fri, 10 Nov 2023 19:00:46 +0000 Subject: [PATCH] Push useAnimatedScrollHandler down everywhere to work around bugs (#1866) * Move useOnMainScroll handlers to leaves * Force Feed to always take handlers * Pass handlers from the pager --- .../hooks/useAnimatedScrollHandler_FIXED.ts | 14 ++++++ src/lib/hooks/useOnMainScroll.ts | 44 +++++++++++++++---- src/view/com/lists/ListItems.tsx | 8 ++-- src/view/com/notifications/Feed.tsx | 8 ++-- src/view/com/pager/PagerWithHeader.tsx | 29 +++++++----- src/view/com/posts/Feed.tsx | 8 ++-- src/view/screens/ProfileFeed.tsx | 10 +++-- src/view/screens/ProfileList.tsx | 6 +-- src/view/screens/SearchMobile.tsx | 6 ++- 9 files changed, 95 insertions(+), 38 deletions(-) diff --git a/src/lib/hooks/useAnimatedScrollHandler_FIXED.ts b/src/lib/hooks/useAnimatedScrollHandler_FIXED.ts index eccfabbb..56a1e8b1 100644 --- a/src/lib/hooks/useAnimatedScrollHandler_FIXED.ts +++ b/src/lib/hooks/useAnimatedScrollHandler_FIXED.ts @@ -1 +1,15 @@ +// Be warned. This Hook is very buggy unless used in a very constrained way. +// To use it safely: +// +// - DO NOT pass its return value as a prop to any user-defined component. +// - DO NOT pass its return value to more than a single component. +// +// In other words, the only safe way to use it is next to the leaf Reanimated View. +// +// Relevant bug reports: +// - https://github.com/software-mansion/react-native-reanimated/issues/5345 +// - https://github.com/software-mansion/react-native-reanimated/issues/5360 +// - https://github.com/software-mansion/react-native-reanimated/issues/5364 +// +// It's great when it works though. export {useAnimatedScrollHandler} from 'react-native-reanimated' diff --git a/src/lib/hooks/useOnMainScroll.ts b/src/lib/hooks/useOnMainScroll.ts index 4cad34f4..2e7a7991 100644 --- a/src/lib/hooks/useOnMainScroll.ts +++ b/src/lib/hooks/useOnMainScroll.ts @@ -1,11 +1,15 @@ -import {useState, useCallback} from 'react' +import {useState, useCallback, useMemo} from 'react' import {NativeSyntheticEvent, NativeScrollEvent} from 'react-native' import {useSetMinimalShellMode, useMinimalShellMode} from '#/state/shell' import {useShellLayout} from '#/state/shell/shell-layout' import {s} from 'lib/styles' import {isWeb} from 'platform/detection' -import {useSharedValue, interpolate, runOnJS} from 'react-native-reanimated' -import {useAnimatedScrollHandler} from './useAnimatedScrollHandler_FIXED' +import { + useSharedValue, + interpolate, + runOnJS, + ScrollHandlers, +} from 'react-native-reanimated' function clamp(num: number, min: number, max: number) { 'worklet' @@ -15,9 +19,10 @@ function clamp(num: number, min: number, max: number) { export type OnScrollCb = ( event: NativeSyntheticEvent, ) => void +export type OnScrollHandler = ScrollHandlers export type ResetCb = () => void -export function useOnMainScroll(): [OnScrollCb, boolean, ResetCb] { +export function useOnMainScroll(): [OnScrollHandler, boolean, ResetCb] { const {headerHeight} = useShellLayout() const [isScrolledDown, setIsScrolledDown] = useState(false) const mode = useMinimalShellMode() @@ -25,12 +30,18 @@ export function useOnMainScroll(): [OnScrollCb, boolean, ResetCb] { const startDragOffset = useSharedValue(null) const startMode = useSharedValue(null) - const scrollHandler = useAnimatedScrollHandler({ - onBeginDrag(e) { + const onBeginDrag = useCallback( + (e: NativeScrollEvent) => { + 'worklet' startDragOffset.value = e.contentOffset.y startMode.value = mode.value }, - onEndDrag(e) { + [mode, startDragOffset, startMode], + ) + + const onEndDrag = useCallback( + (e: NativeScrollEvent) => { + 'worklet' startDragOffset.value = null startMode.value = null if (e.contentOffset.y < headerHeight.value / 2) { @@ -41,7 +52,12 @@ export function useOnMainScroll(): [OnScrollCb, boolean, ResetCb] { setMode(Math.round(mode.value) === 1) } }, - onScroll(e) { + [startDragOffset, startMode, setMode, mode, headerHeight], + ) + + const onScroll = useCallback( + (e: NativeScrollEvent) => { + 'worklet' // Keep track of whether we want to show "scroll to top". if (!isScrolledDown && e.contentOffset.y > s.window.height) { runOnJS(setIsScrolledDown)(true) @@ -86,7 +102,17 @@ export function useOnMainScroll(): [OnScrollCb, boolean, ResetCb] { startMode.value = mode.value } }, - }) + [headerHeight, mode, setMode, isScrolledDown, startDragOffset, startMode], + ) + + const scrollHandler: ScrollHandlers = useMemo( + () => ({ + onBeginDrag, + onEndDrag, + onScroll, + }), + [onBeginDrag, onEndDrag, onScroll], + ) return [ scrollHandler, diff --git a/src/view/com/lists/ListItems.tsx b/src/view/com/lists/ListItems.tsx index 3658e552..fe7b9b78 100644 --- a/src/view/com/lists/ListItems.tsx +++ b/src/view/com/lists/ListItems.tsx @@ -19,9 +19,10 @@ import {useAnalytics} from 'lib/analytics/analytics' import {usePalette} from 'lib/hooks/usePalette' import {useWebMediaQueries} from 'lib/hooks/useWebMediaQueries' import {s} from 'lib/styles' -import {OnScrollCb} from 'lib/hooks/useOnMainScroll' +import {OnScrollHandler} from 'lib/hooks/useOnMainScroll' import {logger} from '#/logger' import {useModalControls} from '#/state/modals' +import {useAnimatedScrollHandler} from '#/lib/hooks/useAnimatedScrollHandler_FIXED' const LOADING_ITEM = {_reactKey: '__loading__'} const EMPTY_ITEM = {_reactKey: '__empty__'} @@ -44,7 +45,7 @@ export const ListItems = observer(function ListItemsImpl({ list: ListModel style?: StyleProp scrollElRef?: MutableRefObject | null> - onScroll?: OnScrollCb + onScroll: OnScrollHandler onPressTryAgain?: () => void renderHeader: () => JSX.Element renderEmptyState: () => JSX.Element @@ -205,6 +206,7 @@ export const ListItems = observer(function ListItemsImpl({ [list.isLoading], ) + const scrollHandler = useAnimatedScrollHandler(onScroll) return ( | null> onPressTryAgain?: () => void - onScroll?: OnScrollCb + onScroll?: OnScrollHandler ListHeaderComponent?: () => JSX.Element }) { const pal = usePalette('default') @@ -129,6 +130,7 @@ export const Feed = observer(function Feed({ [view], ) + const scrollHandler = useAnimatedScrollHandler(onScroll || {}) return ( @@ -161,7 +163,7 @@ export const Feed = observer(function Feed({ } onEndReached={onEndReached} onEndReachedThreshold={0.6} - onScroll={onScroll} + onScroll={scrollHandler} scrollEventThrottle={1} contentContainerStyle={s.contentContainer} // @ts-ignore our .web version only -prf diff --git a/src/view/com/pager/PagerWithHeader.tsx b/src/view/com/pager/PagerWithHeader.tsx index f3ea4a1d..8b9e0c85 100644 --- a/src/view/com/pager/PagerWithHeader.tsx +++ b/src/view/com/pager/PagerWithHeader.tsx @@ -1,5 +1,10 @@ import * as React from 'react' -import {LayoutChangeEvent, StyleSheet, View} from 'react-native' +import { + LayoutChangeEvent, + NativeScrollEvent, + StyleSheet, + View, +} from 'react-native' import Animated, { Easing, useAnimatedReaction, @@ -11,14 +16,13 @@ import Animated, { import {Pager, PagerRef, RenderTabBarFnProps} from 'view/com/pager/Pager' import {TabBar} from './TabBar' import {useWebMediaQueries} from 'lib/hooks/useWebMediaQueries' -import {OnScrollCb} from 'lib/hooks/useOnMainScroll' -import {useAnimatedScrollHandler} from 'lib/hooks/useAnimatedScrollHandler_FIXED' +import {OnScrollHandler} from 'lib/hooks/useOnMainScroll' const SCROLLED_DOWN_LIMIT = 200 interface PagerWithHeaderChildParams { headerHeight: number - onScroll: OnScrollCb + onScroll: OnScrollHandler isScrolledDown: boolean } @@ -141,11 +145,10 @@ export const PagerWithHeader = React.forwardRef( ) // props to pass into children render functions - const onScroll = useAnimatedScrollHandler({ - onScroll(e) { - scrollY.value = e.contentOffset.y - }, - }) + function onScrollWorklet(e: NativeScrollEvent) { + 'worklet' + scrollY.value = e.contentOffset.y + } const onPageSelectedInner = React.useCallback( (index: number) => { @@ -192,7 +195,9 @@ export const PagerWithHeader = React.forwardRef( output = child({ headerHeight, isScrolledDown, - onScroll: i === currentPage ? onScroll : noop, + onScroll: { + onScroll: i === currentPage ? onScrollWorklet : noop, + }, }) } // Pager children must be noncollapsible plain s. @@ -225,7 +230,9 @@ const styles = StyleSheet.create({ }, }) -function noop() {} +function noop() { + 'worklet' +} function toArray(v: T | T[]): T[] { if (Array.isArray(v)) { diff --git a/src/view/com/posts/Feed.tsx b/src/view/com/posts/Feed.tsx index 1ecb1491..5b517f4c 100644 --- a/src/view/com/posts/Feed.tsx +++ b/src/view/com/posts/Feed.tsx @@ -14,10 +14,11 @@ import {FeedErrorMessage} from './FeedErrorMessage' import {PostsFeedModel} from 'state/models/feeds/posts' import {FeedSlice} from './FeedSlice' import {LoadMoreRetryBtn} from '../util/LoadMoreRetryBtn' -import {OnScrollCb} from 'lib/hooks/useOnMainScroll' +import {OnScrollHandler} from 'lib/hooks/useOnMainScroll' import {s} from 'lib/styles' import {useAnalytics} from 'lib/analytics/analytics' import {usePalette} from 'lib/hooks/usePalette' +import {useAnimatedScrollHandler} from '#/lib/hooks/useAnimatedScrollHandler_FIXED' import {useTheme} from 'lib/ThemeContext' import {logger} from '#/logger' @@ -43,7 +44,7 @@ export const Feed = observer(function Feed({ feed: PostsFeedModel style?: StyleProp scrollElRef?: MutableRefObject | null> - onScroll?: OnScrollCb + onScroll?: OnScrollHandler scrollEventThrottle?: number renderEmptyState: () => JSX.Element renderEndOfFeed?: () => JSX.Element @@ -157,6 +158,7 @@ export const Feed = observer(function Feed({ [feed.isLoadingMore, feed.hasMore, feed.isEmpty, renderEndOfFeed], ) + const scrollHandler = useAnimatedScrollHandler(onScroll || {}) return ( void - onScroll: OnScrollCb + onScroll: OnScrollHandler }) { const pal = usePalette('default') const {_} = useLingui() + const scrollHandler = useAnimatedScrollHandler(onScroll) if (!feedInfo) { return @@ -456,7 +458,7 @@ const AboutSection = observer(function AboutPageImpl({ + onScroll={scrollHandler}> void - onScroll: OnScrollCb + onScroll: OnScrollHandler headerHeight: number isScrolledDown: boolean } diff --git a/src/view/screens/SearchMobile.tsx b/src/view/screens/SearchMobile.tsx index c1df58ff..92c255d5 100644 --- a/src/view/screens/SearchMobile.tsx +++ b/src/view/screens/SearchMobile.tsx @@ -14,6 +14,7 @@ import { } from 'lib/routes/types' import {observer} from 'mobx-react-lite' import {Text} from 'view/com/util/text/Text' +import {useAnimatedScrollHandler} from '#/lib/hooks/useAnimatedScrollHandler_FIXED' import {useStores} from 'state/index' import {UserAutocompleteModel} from 'state/models/discovery/user-autocomplete' import {SearchUIModel} from 'state/models/ui/search' @@ -131,6 +132,7 @@ export const SearchScreen = withAuthRequired( } }, []) + const scrollHandler = useAnimatedScrollHandler(onMainScroll) return ( @@ -156,8 +158,8 @@ export const SearchScreen = withAuthRequired( ref={scrollViewRef} testID="searchScrollView" style={pal.view} - onScroll={onMainScroll} - scrollEventThrottle={100}> + onScroll={scrollHandler} + scrollEventThrottle={1}> {query && autocompleteView.suggestions.length ? ( <> {autocompleteView.suggestions.map((suggestion, index) => (