[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] CRITICAL: [UX Reliability] [$1000] Chat - Chat does not scroll down when sending messages

[kbecciv]

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.64-0
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify app.
2. Go to empty chat.
3. Send a text.

Expected Result:

The chat view will scroll down to show the latest message.

Actual Result:

The chat view does not scroll down to show the latest message.

Workaround:

n/a

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

Bug6457776_1713809495909.RPReplay_Final1713809407.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016dcc2d8605cc9198
• Upwork Job ID: 1782503029072748544
• Last Price Increase: 2024-05-30
• Automatic offers:
• DylanDylann | Reviewer | 0
• ikevin127 | Contributor | 0
• nkdengineer | Contributor | 102574769

Issue Owner

Current Issue Owner: @anmurali

[melvin-bot]

Triggered auto assignment to @luacmartins (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kbecciv]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016dcc2d8605cc9198

[melvin-bot]

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

[mountiny]

Looking for proposals

[luacmartins]

Gonna look into this one

[mountiny]

it might be related to #40547

[josh-prof]

I found the issue, and looking for possible solutions.

[luacmartins]

Cool, let us know what you find @climax0916

[josh-prof]

When adding comments, shouldEnableAutoScroll turns to False so ReportActionsList doesn't scroll to the latest message.

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

Lines 488 to 512 in a43595b

	const shouldEnableAutoScroll = hasNewestReportAction && (!reportActionID || !isNavigatingToLinkedMessage);
	return (
	<>
	<ReportActionsList
	report={report}
	transactionThreadReport={transactionThreadReport}
	reportActions={reportActions}
	parentReportAction={parentReportAction}
	parentReportActionForTransactionThread={parentReportActionForTransactionThread}
	onLayout={recordTimeToMeasureItemLayout}
	sortedReportActions={reportActionsToDisplay}
	mostRecentIOUReportActionID={mostRecentIOUReportActionID}
	loadOlderChats={loadOlderChats}
	loadNewerChats={loadNewerChats}
	isLoadingInitialReportActions={isLoadingInitialReportActions}
	isLoadingOlderReportActions={isLoadingOlderReportActions}
	isLoadingNewerReportActions={isLoadingNewerReportActions}
	listID={listID}
	onContentSizeChange={onContentSizeChange}
	shouldEnableAutoScrollToTopThreshold={shouldEnableAutoScroll}
	/>
	<UserTypingEventListener report={report} />
	<PopoverReactionList ref={reactionListRef} />
	</>
	);

[josh-prof]

@luacmartins Am I right?

[luacmartins]

Why does it become false?

[josh-prof]

When adding new transactions, this API.write() function is called:

App/src/libs/actions/Report.ts

Lines 604 to 608 in a43595b

	API.write(commandName, parameters, {
	optimisticData,
	successData,
	failureData,
	});

Then, Onyx.update(optimisticData) is called for update Onyx storage:

App/src/libs/API/index.ts

Lines 56 to 63 in a43595b

	Log.info('Called API write', false, {command, ...apiCommandParameters});
	const {optimisticData, ...onyxDataWithoutOptimisticData} = onyxData;
	// Optimistically update Onyx
	if (optimisticData) {
	Onyx.update(optimisticData);
	}

After that, render again because of Onyx storage changed. But, here hasNewestReportAction will be False:

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

Line 235 in a43595b

const hasNewestReportAction = reportActions[0]?.created === report.lastVisibleActionCreated || reportActions[0]?.created === transactionThreadReport?.lastVisibleActionCreated;

Because of this, the scroll doesn't down to the latest message.

[DylanDylann]

I think this issue have the same RCA with #40664

[ikevin127]

Proposal

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

Chat does not scroll down when sending messages.

What is the root cause of that problem?

Two things which both cause our issue:

1.

Whenever we start a new chat there's this new action which I assume was introduced by BE recently: an additional new ADDCOMMENT action comes in right after the "CREATED" action, which is an automated message (one in the middle):

