Skip to content

Commit bc8eb82

Browse files
authored
fix: flash list scroll to behaviour (#3602)
## 🎯 Goal This PR addresses 2 issues with `MessageFlashList` which have made `scrollTo` behaviour unreliable. 1. It should fix (or at least make a lot better) the issue of being snapped back to the end of the list whenever we try to scroll to a quoted message for the very first time 2. It should address not being able to scroll to the first unread message Even though these 2 issues are seemingly similar, their underlying causes are completely different. #### Quoted message scroll to This particular issue happens specifically when we've loaded the messages into memory that we want to scroll to (so loading a completely different `messageSet` is working fine). The offender here is MVCP and scrolling to bottom in particular. I'm pretty certain that there's an upstream bug here, however I did not have a chance to debug it more thoroughly and find the actual root cause. Roughly what goes on is the following: - The list mounts, MVCP gets armed - We press on a quoted message and begin scrolling to it - Mid scroll, MVCP decides something's changed (because of the fact that recycling kicks in and layout gets revalidated) and triggers a pending scroll - The scroll is immediately consumed, snapping us back just after the quoted message scroll finishes I actually had a patch in `FlashList` which allows us to clear all pending MVCP scrolls and also suspend MVCP from doing anything and it worked like a charm (like an imperative API). However for now, this will have to do. At the very least, even when the issue happens it should reconcile shortly after and fix its positioning. #### Initial scroll to first unread This issue on the other hand is completely unrelated to MVCP. It's actually related to our automatic scrolling mechanism, which happens on mount and then also whenever `autoscrollToRecent` actually changes. Namely, if we look into `FlashList`'s implementation we can see that `scrollToEnd` is actually ultimately wrapped within a `setTimeout`, likely to try to delay it natively on the JS runtime for timing purposes. However, this also means that if a state update happens really quickly (for example `targetedMessage` updating) we'll end up making the 2 scrolls race. `scrollToEnd` typically wins because it's invoked later and also because it invokes the underlying scroll view's ref rather than some abstraction. We need this custom handling because `initialScrollIndex` for `FlashList` is very often not correct at all (and off by some number of offset). This is especially true whenever we scroll between 2 message sets and virtually load new data. I've yet to find why this is but maybe some day. To prevent this, we expose a new API on the `Channel` level that allows us to anticipate when we're about to scroll to a targeted message, bypassing the `scrollToEnd` entirely in favor of having a pending scroll. These fixes will be ported back to V8 as well. ## 🛠 Implementation details <!-- Provide a description of the implementation --> ## 🎨 UI Changes <!-- Add relevant screenshots --> <details> <summary>iOS</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> <details> <summary>Android</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> ## 🧪 Testing <!-- Explain how this change can be tested (or why it can't be tested) --> ## ☑️ Checklist - [ ] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required) - [ ] PR targets the `develop` branch - [ ] Documentation is updated - [ ] New code is tested in main example apps, including all possible scenarios - [ ] SampleApp iOS and Android - [ ] Expo iOS and Android
1 parent daf6e29 commit bc8eb82

4 files changed

Lines changed: 60 additions & 31 deletions

File tree

package/src/components/Channel/Channel.tsx

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,18 @@ const ChannelWithContext = (props: PropsWithChildren<ChannelPropsWithContext>) =
567567
channel,
568568
});
569569

570+
const shouldLoadInitialChannelAtFirstUnreadMessage = useStableCallback((unreadCount?: number) => {
571+
if (messageId || !initialScrollToFirstUnreadMessage || !client.user) {
572+
return false;
573+
}
574+
575+
return (unreadCount ?? channel.countUnread()) > scrollToFirstUnreadThreshold;
576+
});
577+
578+
const hasPendingInitialTargetLoad = useStableCallback(() => {
579+
return !!messageId || shouldLoadInitialChannelAtFirstUnreadMessage();
580+
});
581+
570582
const { setMessages: copyMessagesStateFromChannel, viewabilityChangedCallback } =
571583
usePrunableMessageList({ maximumMessageLimit, setMessages: rawCopyMessagesStateFromChannel });
572584

