Skip to content
Merged
49 changes: 37 additions & 12 deletions src/pages/home/report/ReportActionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ function ReportActionsList({
const lastMessageTime = useRef<string | null>(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}`);
const [accountID] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.accountID});
Expand Down Expand Up @@ -443,23 +444,47 @@ function ReportActionsList({
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);

const isNewMessageDisplayed = useMemo(() => {
const prevActions = Object.values(prevSortedVisibleReportActionsObjects);
const lastPrevVisibleAction = prevActions.at(0);
return lastAction?.reportActionID !== lastPrevVisibleAction?.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 || scrollingVerticalOffset.current === 0 || !isReportScreenTopmostCentralPane()) {
return;
}
if (!hasNewestReportActionRef.current) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, do we have any case that needs to run this navigation runAfterInteractions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is to make sure that the scroll occurs after any high-priority interaction in the RN event loop, most likely animation, without it it can lead to some jerky animations or incomplete rendering.

It was already added in the code before that PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right. I mean previously, this navigation is put inside runAfterInteractions, but with our change, it's moved outside of runAfterInteractions. I'm checking if it's safe to move outside.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it by hardcoding reportID to navigate to and I didn't see any issues. Here is an example video:

navigation.mp4

return;
}
if (!isNewMessageDisplayed) {
setPendingBottomScroll(true);
} else {
InteractionManager.runAfterInteractions(() => {
reportScrollManager.scrollToBottom();
});
}
},
[reportScrollManager, report.reportID, isNewMessageDisplayed],
);

useEffect(() => {
if (!pendingBottomScroll || scrollingVerticalOffset.current === 0) {
return;
}

if (isNewMessageDisplayed) {
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 || !isReportScreenTopmostCentralPane()) {
return;
}
if (!hasNewestReportActionRef.current) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID));
return;
}
reportScrollManager.scrollToBottom();
setPendingBottomScroll(false);
});
},
[reportScrollManager, report.reportID],
);
}
}, [pendingBottomScroll, reportScrollManager, isNewMessageDisplayed]);

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,
Expand Down