From 76f9ee3ed2da1e71725d236e64c511d714fcd37b Mon Sep 17 00:00:00 2001 From: Santhosh Vaiyapuri <3846977+santhoshvai@users.noreply.github.com> Date: Tue, 21 Mar 2023 10:08:47 +0100 Subject: [PATCH] fix: initial scroll to first unread message was broken when there more than 55 unread messages (#2020) * fix: refresh unread count on notif mark_read event * fix: remove unnecessary creation of new function for targeted message * fix: do not delete the read state * fix: unread message load * chore: add back messageList dep * fix: tsc error * fix: proper obj spread removal * fix: set initial scroll done even when targeted message was not found * chore: remove commented out code --- package/src/components/Channel/Channel.tsx | 50 +++++--- .../Channel/hooks/useTargetedMessage.ts | 6 +- .../ChannelPreview/ChannelPreview.tsx | 7 ++ .../components/MessageList/MessageList.tsx | 113 +++++++++--------- .../MessageList/utils/getReadStates.ts | 7 +- 5 files changed, 105 insertions(+), 78 deletions(-) diff --git a/package/src/components/Channel/Channel.tsx b/package/src/components/Channel/Channel.tsx index a2a5f74d12..4e97525f22 100644 --- a/package/src/components/Channel/Channel.tsx +++ b/package/src/components/Channel/Channel.tsx @@ -833,22 +833,42 @@ const ChannelWithContext = < */ const loadChannelAtFirstUnreadMessage = () => { if (!channel) return; - const unreadCount = channel.countUnread(); - if (unreadCount <= scrollToFirstUnreadThreshold) return; - // temporarily clear existing messages so that messageList component gets a list change and does not scroll to any unread message first before loading completes - setMessages([]); + let unreadMessageIdToScrollTo: string | undefined; // query for messages around the last read date - return channelQueryCallRef.current(async () => { - setLoading(true); - const lastReadDate = channel.lastRead() || new Date(0); - await channel.query({ - messages: { - created_at_around: lastReadDate, - limit: 25, - }, - }); - setLoading(false); - }); + return channelQueryCallRef.current( + async () => { + setLoading(true); + const lastReadDate = channel.lastRead(); + // if last read date is present we can just fetch messages around that date + // last read date not being present is an edge case if somewhere the user of SDK deletes the read state (this will usually never happen) + if (lastReadDate) { + setHasNoMoreRecentMessagesToLoad(false); // we are jumping to a message, hence we do not know for sure anymore if there are no more recent messages + // get totally 30 messages... max 15 before last read date and max 15 after last read date + // ref: https://github.com/GetStream/chat/pull/2588 + await channel.query( + { + messages: { + created_at_around: lastReadDate, + limit: 30, + }, + }, + 'new', + ); + unreadMessageIdToScrollTo = channel.state.messages.find( + (m) => lastReadDate < m.created_at, + )?.id; + } else { + // we just load the latest messages (25 is the default) and we cant scroll to first unread message + await channel.state.loadMessageIntoState('latest'); + } + setLoading(false); + }, + () => { + if (unreadMessageIdToScrollTo) { + setTargetedMessage(unreadMessageIdToScrollTo); + } + }, + ); }; /** diff --git a/package/src/components/Channel/hooks/useTargetedMessage.ts b/package/src/components/Channel/hooks/useTargetedMessage.ts index d49a4e4676..390a436a7e 100644 --- a/package/src/components/Channel/hooks/useTargetedMessage.ts +++ b/package/src/components/Channel/hooks/useTargetedMessage.ts @@ -14,7 +14,7 @@ export const useTargetedMessage = (messageId?: string) => { }; }, []); - const setTargetedMessageTimeout = (messageId: string) => { + const setTargetedMessageTimeoutRef = useRef((messageId: string) => { clearTargetedMessageCall.current && clearTimeout(clearTargetedMessageCall.current); clearTargetedMessageCall.current = setTimeout(() => { @@ -22,10 +22,10 @@ export const useTargetedMessage = (messageId?: string) => { }, 3000); setTargetedMessage(messageId); - }; + }); return { - setTargetedMessage: setTargetedMessageTimeout, + setTargetedMessage: setTargetedMessageTimeoutRef.current, targetedMessage, }; }; diff --git a/package/src/components/ChannelPreview/ChannelPreview.tsx b/package/src/components/ChannelPreview/ChannelPreview.tsx index 7d13ed52b5..a7cbf66c3d 100644 --- a/package/src/components/ChannelPreview/ChannelPreview.tsx +++ b/package/src/components/ChannelPreview/ChannelPreview.tsx @@ -52,6 +52,13 @@ const ChannelPreviewWithContext = < const channelLastMessage = channel.lastMessage(); const channelLastMessageString = `${channelLastMessage?.id}${channelLastMessage?.updated_at}`; + useEffect(() => { + const { unsubscribe } = client.on('notification.mark_read', () => { + setUnread(channel.countUnread()); + }); + return unsubscribe; + }, []); + useEffect(() => { if ( channelLastMessage && diff --git a/package/src/components/MessageList/MessageList.tsx b/package/src/components/MessageList/MessageList.tsx index d4a64bc949..96a39333da 100644 --- a/package/src/components/MessageList/MessageList.tsx +++ b/package/src/components/MessageList/MessageList.tsx @@ -272,7 +272,6 @@ const MessageListWithContext = < overlay, reloadChannel, ScrollToBottomButton, - scrollToFirstUnreadThreshold, selectedPicker, setFlatListRef, setMessages, @@ -332,8 +331,7 @@ const MessageListWithContext = < * If the prop `initialScrollToFirstUnreadMessage` was enabled, then we scroll to the unread msg and set it to true * If not, the default offset of 0 for flatList means that it has been set already */ - const initialScrollSet = useRef(!initialScrollToFirstUnreadMessage); - + const [isInitialScrollDone, setInitialScrollDone] = useState(!initialScrollToFirstUnreadMessage); const channelResyncScrollSet = useRef(true); /** @@ -341,6 +339,11 @@ const MessageListWithContext = < */ const scrollToDebounceTimeoutRef = useRef(); + /** + * The timeout id used to lazier load the initial scroll set flag + */ + const initialScrollSettingTimeoutRef = useRef(); + /** * If a messageId was requested to scroll to but was unloaded, * this flag keeps track of it to scroll to it after loading the message @@ -423,29 +426,39 @@ const MessageListWithContext = < }, [disabled]); useEffect(() => { - /** - * 1. !initialScrollToFirstUnreadMessage && channel.countUnread() > 0 - * - * In this case MessageList won't scroll to first unread message when opened, so we can mark - * the channel as read right after opening. - * - * 2. initialScrollToFirstUnreadMessage && channel.countUnread() <= scrollToFirstUnreadThreshold - * - * In this case MessageList will be opened to first unread message. - * But if there are not enough (scrollToFirstUnreadThreshold) unread messages, then MessageList - * won't need to scroll up. So we can safely mark the channel as read right after opening. - */ - const shouldMarkReadOnFirstLoad = - !loading && - channel && - ((!initialScrollToFirstUnreadMessage && channel.countUnread() > 0) || - (initialScrollToFirstUnreadMessage && - channel.countUnread() <= scrollToFirstUnreadThreshold)); - - if (shouldMarkReadOnFirstLoad) { + const getShouldMarkReadAutomatically = (): boolean => { + if (loading || !channel) { + // nothing to do + return false; + } else if (channel.countUnread() > 0) { + if (!initialScrollToFirstUnreadMessage) { + /* + * In this case MessageList won't scroll to first unread message when opened, so we can mark + * the channel as read right after opening. + * */ + return true; + } else { + /* + * In this case MessageList will be opened to first unread message. + * But if there are were not enough unread messages, so that scrollToBottom button was not shown + * then MessageList won't need to scroll up. So we can safely mark the channel as read right after opening. + * + * NOTE: we must ensure that initial scroll is done, otherwise we do not wait till the unread scroll is finished + * */ + if (scrollToBottomButtonVisible) return false; + /* if scrollToBottom button was not visible, wait till + * - initial scroll is done (indicates that if scrolling to index was needed it was triggered) + * */ + return isInitialScrollDone; + } + } + return false; + }; + + if (getShouldMarkReadAutomatically()) { markRead(); } - }, [loading]); + }, [loading, scrollToBottomButtonVisible, isInitialScrollDone]); useEffect(() => { const lastReceivedMessage = getLastReceivedMessage(messageList); @@ -492,7 +505,7 @@ const MessageListWithContext = < if (threadList || hasNoMoreRecentMessagesToLoad) { scrollToBottomIfNeeded(); - } else if (!scrollToBottomButtonVisible) { + } else { setScrollToBottomButtonVisible(true); } @@ -529,23 +542,17 @@ const MessageListWithContext = < if (!channel || (!channel.initialized && !channel.offlineMode)) return null; const lastRead = channel.lastRead(); - const countUnread = channel.countUnread(); function isMessageUnread(messageArrayIndex: number): boolean { - if (lastRead && message.created_at) { - return lastRead < message.created_at; - } else { - const isLatestMessageSetShown = !!channel.state.messageSets.find( - (set) => set.isCurrent && set.isLatest, - ); - return isLatestMessageSetShown && messageArrayIndex <= countUnread - 1; + const msg = messageList?.[messageArrayIndex]; + if (lastRead && msg?.created_at) { + return lastRead < msg.created_at; } + return false; } const isCurrentMessageUnread = isMessageUnread(index); - const isLastMessageUnread = isMessageUnread(index + 1); - const showUnreadUnderlay = isCurrentMessageUnread && scrollToBottomButtonVisible; - const insertInlineUnreadIndicator = showUnreadUnderlay && !isLastMessageUnread; + const insertInlineUnreadIndicator = showUnreadUnderlay && !isMessageUnread(index + 1); // show only if previous message is read if (message.type === 'system') { return ( @@ -742,8 +749,8 @@ const MessageListWithContext = < maybeCallOnEndReached(); } - // Show scrollToBottom button once scroll position goes beyond 300. - const isScrollAtBottom = offset <= 300; + // Show scrollToBottom button once scroll position goes beyond 150. + const isScrollAtBottom = offset <= 150; const showScrollToBottomButton = !isScrollAtBottom || !hasNoMoreRecentMessagesToLoad; const shouldMarkRead = @@ -829,21 +836,16 @@ const MessageListWithContext = < * Note: This effect fires on every list change with a small debounce so that scrolling isnt abrupted by an immediate rerender */ useEffect(() => { - if (scrollToDebounceTimeoutRef.current) clearTimeout(scrollToDebounceTimeoutRef.current); scrollToDebounceTimeoutRef.current = setTimeout(() => { + if (initialScrollToFirstUnreadMessage) { + initialScrollSettingTimeoutRef.current = setTimeout(() => { + // small timeout to ensure that handleScroll is called after scrollToIndex to set this flag + setInitialScrollDone(true); + }, 500); + } // goToMessage method might have requested to scroll to a message let messageIdToScroll: string | undefined = messageIdToScrollToRef.current; - const countUnread = channelRef.current?.countUnread(); - if ( - !initialScrollSet.current && - initialScrollToFirstUnreadMessage && - countUnread > scrollToFirstUnreadThreshold - ) { - // find the first unread message, if we have to initially scroll to an unread message - if (messageList.length >= countUnread) { - messageIdToScroll = messageList[countUnread - 1].id; - } - } else if (targetedMessage && messageIdLastScrolledToRef.current !== targetedMessage) { + if (targetedMessage && messageIdLastScrolledToRef.current !== targetedMessage) { // if some messageId was targeted but not scrolledTo yet // we have scroll to there after loading completes messageIdToScroll = targetedMessage; @@ -862,14 +864,13 @@ const MessageListWithContext = < messageIdToScrollToRef.current = undefined; // keep track of this messageId, so that we dont scroll to again for targeted message change messageIdLastScrolledToRef.current = messageIdToScroll; - if (!initialScrollSet.current && initialScrollToFirstUnreadMessage) { - initialScrollSet.current = true; - } else { - setTargetedMessage(messageIdToScroll); - } } }, 150); - }, [channel.initialized, messageList, targetedMessage, initialScrollToFirstUnreadMessage]); + return () => { + clearTimeout(scrollToDebounceTimeoutRef.current); + clearTimeout(initialScrollSettingTimeoutRef.current); + }; + }, [targetedMessage, initialScrollToFirstUnreadMessage, messageList]); const messagesWithImages = legacyImageViewerSwipeBehaviour && diff --git a/package/src/components/MessageList/utils/getReadStates.ts b/package/src/components/MessageList/utils/getReadStates.ts index c9318845b1..4092f3377c 100644 --- a/package/src/components/MessageList/utils/getReadStates.ts +++ b/package/src/components/MessageList/utils/getReadStates.ts @@ -25,10 +25,9 @@ export const getReadStates = < /** * Channel read state is stored by user and we only care about users who aren't the client */ - if (clientUserId) { - delete read[clientUserId]; - } - const members = Object.values(read); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { [clientUserId ?? '']: _ignore, ...filteredRead } = read; + const members = Object.values(filteredRead); /** * Track number of members who have read previous messages