@@ -693,6 +705,7 @@ const ChannelWithContext = (props: PropsWithChildren<ChannelPropsWithContext>) =
693705
const initChannel = async () => {
694706
setLastRead(new Date());
695707
const unreadCount = channel.countUnread();
708+
const shouldLoadAtFirstUnread = shouldLoadInitialChannelAtFirstUnreadMessage(unreadCount);
696709
if (!channel || !shouldSyncChannel) {
697710
return;
698711
}
@@ -722,13 +735,14 @@ const ChannelWithContext = (props: PropsWithChildren<ChannelPropsWithContext>) =
722735

723736
if (messageId) {
724737
await loadChannelAroundMessage({ messageId, setTargetedMessage });
725-
} else if (
726-
initialScrollToFirstUnreadMessage &&
727-
client.user &&
728-
unreadCount > scrollToFirstUnreadThreshold
729-
) {
738+
} else if (shouldLoadAtFirstUnread) {
739+
const clientUserId = client.user?.id;
740+
if (!clientUserId) {
741+
return;
742+
}
743+
730744
// eslint-disable-next-line @typescript-eslint/no-unused-vars
731-
const { user, ...ownReadState } = channel.state.read[client.user.id];
745+
const { user, ...ownReadState } = channel.state.read[clientUserId];
732746

733747
await loadChannelAtFirstUnreadMessage({
734748
channelUnreadState: ownReadState,
@@ -1578,6 +1592,7 @@ const ChannelWithContext = (props: PropsWithChildren<ChannelPropsWithContext>) =
15781592
setChannelUnreadState,
15791593
setLastRead,
15801594
setTargetedMessage,
1595+
hasPendingInitialTargetLoad,
15811596
targetedMessage,
15821597
threadList,
15831598
uploadAbortControllerRef,

package/src/components/Channel/hooks/useCreateChannelContext.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export const useCreateChannelContext = ({
2727
setChannelUnreadState,
2828
setLastRead,
2929
setTargetedMessage,
30+
hasPendingInitialTargetLoad,
3031
targetedMessage,
3132
threadList,
3233
uploadAbortControllerRef,
@@ -69,6 +70,7 @@ export const useCreateChannelContext = ({
6970
setChannelUnreadState,
7071
setLastRead,
7172
setTargetedMessage,
73+
hasPendingInitialTargetLoad,
7274
targetedMessage,
7375
threadList,
7476
uploadAbortControllerRef,

package/src/components/MessageList/MessageFlashList.tsx

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ type MessageFlashListPropsWithContext = Pick<
132132
| 'scrollToFirstUnreadThreshold'
133133
| 'setChannelUnreadState'
134134
| 'setTargetedMessage'
135+
| 'hasPendingInitialTargetLoad'
135136
| 'targetedMessage'
136137
| 'threadList'
137138
| 'maximumMessageLimit'
@@ -289,6 +290,7 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
289290
setChannelUnreadState,
290291
setFlatListRef,
291292
setTargetedMessage,
293+
hasPendingInitialTargetLoad,
292294
targetedMessage,
293295
thread,
294296
threadInstance,
@@ -388,11 +390,15 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
388390

389391
useEffect(() => {
390392
if (autoscrollToRecent && flashListRef.current) {
393+
if (hasPendingInitialTargetLoad?.()) {
394+
return;
395+
}
396+
391397
flashListRef.current.scrollToEnd({
392398
animated: true,
393399
});
394400
}
395-
}, [autoscrollToRecent]);
401+
}, [autoscrollToRecent, hasPendingInitialTargetLoad]);
396402

397403
const maintainVisibleContentPosition = useMemo(() => {
398404
return {
@@ -408,18 +414,6 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
408414
}
409415
}, [disabled]);
410416

411-
const indexToScrollToRef = useRef<number | undefined>(undefined);
412-
413-
const initialIndexToScrollTo = useMemo(() => {
414-
return targetedMessage
415-
? processedMessageList.findIndex((message) => message?.id === targetedMessage)
416-
: -1;
417-
}, [processedMessageList, targetedMessage]);
418-
419-
useEffect(() => {
420-
indexToScrollToRef.current = initialIndexToScrollTo;
421-
}, [initialIndexToScrollTo]);
422-
423417
/**
424418
* Check if a messageId needs to be scrolled to after list loads, and scroll to it
425419
* Note: This effect fires on every list change with a small debounce so that scrolling isnt abrupted by an immediate rerender
@@ -440,13 +434,29 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
440434
scrollToDebounceTimeoutRef.current = setTimeout(() => {
441435
clearTimeout(scrollToDebounceTimeoutRef.current);
442436

443-
// now scroll to it
444-
flashListRef.current?.scrollToIndex({
445-
animated: true,
446-
index: indexOfParentInMessageList,
447-
viewPosition: 0.5,
437+
const scrollToIndex = async () => {
438+
const list = flashListRef.current;
439+
440+
if (!list) {
441+
return false;
442+
}
443+
444+
await list.scrollToIndex({
445+
index: indexOfParentInMessageList,
446+
animated: true,
447+
viewPosition: 0.5,
448+
});
449+
450+
return true;
451+
};
452+
453+
requestAnimationFrame(async () => {
454+
await scrollToIndex();
455+
requestAnimationFrame(async () => {
456+
await scrollToIndex();
457+
setTargetedMessage(undefined);
458+
});
448459
});
449-
setTargetedMessage(undefined);
450460
}, WAIT_FOR_SCROLL_TIMEOUT);
451461
}
452462
}, [loadChannelAroundMessage, processedMessageList, setTargetedMessage, targetedMessage]);
@@ -456,8 +466,6 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
456466
(message) => message?.id === messageId,
457467
);
458468

459-
indexToScrollToRef.current = indexOfParentInMessageList;
460-
461469
try {
462470
if (indexOfParentInMessageList === -1) {
463471
clearTimeout(scrollToDebounceTimeoutRef.current);
@@ -529,7 +537,6 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
529537
setScrollToBottomButtonVisible(true);
530538
return;
531539
} else {
532-
indexToScrollToRef.current = undefined;
533540
setAutoscrollToRecent(true);
534541
}
535542
const latestNonCurrentMessageBeforeUpdate = latestNonCurrentMessageBeforeUpdateRef.current;
@@ -1064,9 +1071,6 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
10641071
data={processedMessageList}
10651072
drawDistance={800}
10661073
getItemType={getItemTypeInternal}
1067-
initialScrollIndex={
1068-
indexToScrollToRef.current === -1 ? undefined : indexToScrollToRef.current
1069-
}
10701074
keyboardShouldPersistTaps='handled'
10711075
keyExtractor={keyExtractor}
10721076
ListFooterComponent={ListFooterComponent}
@@ -1203,6 +1207,7 @@ export const MessageFlashList = (props: MessageFlashListProps) => {
12031207
scrollToFirstUnreadThreshold,
12041208
setChannelUnreadState,
12051209
setTargetedMessage,
1210+
hasPendingInitialTargetLoad,
12061211
targetedMessage,
12071212
threadList,
12081213
} = useChannelContext();
@@ -1246,6 +1251,7 @@ export const MessageFlashList = (props: MessageFlashListProps) => {
12461251
scrollToFirstUnreadThreshold,
12471252
setChannelUnreadState,
12481253
setTargetedMessage,
1254+
hasPendingInitialTargetLoad,
12491255
shouldShowUnreadUnderlay,
12501256
targetedMessage,
12511257
thread,

package/src/contexts/channelContext/ChannelContext.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ export type ChannelContextValue = {
112112
setChannelUnreadState: (data: ChannelUnreadStateStoreType['channelUnreadState']) => void;
113113
setLastRead: React.Dispatch<React.SetStateAction<Date | undefined>>;
114114
setTargetedMessage: (messageId?: string) => void;
115+
/**
116+
* Returns true when Channel is about to load an initial targeted message.
117+
*
118+
* @internal
119+
*/
120+
hasPendingInitialTargetLoad?: () => boolean;
115121
/**
116122
* Abort controller for cancelling async requests made for uploading images/files
117123
* Its a map of filename and AbortController

0 commit comments

Comments
 (0)