[HOLD for payment 2024-07-10] [$250] Chat does not scroll to the last message when a new message received

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.82-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @youssef-lr
Slack conversation:

Action Performed:

1. Go to a report with lots of messages
2. Send a message to user B

Expected Result:

For user B the chat scrolls to the last message received

Actual Result:

Does not scroll

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Recording.205.mp4

Screen.Recording.2024-06-11.at.21.37.40.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0178751bd086a2062a
• Upwork Job ID: 1802672136974977044
• Last Price Increase: 2024-06-17
• Automatic offers:
• s77rt | Reviewer | 102816257
• tsa321 | Contributor | 102816260

Issue Owner

Current Issue Owner: @bfitzexpensify

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[tsa321]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Chat does not scroll to the last message when a new message received

What is the root cause of that problem?

When user receive new message, the pushJSON data contains onyx update for report and the new reportAction.
The report onyx data will update lastVisibleAcitonCreated and reportAction will add new latest reportAction.
In here:

App/src/pages/home/report/ReportActionsView.js

Line 215 in a5d0c4d

const hasNewestReportAction = lodashGet(reportActions[0], 'created') === props.report.lastVisibleActionCreated;

when onyx update on report data the hasNewestReportAction will be false because the reportActions isn't updated yet, then the onyx will update reportActions which will make hasNewestReportAction to be true. So there is toggling false then true after the complete update of data from push event.

The hasNewestReportAction will affect enableAutoscrollToTopThreshold value, which is the scrolling logic depends on.

In here:

App/src/components/FlatList/MVCPFlatList.js

Lines 101 to 102 in a5d0c4d

	if (mvcpAutoscrollToTopThreshold != null && scrollOffset <= mvcpAutoscrollToTopThreshold) {
	scrollToOffset(0, true);

when the pushData update completed, the mvcpAutoscrollToTopThreshold value used in here isn't the latest mvcpAutoscrollToTopThreshold. the value is still undefined because of the update from report onyx data and hasn't been updated after reportAction onyx merged. So when the delta value is higher 0.5 / new message inserted in list, the value of mvcpAutoscrollToTopThreshold hasn't been updated.

Since the `adjustForMaintainVisibleContentPosition` is used in subscriber of `MutationObserver`, we need the updated value right at when message is inserted into the list. So we need the latest value and using ref here can solve the issue. If we are using useCallback that depend on the `mvcpAutoscrollToTopThreshold` the new subscriber will be created and will disturb detection in `mutationObserver` when observing the list content difference. Sometimes I noticed, when new subscriber based on new callback produced by useCallback, the list is already changes and delta difference is still 0. So we must remove the dependency of `mvcpAutoscrollToTopThreshold` in `adjustForMaintainVisibleContentPosition` and use ref of mvcpAutoscrollToTopThreshold.

What changes do you think we should make in order to solve the problem?

We must remove the dependency of mvcpAutoscrollToTopThreshold in adjustForMaintainVisibleContentPosition and we could use ref of mvcpAutoscrollToTopThreshold at the top of the function component and immediately update it when new mvcpAutoscrollToTopThreshold available, then use it inside adjustForMaintainVisibleContentPosition

The code could be:

const mvcpAutoscrollToTopThresholdRef = useRef(mvcpAutoscrollToTopThreshold);
mvcpAutoscrollToTopThresholdRef.current = mvcpAutoscrollToTopThreshold;
....
// then in adjustForMaintainVisibleContentPosition:
if (mvcpAutoscrollToTopThresholdRef.current != null && scrollOffset <= mvcpAutoscrollToTopThresholdRef.current) {
     scrollToOffset(0, true);
}

and we must remove mvcpAutoscrollToTopThreshold in :

App/src/components/FlatList/index.tsx

Line 112 in f838795

}, [getScrollOffset, scrollToOffset, mvcpMinIndexForVisible, mvcpAutoscrollToTopThreshold, horizontal]);

Other alternative is (workaround):

We could use useMemo to avoid unnecessary value changes on hasNewestReportAction when the onyx pushJSON data isn't fully merged yet.

In here, the code could be:

const hasNewestReportAction = useMemo(() => lodashGet(reportActions[0], 'created') === props.report.lastVisibleActionCreated, [reportActions.length]);

So when new report action is updated, the shouldEnableAutoScroll that depend on hasNewestReportAction will keep on true and the List will scroll.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0178751bd086a2062a

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

[s77rt]

I'm not able to reproduce the bug. @tsa321 Is this still reproducible for you?

Screen.Recording.2024-06-17.at.11.50.57.PM.mov

[tsa321]

@s77rt yes, I can still reprdouce it. You must scroll up to 1 or 2 messages above before receiving new message.

macos-web-d.mp4