Fix jumps when navigating into long threads (#2878)

* Reveal parents in chunks to fix scroll jumps

Co-authored-by: Hailey <me@haileyok.com>

* Prevent layout jump when navigating to QT due to missing metrics

---------

Co-authored-by: Hailey <me@haileyok.com>
zio/stable
dan 2024-02-16 18:07:47 +00:00 committed by GitHub
parent e303940eaa
commit c5641ac2b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 74 additions and 40 deletions

View File

@ -43,10 +43,13 @@ import {
usePreferencesQuery, usePreferencesQuery,
} from '#/state/queries/preferences' } from '#/state/queries/preferences'
import {useSession} from '#/state/session' import {useSession} from '#/state/session'
import {isAndroid, isNative} from '#/platform/detection' import {isAndroid, isNative, isWeb} from '#/platform/detection'
import {logger} from '#/logger'
import {moderatePost_wrapped as moderatePost} from '#/lib/moderatePost_wrapped' import {moderatePost_wrapped as moderatePost} from '#/lib/moderatePost_wrapped'
// FlatList maintainVisibleContentPosition breaks if too many items
// are prepended. This seems to be an optimal number based on *shrug*.
const PARENTS_CHUNK_SIZE = 15
const MAINTAIN_VISIBLE_CONTENT_POSITION = { const MAINTAIN_VISIBLE_CONTENT_POSITION = {
// We don't insert any elements before the root row while loading. // We don't insert any elements before the root row while loading.
// So the row we want to use as the scroll anchor is the first row. // So the row we want to use as the scroll anchor is the first row.
@ -165,8 +168,10 @@ function PostThreadLoaded({
const {isMobile, isTabletOrMobile} = useWebMediaQueries() const {isMobile, isTabletOrMobile} = useWebMediaQueries()
const ref = useRef<ListMethods>(null) const ref = useRef<ListMethods>(null)
const highlightedPostRef = useRef<View | null>(null) const highlightedPostRef = useRef<View | null>(null)
const [maxVisible, setMaxVisible] = React.useState(100) const [maxParents, setMaxParents] = React.useState(
const [isPTRing, setIsPTRing] = React.useState(false) isWeb ? Infinity : PARENTS_CHUNK_SIZE,
)
const [maxReplies, setMaxReplies] = React.useState(100)
const treeView = React.useMemo( const treeView = React.useMemo(
() => !!threadViewPrefs.lab_treeViewEnabled && hasBranchingReplies(thread), () => !!threadViewPrefs.lab_treeViewEnabled && hasBranchingReplies(thread),
[threadViewPrefs, thread], [threadViewPrefs, thread],
@ -206,10 +211,18 @@ function PostThreadLoaded({
// maintainVisibleContentPosition and onContentSizeChange // maintainVisibleContentPosition and onContentSizeChange
// to "hold onto" the correct row instead of the first one. // to "hold onto" the correct row instead of the first one.
} else { } else {
// Everything is loaded. // Everything is loaded
arr.push(TOP_COMPONENT) let startIndex = Math.max(0, parents.length - maxParents)
for (const parent of parents) { if (startIndex === 0) {
arr.push(parent) arr.push(TOP_COMPONENT)
} else {
// When progressively revealing parents, rendering a placeholder
// here will cause scrolling jumps. Don't add it unless you test it.
// QT'ing this thread is a great way to test all the scrolling hacks:
// https://bsky.app/profile/www.mozzius.dev/post/3kjqhblh6qk2o
}
for (let i = startIndex; i < parents.length; i++) {
arr.push(parents[i])
} }
} }
} }
@ -220,17 +233,18 @@ function PostThreadLoaded({
if (highlightedPost.ctx.isChildLoading) { if (highlightedPost.ctx.isChildLoading) {
arr.push(CHILD_SPINNER) arr.push(CHILD_SPINNER)
} else { } else {
for (const reply of replies) { for (let i = 0; i < replies.length; i++) {
arr.push(reply) arr.push(replies[i])
if (i === maxReplies) {
arr.push(LOAD_MORE)
break
}
} }
arr.push(BOTTOM_COMPONENT) arr.push(BOTTOM_COMPONENT)
} }
} }
if (arr.length > maxVisible) {
arr = arr.slice(0, maxVisible).concat([LOAD_MORE])
}
return arr return arr
}, [skeleton, maxVisible, deferParents]) }, [skeleton, deferParents, maxParents, maxReplies])
// This is only used on the web to keep the post in view when its parents load. // This is only used on the web to keep the post in view when its parents load.
// On native, we rely on `maintainVisibleContentPosition` instead. // On native, we rely on `maintainVisibleContentPosition` instead.
@ -258,15 +272,28 @@ function PostThreadLoaded({
} }
}, [thread]) }, [thread])
const onPTR = React.useCallback(async () => { // On native, we reveal parents in chunks. Although they're all already
setIsPTRing(true) // loaded and FlatList already has its own virtualization, unfortunately FlatList
try { // has a bug that causes the content to jump around if too many items are getting
await onRefresh() // prepended at once. It also jumps around if items get prepended during scroll.
} catch (err) { // To work around this, we prepend rows after scroll bumps against the top and rests.
logger.error('Failed to refresh posts thread', {message: err}) const needsBumpMaxParents = React.useRef(false)
const onStartReached = React.useCallback(() => {
if (maxParents < skeleton.parents.length) {
needsBumpMaxParents.current = true
} }
setIsPTRing(false) }, [maxParents, skeleton.parents.length])
}, [setIsPTRing, onRefresh]) const bumpMaxParentsIfNeeded = React.useCallback(() => {
if (!isNative) {
return
}
if (needsBumpMaxParents.current) {
needsBumpMaxParents.current = false
setMaxParents(n => n + PARENTS_CHUNK_SIZE)
}
}, [])
const onMomentumScrollEnd = bumpMaxParentsIfNeeded
const onScrollToTop = bumpMaxParentsIfNeeded
const renderItem = React.useCallback( const renderItem = React.useCallback(
({item, index}: {item: RowItem; index: number}) => { ({item, index}: {item: RowItem; index: number}) => {
@ -301,7 +328,7 @@ function PostThreadLoaded({
} else if (item === LOAD_MORE) { } else if (item === LOAD_MORE) {
return ( return (
<Pressable <Pressable
onPress={() => setMaxVisible(n => n + 50)} onPress={() => setMaxReplies(n => n + 50)}
style={[pal.border, pal.view, styles.itemContainer]} style={[pal.border, pal.view, styles.itemContainer]}
accessibilityLabel={_(msg`Load more posts`)} accessibilityLabel={_(msg`Load more posts`)}
accessibilityHint=""> accessibilityHint="">
@ -345,6 +372,8 @@ function PostThreadLoaded({
const next = isThreadPost(posts[index - 1]) const next = isThreadPost(posts[index - 1])
? (posts[index - 1] as ThreadPost) ? (posts[index - 1] as ThreadPost)
: undefined : undefined
const hasUnrevealedParents =
index === 0 && maxParents < skeleton.parents.length
return ( return (
<View <View
ref={item.ctx.isHighlightedPost ? highlightedPostRef : undefined} ref={item.ctx.isHighlightedPost ? highlightedPostRef : undefined}
@ -360,7 +389,9 @@ function PostThreadLoaded({
hasMore={item.ctx.hasMore} hasMore={item.ctx.hasMore}
showChildReplyLine={item.ctx.showChildReplyLine} showChildReplyLine={item.ctx.showChildReplyLine}
showParentReplyLine={item.ctx.showParentReplyLine} showParentReplyLine={item.ctx.showParentReplyLine}
hasPrecedingItem={!!prev?.ctx.showChildReplyLine} hasPrecedingItem={
!!prev?.ctx.showChildReplyLine || hasUnrevealedParents
}
onPostReply={onRefresh} onPostReply={onRefresh}
/> />
</View> </View>
@ -383,6 +414,8 @@ function PostThreadLoaded({
onRefresh, onRefresh,
deferParents, deferParents,
treeView, treeView,
skeleton.parents.length,
maxParents,
_, _,
], ],
) )
@ -393,9 +426,10 @@ function PostThreadLoaded({
data={posts} data={posts}
keyExtractor={item => item._reactKey} keyExtractor={item => item._reactKey}
renderItem={renderItem} renderItem={renderItem}
refreshing={isPTRing}
onRefresh={onPTR}
onContentSizeChange={isNative ? undefined : onContentSizeChangeWeb} onContentSizeChange={isNative ? undefined : onContentSizeChangeWeb}
onStartReached={onStartReached}
onMomentumScrollEnd={onMomentumScrollEnd}
onScrollToTop={onScrollToTop}
maintainVisibleContentPosition={ maintainVisibleContentPosition={
isNative ? MAINTAIN_VISIBLE_CONTENT_POSITION : undefined isNative ? MAINTAIN_VISIBLE_CONTENT_POSITION : undefined
} }

View File

@ -44,6 +44,7 @@ import {Shadow, usePostShadow, POST_TOMBSTONE} from '#/state/cache/post-shadow'
import {ThreadPost} from '#/state/queries/post-thread' import {ThreadPost} from '#/state/queries/post-thread'
import {useSession} from 'state/session' import {useSession} from 'state/session'
import {WhoCanReply} from '../threadgate/WhoCanReply' import {WhoCanReply} from '../threadgate/WhoCanReply'
import {LoadingPlaceholder} from '../util/LoadingPlaceholder'
export function PostThreadItem({ export function PostThreadItem({
post, post,
@ -164,8 +165,6 @@ let PostThreadItemLoaded = ({
() => countLines(richText?.text) >= MAX_POST_LINES, () => countLines(richText?.text) >= MAX_POST_LINES,
) )
const {currentAccount} = useSession() const {currentAccount} = useSession()
const hasEngagement = post.likeCount || post.repostCount
const rootUri = record.reply?.root?.uri || post.uri const rootUri = record.reply?.root?.uri || post.uri
const postHref = React.useMemo(() => { const postHref = React.useMemo(() => {
const urip = new AtUri(post.uri) const urip = new AtUri(post.uri)
@ -357,9 +356,16 @@ let PostThreadItemLoaded = ({
translatorUrl={translatorUrl} translatorUrl={translatorUrl}
needsTranslation={needsTranslation} needsTranslation={needsTranslation}
/> />
{hasEngagement ? ( {post.repostCount !== 0 || post.likeCount !== 0 ? (
// Show this section unless we're *sure* it has no engagement.
<View style={[styles.expandedInfo, pal.border]}> <View style={[styles.expandedInfo, pal.border]}>
{post.repostCount ? ( {post.repostCount == null && post.likeCount == null && (
// If we're still loading and not sure, assume this post has engagement.
// This lets us avoid a layout shift for the common case (embedded post with likes/reposts).
// TODO: embeds should include metrics to avoid us having to guess.
<LoadingPlaceholder width={50} height={20} />
)}
{post.repostCount != null && post.repostCount !== 0 ? (
<Link <Link
style={styles.expandedInfoItem} style={styles.expandedInfoItem}
href={repostsHref} href={repostsHref}
@ -374,10 +380,8 @@ let PostThreadItemLoaded = ({
{pluralize(post.repostCount, 'repost')} {pluralize(post.repostCount, 'repost')}
</Text> </Text>
</Link> </Link>
) : ( ) : null}
<></> {post.likeCount != null && post.likeCount !== 0 ? (
)}
{post.likeCount ? (
<Link <Link
style={styles.expandedInfoItem} style={styles.expandedInfoItem}
href={likesHref} href={likesHref}
@ -392,13 +396,9 @@ let PostThreadItemLoaded = ({
{pluralize(post.likeCount, 'like')} {pluralize(post.likeCount, 'like')}
</Text> </Text>
</Link> </Link>
) : ( ) : null}
<></>
)}
</View> </View>
) : ( ) : null}
<></>
)}
<View style={[s.pl10, s.pr10, s.pb5]}> <View style={[s.pl10, s.pr10, s.pb5]}>
<PostCtrls <PostCtrls
big big