From ea29168f4336ff0657b9fa8a6960a4f3d0f75b55 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Thu, 12 Dec 2024 22:41:10 +0100 Subject: [PATCH 1/8] add logic to scrol when message appears --- src/pages/home/report/ReportActionsList.tsx | 35 +++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 24119bedaa8c..254099b341d9 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -164,6 +164,7 @@ function ReportActionsList({ const lastMessageTime = useRef(null); const [isVisible, setIsVisible] = useState(Visibility.isVisible); const isFocused = useIsFocused(); + const [pendingBottomScroll, setPendingBottomScroll] = useState(false); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`); const [accountID] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.accountID}); @@ -426,6 +427,12 @@ function ReportActionsList({ // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); + const shouldScrollToBottom = useCallback(() => { + const prevActions = Object.values(prevSortedVisibleReportActionsObjects); + const lastPrevAction = prevActions.at(0); // Safely access the first element + return lastAction?.reportActionID === lastPrevAction?.reportActionID; + }, [prevSortedVisibleReportActionsObjects, lastAction]); + const scrollToBottomForCurrentUserAction = useCallback( (isFromCurrentUser: boolean) => { // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where @@ -433,6 +440,11 @@ function ReportActionsList({ if (!isFromCurrentUser) { return; } + + if (!isFromCurrentUser || scrollingVerticalOffset.current === 0) { + return; + } + if (!hasNewestReportActionRef.current) { if (isInNarrowPaneModal) { return; @@ -440,10 +452,29 @@ function ReportActionsList({ Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)); return; } - InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom()); + + if (shouldScrollToBottom()) { + setPendingBottomScroll(true); + } else { + InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom()); + } }, - [isInNarrowPaneModal, reportScrollManager, report.reportID], + [isInNarrowPaneModal, reportScrollManager, report.reportID, shouldScrollToBottom], ); + + useEffect(() => { + if (!pendingBottomScroll) { + return; + } + + if (shouldScrollToBottom()) { + InteractionManager.runAfterInteractions(() => { + reportScrollManager.scrollToBottom(); + setPendingBottomScroll(false); + }); + } + }, [pendingBottomScroll, prevSortedVisibleReportActionsObjects, reportScrollManager, shouldScrollToBottom]); + useEffect(() => { // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function? // Answer: On web, when navigating to another report screen, the previous report screen doesn't get unmounted, From 768c6d0cf15f36ed3031f9f577db21359f1b5e88 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Thu, 12 Dec 2024 23:26:21 +0100 Subject: [PATCH 2/8] adjust isNewMessageDisplayed --- src/pages/home/report/ReportActionsList.tsx | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 254099b341d9..fd008c69ef4c 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -427,19 +427,16 @@ function ReportActionsList({ // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); - const shouldScrollToBottom = useCallback(() => { + const isNewMessageDisplayed = useCallback(() => { const prevActions = Object.values(prevSortedVisibleReportActionsObjects); - const lastPrevAction = prevActions.at(0); // Safely access the first element - return lastAction?.reportActionID === lastPrevAction?.reportActionID; + const lastPrevAction = prevActions.at(0); + return lastAction?.reportActionID !== lastPrevAction?.reportActionID; }, [prevSortedVisibleReportActionsObjects, lastAction]); const scrollToBottomForCurrentUserAction = useCallback( (isFromCurrentUser: boolean) => { // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where // they are now in the list. - if (!isFromCurrentUser) { - return; - } if (!isFromCurrentUser || scrollingVerticalOffset.current === 0) { return; @@ -453,13 +450,13 @@ function ReportActionsList({ return; } - if (shouldScrollToBottom()) { + if (!isNewMessageDisplayed()) { setPendingBottomScroll(true); } else { InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom()); } }, - [isInNarrowPaneModal, reportScrollManager, report.reportID, shouldScrollToBottom], + [isInNarrowPaneModal, reportScrollManager, report.reportID, isNewMessageDisplayed], ); useEffect(() => { @@ -467,13 +464,13 @@ function ReportActionsList({ return; } - if (shouldScrollToBottom()) { + if (isNewMessageDisplayed()) { InteractionManager.runAfterInteractions(() => { reportScrollManager.scrollToBottom(); setPendingBottomScroll(false); }); } - }, [pendingBottomScroll, prevSortedVisibleReportActionsObjects, reportScrollManager, shouldScrollToBottom]); + }, [pendingBottomScroll, prevSortedVisibleReportActionsObjects, reportScrollManager, isNewMessageDisplayed]); useEffect(() => { // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function? From 99f35025cac21612d6935b028f9ea3d592d2d556 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Wed, 18 Dec 2024 16:23:14 +0100 Subject: [PATCH 3/8] changes after main merge --- src/pages/home/report/ReportActionsList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 5617f7f99dd1..9c1f0e3f6bd7 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -438,7 +438,7 @@ function ReportActionsList({ if (!isNewMessageDisplayed()) { setPendingBottomScroll(true); } else { - InteractionManager.runAfterInteractions(() => reportScrollManager.scrollToBottom()); + reportScrollManager.scrollToBottom(); } }); }, From 925a3e0102b4045d2f099e351676fa2b2a760032 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Wed, 18 Dec 2024 16:47:51 +0100 Subject: [PATCH 4/8] adjust after mergin main --- src/pages/home/report/ReportActionsList.tsx | 35 +++++++++++---------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 9c1f0e3f6bd7..c1a56fa45905 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -422,31 +422,32 @@ function ReportActionsList({ const scrollToBottomForCurrentUserAction = useCallback( (isFromCurrentUser: boolean) => { - InteractionManager.runAfterInteractions(() => { - // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where - // they are now in the list. - if (!isFromCurrentUser) { - return; - } - if (!hasNewestReportActionRef.current) { - if (isInNarrowPaneModal) { - return; - } - Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)); + // InteractionManager.runAfterInteractions(() => { + // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where + // they are now in the list. + if (!isFromCurrentUser || scrollingVerticalOffset.current === 0) { + return; + } + if (!hasNewestReportActionRef.current) { + if (isInNarrowPaneModal) { return; } - if (!isNewMessageDisplayed()) { - setPendingBottomScroll(true); - } else { + Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)); + return; + } + if (!isNewMessageDisplayed()) { + setPendingBottomScroll(true); + } else { + InteractionManager.runAfterInteractions(() => { reportScrollManager.scrollToBottom(); - } - }); + }); + } }, [isInNarrowPaneModal, reportScrollManager, report.reportID, isNewMessageDisplayed], ); useEffect(() => { - if (!pendingBottomScroll) { + if (!pendingBottomScroll || scrollingVerticalOffset.current === 0) { return; } From 273498ab916a3f9ea5e67bccaa792c36402fc82f Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Wed, 18 Dec 2024 17:16:54 +0100 Subject: [PATCH 5/8] adjust to new eslint rules --- src/pages/home/report/ReportActionsList.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index c1a56fa45905..eaffd2400f4d 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -169,7 +169,7 @@ function ReportActionsList({ const isFocused = useIsFocused(); const [pendingBottomScroll, setPendingBottomScroll] = useState(false); - const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`); + const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? CONST.DEFAULT_NUMBER_ID}`); const [accountID] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.accountID}); useEffect(() => { @@ -184,7 +184,7 @@ function ReportActionsList({ const readActionSkipped = useRef(false); const hasHeaderRendered = useRef(false); const hasFooterRendered = useRef(false); - const linkedReportActionID = route?.params?.reportActionID ?? '-1'; + const linkedReportActionID = route?.params?.reportActionID ?? CONST.DEFAULT_NUMBER_ID; const lastAction = sortedVisibleReportActions.at(0); const sortedVisibleReportActionsObjects: OnyxTypes.ReportActions = useMemo( From a6a79d754b7a745387f03082c2dbd65dbab7344a Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Thu, 2 Jan 2025 14:25:58 +0100 Subject: [PATCH 6/8] refactor useCallstack to useMemo --- src/pages/home/report/ReportActionsList.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 0b279ad43d63..187b228d75ed 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -414,7 +414,7 @@ function ReportActionsList({ // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); - const isNewMessageDisplayed = useCallback(() => { + const isNewMessageDisplayed = useMemo(() => { const prevActions = Object.values(prevSortedVisibleReportActionsObjects); const lastPrevAction = prevActions.at(0); return lastAction?.reportActionID !== lastPrevAction?.reportActionID; @@ -424,7 +424,6 @@ function ReportActionsList({ (isFromCurrentUser: boolean) => { // If a new comment is added and it's from the current user scroll to the bottom otherwise leave the user positioned where // they are now in the list. - if (!isFromCurrentUser || scrollingVerticalOffset.current === 0 || !isReportScreenTopmostCentralPane()) { return; } @@ -432,7 +431,7 @@ function ReportActionsList({ Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)); return; } - if (!isNewMessageDisplayed()) { + if (!isNewMessageDisplayed) { setPendingBottomScroll(true); } else { InteractionManager.runAfterInteractions(() => { @@ -448,7 +447,7 @@ function ReportActionsList({ return; } - if (isNewMessageDisplayed()) { + if (isNewMessageDisplayed) { InteractionManager.runAfterInteractions(() => { reportScrollManager.scrollToBottom(); setPendingBottomScroll(false); From 37b1e7b4f98f27d1757ea0d11a072dc3771b1ea5 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Thu, 9 Jan 2025 15:18:52 +0100 Subject: [PATCH 7/8] remove unnecessery useEffect deps --- src/pages/home/report/ReportActionsList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index 187b228d75ed..786c213bb729 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -453,7 +453,7 @@ function ReportActionsList({ setPendingBottomScroll(false); }); } - }, [pendingBottomScroll, prevSortedVisibleReportActionsObjects, reportScrollManager, isNewMessageDisplayed]); + }, [pendingBottomScroll, reportScrollManager, isNewMessageDisplayed]); useEffect(() => { // Why are we doing this, when in the cleanup of the useEffect we are already calling the unsubscribe function? From 929a4f8336507c63390a1bc3d2f2777227837494 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Mon, 20 Jan 2025 19:08:44 +0100 Subject: [PATCH 8/8] rename variable to be more descriptive --- src/pages/home/report/ReportActionsList.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index ee1708de709a..8b6e20029c13 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -446,8 +446,8 @@ function ReportActionsList({ const isNewMessageDisplayed = useMemo(() => { const prevActions = Object.values(prevSortedVisibleReportActionsObjects); - const lastPrevAction = prevActions.at(0); - return lastAction?.reportActionID !== lastPrevAction?.reportActionID; + const lastPrevVisibleAction = prevActions.at(0); + return lastAction?.reportActionID !== lastPrevVisibleAction?.reportActionID; }, [prevSortedVisibleReportActionsObjects, lastAction]); const scrollToBottomForCurrentUserAction = useCallback(