From 39664bfde6165fa2c52ba8e6bf9a9ce0e3e25a50 Mon Sep 17 00:00:00 2001 From: Eric Bailey Date: Sun, 8 Jun 2025 16:02:51 -0500 Subject: [PATCH] Ok I think scrolling is solid --- src/components/Layout/Header/index.tsx | 16 ++- src/screens/PostThread/index.tsx | 164 +++++++++++++++-------- src/state/queries/usePostThread/index.ts | 17 +-- 3 files changed, 128 insertions(+), 69 deletions(-) diff --git a/src/components/Layout/Header/index.tsx b/src/components/Layout/Header/index.tsx index 44faa96498..b308c786e2 100644 --- a/src/components/Layout/Header/index.tsx +++ b/src/components/Layout/Header/index.tsx @@ -1,24 +1,29 @@ import {createContext, useCallback, useContext} from 'react' -import {GestureResponderEvent, Keyboard, View} from 'react-native' +import { + type GestureResponderEvent, + Keyboard, + View, + type ViewProps, +} from 'react-native' import {msg} from '@lingui/macro' import {useLingui} from '@lingui/react' import {useNavigation} from '@react-navigation/native' import {HITSLOP_30} from '#/lib/constants' -import {NavigationProp} from '#/lib/routes/types' +import {type NavigationProp} from '#/lib/routes/types' import {isIOS} from '#/platform/detection' import {useSetDrawerOpen} from '#/state/shell' import { atoms as a, platform, - TextStyleProp, + type TextStyleProp, useBreakpoints, useGutters, useLayoutBreakpoints, useTheme, web, } from '#/alf' -import {Button, ButtonIcon, ButtonProps} from '#/components/Button' +import {Button, ButtonIcon, type ButtonProps} from '#/components/Button' import {ArrowLeft_Stroke2_Corner0_Rounded as ArrowLeft} from '#/components/icons/Arrow' import {Menu_Stroke2_Corner0_Rounded as Menu} from '#/components/icons/Menu' import { @@ -35,11 +40,13 @@ export function Outer({ noBottomBorder, headerRef, sticky = true, + onLayout, }: { children: React.ReactNode noBottomBorder?: boolean headerRef?: React.MutableRefObject sticky?: boolean + onLayout?: ViewProps['onLayout'] }) { const t = useTheme() const gutters = useGutters([0, 'base']) @@ -50,6 +57,7 @@ export function Outer({ return ( (null) - const headerRef = useRef(null) const anchorRef = useRef(null) + const headerRef = useRef(null) + const headerHeight = useRef(0) /* * On a cold load, parents are not prepended until the anchor post has @@ -107,11 +108,9 @@ export function Inner({uri}: {uri: string | undefined}) { * this is always true. And when a user changes thread parameters, we also * manually set this to true. */ - const shouldScrollToAnchor = useRef(true) + const shouldHandleScroll = useRef(true) /** - * WEB ONLY - * - * Called any time the content size of the list changes, just before paint. + * Called any time the content size of the list changes, _just_ before paint. * * We want this to fire every time we change params (which will reset * `deferParents` via `onLayout` on the anchor post, due to the key change), @@ -122,46 +121,85 @@ export function Inner({uri}: {uri: string | undefined}) { * in the anchor being pinned as the first item. */ const onContentSizeChangeWebOnly = web(() => { - const anchorElement = anchorRef.current as any as Element - const headerElement = headerRef.current as any as Element + const list = listRef.current + const anchor = anchorRef.current as any as Element - if (anchorElement && headerElement) { - const anchorOffsetTop = anchorElement.getBoundingClientRect().top - const headerHeight = headerElement.getBoundingClientRect().height + if (list && anchor && shouldHandleScroll.current) { + const anchorOffsetTop = anchor.getBoundingClientRect().top - if (shouldScrollToAnchor.current) { - /* - * `deferParents` is `true` on a cold load, and always reset to - * `true` when params change (via the key on the anchor post). - * - * On a cold load, on the first pass of this logic, the anchor post is - * the first item in the list. Therefore `anchorOffsetTop - headerHeight` - * will be 0. - * - * On a warm load, on the first pass of this logic, the anchor post may - * not move (if there are no parents above it), or it may have gone off - * the screen above, because of the sudden lack of parents due to - * `deferParents === true`. This negative value (minus `headerHeight`) - * will result in a _negative_ `offset` value, which will scroll the - * anchor post _down_ to the top of the screen. - * - * Once parents are prepended, this will fire again. Now, the - * `anchorOffsetTop` will be positive, which minus the header height, - * will give us a _positive_ offset, which will scroll the anchor post - * back _up_ to the top of the screen. - */ - listRef.current?.scrollToOffset({ - offset: anchorOffsetTop - headerHeight, - }) + /* + * `deferParents` is `true` on a cold load, and always reset to + * `true` when params change via `prepareForParamsUpdate`. + * + * On a cold load or a push to a new post, on the first pass of this + * logic, the anchor post is the first item in the list. Therefore + * `anchorOffsetTop - headerHeight` will be 0. + * + * When a user changes thread params, on the first pass of this logic, + * the anchor post may not move (if there are no parents above it), or it + * may have gone off the screen above, because of the sudden lack of + * parents due to `deferParents === true`. This negative value (minus + * `headerHeight`) will result in a _negative_ `offset` value, which will + * scroll the anchor post _down_ to the top of the screen. + * + * However, `prepareForParamsUpdate` also resets scroll to `0`, so when a user + * changes params, the anchor post's offset will actually be equivalent + * to the `headerHeight` because of how the DOM is stacked on web. + * Therefore, `anchorOffsetTop - headerHeight` will once again be 0, + * which means the first pass in this case will result in no scroll. + * + * Then, once parents are prepended, this will fire again. Now, the + * `anchorOffsetTop` will be positive, which minus the header height, + * will give us a _positive_ offset, which will scroll the anchor post + * back _up_ to the top of the screen. + */ + list.scrollToOffset({ + offset: anchorOffsetTop - headerHeight.current, + }) - /* - * After the second pass, `deferParents` will be `false`, and we need - * to ensure this doesn't run again until scroll handling is requested - * again via `shouldScrollToAnchor.current === true` and a params - * change. - */ - if (!deferParents) shouldScrollToAnchor.current = false - } + /* + * After the second pass, `deferParents` will be `false`, and we need + * to ensure this doesn't run again until scroll handling is requested + * again via `shouldHandleScroll.current === true` and a params + * change via `prepareForParamsUpdate`. + */ + if (!deferParents) shouldHandleScroll.current = false + } + }) + + /** + * Ditto the above, but for native. + */ + const onContentSizeChangeNativeOnly = native(() => { + const list = listRef.current + const anchor = anchorRef.current + + if (list && anchor && shouldHandleScroll.current) { + /* + * `prepareForParamsUpdate` is called any time the user changes thread params like + * `view` or `sort`, which sets `deferParents(true)` and resets the + * scroll to the top of the list. However, there is a split second + * where the top of the list is wherever the parents _just were_. So if + * there were parents, the anchor is not at the top of the list just + * prior to this handler being called. + * + * Once this handler is called, the anchor post is the first item in + * the list (because of `deferParents` being `true`), and so we can + * synchronously scroll the list back to the top of the list (which is + * 0 on native, no need to handle `headerHeight`). + */ + list.scrollToOffset({ + animated: false, + offset: 0, + }) + + /* + * After this first pass, `deferParents` will be `false`, and those + * will render in. However, the anchor post will retain its position + * because of `maintainVisibleContentPosition` handling on native. So we + * don't need to let this handler run again, like we do on web. + */ + shouldHandleScroll.current = false } }) @@ -259,10 +297,10 @@ export function Inner({uri}: {uri: string | undefined}) { return ( { + setDeferParents(true) + setMaxParentCount(PARENT_CHUNK_SIZE) + setMaxChildrenCount(CHILDREN_CHUNK_SIZE) + listRef.current?.scrollToOffset({ + animated: false, + offset: 0, + }) + shouldHandleScroll.current = true + }, [setDeferParents, setMaxParentCount, setMaxChildrenCount]) + const setSortWrapped = useCallback( (sort: string) => { - setDeferParents(true) - shouldScrollToAnchor.current = true + prepareForParamsUpdate() thread.actions.setSort(sort) }, - [thread, setDeferParents], + [thread, prepareForParamsUpdate], ) const setViewWrapped = useCallback( (view: ThreadViewOption) => { + prepareForParamsUpdate() thread.actions.setView(view) - setDeferParents(true) - shouldScrollToAnchor.current = true }, - [thread, setDeferParents], + [thread, prepareForParamsUpdate], ) return ( <> - + { + headerHeight.current = e.nativeEvent.layout.height + }}> @@ -391,7 +442,10 @@ export function Inner({uri}: {uri: string | undefined}) { data={slices} renderItem={renderItem} keyExtractor={keyExtractor} - onContentSizeChange={onContentSizeChangeWebOnly} + onContentSizeChange={platform({ + web: onContentSizeChangeWebOnly, + default: onContentSizeChangeNativeOnly, + })} onStartReached={onStartReached} onEndReached={onEndReached} onEndReachedThreshold={2} diff --git a/src/state/queries/usePostThread/index.ts b/src/state/queries/usePostThread/index.ts index 09ec354629..fa062cd991 100644 --- a/src/state/queries/usePostThread/index.ts +++ b/src/state/queries/usePostThread/index.ts @@ -56,16 +56,13 @@ export function usePostThread({anchor}: {anchor?: string}) { enabled: isThreadPreferencesLoaded && !!anchor && !!moderationOpts, queryKey: postThreadQueryKey, async queryFn(ctx) { - const {data} = await wait( - 400, - agent.app.bsky.unspecced.getPostThreadV2({ - anchor: anchor!, - branchingFactor: view === 'linear' ? 1 : undefined, - below: BELOW, - sort: sort, - prioritizeFollowedUsers: prioritizeFollowedUsers, - }), - ) + const {data} = await agent.app.bsky.unspecced.getPostThreadV2({ + anchor: anchor!, + branchingFactor: view === 'linear' ? 1 : undefined, + below: BELOW, + sort: sort, + prioritizeFollowedUsers: prioritizeFollowedUsers, + }) /* * Initialize `ctx.meta` to track if we know we have additional replies