This addition causes hasNewestReportAction to return false since sortedReportActions?.[0] is now the automated ADDCOMMENT action (first in sortedReportActions) instead of the user posted message like it used to be -> meaning that the the automated message's created date is not equal to the report's lastVisibleActionCreated date because the lastVisibleActionCreated date equals to the sortedReportActions?.[1] at index 1 (user's comment) hence the condition returns false.

This makes it such that the scrollToBottomForCurrentUserAction returns early because of the !hasNewestReportActionRef.current check being true.

2.

Due to recent new architecture being enabled, it looks like the native specific scrollToBottom function does not trigger the scrolling anymore even though it's called and this happens because there's a few things going on when we post a comment from JS's point of view and because scrollToBottom gets called while the stack is ongoing (not cleared yet), the function does not perform it's logic as expected.

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

We have a two part solution to address both parts of the RCA:

1.

The first part of the RCA was already handled by PR #40286, specifically this line where we make sure we don't early return when the user posts their first comment when the first action is a whisper (automated comment).

2.
In order to have the scrollToBottom function work as expected, similar to what I proposed in issue #38855 and was selected / approved -> use deferred execution via requestAnimationFrame to wrap the contents of the function like so:

    const scrollToBottom = useCallback(() => {
        requestAnimationFrame(() => {
            if (!flatListRef?.current) {
                return;
            }
    
            setScrollPosition({offset: 0});
    
            flatListRef.current?.scrollToOffset({animated: false, offset: 0});
        });
    }, [flatListRef, setScrollPosition]);

This ensures that the code within scrollToBottom will be executed on the next tick, essentially telling the JavaScript runtime to execute this code after the current call stack has cleared.

Here's a test branch of the solution: test/40724 and it's diff.

Videos (before / after)

iOS: Native

Before	After
before.mov	after.mov

[mountiny]

Based on the previous comments this seems like it might not be a deploy blocker as if its related to the new architecture, that has already been deployed to production

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix chat doesn't scroll to bottom #43021

If no regressions arise, payment will be issued on 2024-06-18. 🎊

For reference, here are some details about the assignees on this issue:

• @parasharrajat requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat] The PR that introduced the bug has been identified. Link to the PR:
• [@parasharrajat] 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:
• [@parasharrajat] 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:
• [@parasharrajat] Determine if we should create a regression test for this bug.
• [@parasharrajat] 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.
• [@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix chat doesn't scroll to bottom #43021

If no regressions arise, payment will be issued on 2024-06-20. 🎊

For reference, here are some details about the assignees on this issue:

• @parasharrajat requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat] The PR that introduced the bug has been identified. Link to the PR:
• [@parasharrajat] 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:
• [@parasharrajat] 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:
• [@parasharrajat] Determine if we should create a regression test for this bug.
• [@parasharrajat] 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.
• [@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[parasharrajat]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat] The PR that introduced the bug has been identified. Link to the PR: Tasks for guided setup #39951
• [@parasharrajat] 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/39951/files#r1647084530
• [@parasharrajat] 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 needed
• [@parasharrajat] Determine if we should create a regression test for this bug. Yes
• [@parasharrajat] 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.

Regression Test Steps

For attachments, text messages, For whispers.

1. Open a DM chat or WS chat that has long history chat
2. Send an attachments
3. Verify that the chat scrolls to the bottom
4. Scroll to the top and send a message
5. Verify that the chat scrolls to the bottom
6. Create a split expense
7. Verify that the chat scrolls to the bottom

For IOU reports, For IOU previews.

1. Open a IOU report
2. Send a message
3. Verify that the chat scrolls to the bottom
4. Submit an expense
5. Verify that the chat scrolls to the bottom
6. For thread replies
7. Open a thread with long history chat
8. Scroll to the top and send a message
9. Verify that the chat scrolls to the bottom

Do you agree 👍 or 👎 ?

[luacmartins]

IMO we can skip TC in this case given that's such an obvious issue that many people would report it. I'm not too passionate about it though so also ok adding it.

[anmurali]

Final payment summary

• @nkdengineer $1000 via Upwork
• @parasharrajat request $1000 via ND approved

[parasharrajat]

Payment requested as per #40724 (comment)

[JmillsExpensify]

$1,000 approved for @parasharrajat

[m-natarajan]

Reopening as per this https://expensify.slack.com/archives/C049HHMV9SM/p1720444720601389

[anmurali]

Closing this again as per that thread we agreed to create a new issue.