mWeb - Chat scrolls back to linked message after receiving new message

[lanitochka17]

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: 9.0.5.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: N/A
Email or phone of affected tester (no customers): u.onyeukwu94@gmail.com
Issue reported by: Applause - Internal Team

Issue found when executing PR #44819

Action Performed:

1. Open a chat conversation with a user(B) that has scrollable history
2. Go to the top comment and copy the link to that comment
3. Open another chat, send link and tap link to navigate to comment
4. Scroll to bottom to chat conversation
5. With User B, send any message to chat

Expected Result:

User receives new message smoothly

Actual Result:

Chat conversation scrolls up to highlighted message that was navigated to initially

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

Add any screenshot/video evidence

Bug6536085_1720456321183.Bug_.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @

[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.

[lanitochka17]

@bfitzexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[jacobkim9881]

I guess this came from #41962

[ishpaul777]

I guess this came from #41962

PR is not on Prod. yet this issue is also reproducable on prod. so not from #41962

[tsa321]

Proposal

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

Chat scrolls back to linked message after receiving new message

What is the root cause of that problem?

Here:

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

Lines 141 to 143 in c973e62

	useLayoutEffect(() => {
	setCurrentReportActionID('');
	}, [route]);

and here:

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

Lines 146 to 157 in c973e62

	const listID = useMemo(() => {
	if (!reportActionID) {
	// Keep the old list ID since we're not in the Comment Linking flow
	return listOldID;
	}
	isFirstLinkedActionRender.current = true;
	const newID = generateNewRandomInt(listOldID, 1, Number.MAX_SAFE_INTEGER);
	// eslint-disable-next-line react-compiler/react-compiler
	listOldID = newID;
	return newID;
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
	}, [route, isLoadingInitialReportActions, reportActionID]);

We the useMemo and useLayoutEffect rely on route, which comes from useRoute. Every time the user receives a new message, the route object changes and triggers both useMemo and useLayoutEffect. Inside useMemo, isFirstLinkedActionRender.current is set to true, causing the report to scroll to the linked message.

The changes to the route object/navigation state occur due to:

App/src/pages/home/report/ReportActionsList.tsx

Line 271 in 4c235a7

Navigation.setParams({referrer: undefined});

The Navigation.setParams({ referrer: undefined }) call alters the navigation state and updates the route object.

What changes should we make to solve the problem?

To address this issue, we should add a check before executing Navigation.setParams({ referrer: undefined }). For example:

if (isFromNotification) {
    Navigation.setParams({ referrer: undefined });
}

Alternative solution:

Another approach could be to adjust the dependencies of useMemo and useLayoutEffect to use route.path instead of route.

[ishpaul777]

i am interested in taking over this issue as c+, i have good context working on comment-linking related issues. : )

[melvin-bot]

@bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

[bfitzexpensify]

Cool, assigning you @ishpaul777

[melvin-bot]

Current assignee @ishpaul777 is eligible for the External assigner, not assigning anyone new.

[ishpaul777]

@tsa321's Proposal looks good to me and test well.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @yuwenmemon, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[tsa321]

@ishpaul777 PR is ready.

[ishpaul777]

fix for this was deployed to Prod yesterday.

[tsa321]

cc: @bfitzexpensify . I think melvin automation broken here.

[tsa321]

^^ cc @bfitzexpensify

[bfitzexpensify]

@tsa321 can you please link your Upwork profile?

[tsa321]

@bfitzexpensify my upwork profile: https://www.upwork.com/freelancers/~01bfc26d267bade652

[bfitzexpensify]

Offer sent @tsa321.

I'm now out of office, so adding a BZ buddy to finalise the payout

@ishpaul777 can you complete the BZ checklist:

• The PR that introduced the bug has been identified. Link to the PR:
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• Determine if we should create a regression test for this bug.
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Triggered auto assignment to @lschurr (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]

@bfitzexpensify I have accepted the offer.

[ishpaul777]

• The PR that introduced the bug has been identified. Link to the PR: Fix mark message from notification #41484

• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/41484/files#r1727608316

• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: not required

• Determine if we should create a regression test for this bug. Not requires

• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. N.A