[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Android - Track expense - Self DM with actionable whispers auto scrolls down when scrolling up

[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: 1.4.78-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:(https://expensify.testrail.io/index.php?/tests/view/4591846)
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify
2. Go to Self DM (better use a new account with no chat in self DM)
3. Track two or three expenses without dismissing actionable whisper for each expense
4. Scroll up

Expected Result:

Chat will not auto scroll down when scrolling up

Actual Result:

Chat with actionable whispers auto scrolls down when scrolling up

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

Bug6498299_1717227931734.Screen_Recording_20240601_153942_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01853953eae54c5779
• Upwork Job ID: 1797797752162476032
• Last Price Increase: 2024-06-04
• Automatic offers:
• akinwale | Reviewer | 102590743
• c3024 | Contributor | 102590745

Issue Owner

Current Issue Owner: @stephanieelliott

[melvin-bot]

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

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

[lanitochka17]

@stephanieelliott 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

[c3024]

Proposal

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

Scrolling up scrolls back down if the last report action is an unresolved actionable whisper on android.

What is the root cause of that problem?

Here,

App/src/components/FlatList/index.android.tsx

Line 26 in e6c89d9

const onMomentumScrollEnd = useCallback((event: NativeSyntheticEvent<NativeScrollEvent>) => setScrollPosition({offset: event.nativeEvent.contentOffset.y}), []);

and

App/src/components/FlatList/index.android.tsx

Line 39 in e6c89d9

onMomentumScrollEnd={onMomentumScrollEnd}

set the scrollPosition causing rerenders of ReportScreen, children and useReportScrollManager

Here,

App/src/hooks/useReportScrollManager/index.native.ts

Line 11 in d3434cb

const scrollToIndex = (index: number) => {

scrollToIndex is created as a new instance because it is not enclosed with useCallback and this useEffect gets triggered

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

Lines 365 to 373 in d3434cb

	useEffect(() => {
	if (index !== 0 || !isActionableWhisper) {
	return;
	}
	if (prevActionResolution !== (action.originalMessage.resolution ?? null)) {
	reportScrollManager.scrollToIndex(index);
	}
	}, [index, action, prevActionResolution, reportScrollManager, isActionableWhisper]);

As mentioned in the comment, this useEffect's purpose is to scroll to index 0 on iOS when the actionable whisper at index 0 is resolved (because it leaves a blank space as mentioned in the comment without this Effect). The resolution field does not exist on the originalMessage for unresolved whispers that is the originalMessage.resolution is undefined. After resolution, the resolution field will have values of different words like nothing, share etc. This hook compares these two and if they are unequal, it means the whisper is resolved and if the whisper is at index 0 it should be scrolled to to avoid the blank space.

Here for whispers we use the action.originalMessage.resolution for prevActionResolution

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

Line 223 in d3434cb

const prevActionResolution = usePrevious(isActionableWhisper ? action.originalMessage.resolution : null);

and this resolution field does not exist for an unresolved whisper so prevActionResolution is undefined but it is compared with a value with fallback as null here

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

Line 370 in d3434cb

if (prevActionResolution !== (action.originalMessage.resolution ?? null)) {

so though both resolution values here are undefined the inequality does not fail because the fallback value of null is taken for action.originalMessage.resolution on the right hand side because action.originalMessage.resolution is undefined and the inequality succeeds and it scrolls to index 0 though the whisper is not resolved.

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

We should remove the fallback value of null here

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

Line 370 in d3434cb

if (prevActionResolution !== (action.originalMessage.resolution ?? null)) {

then the inequality fails and report is not scrolled to index 0 when the whisper is not resolved.

This useEffect is applicable only for actionable whispers

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

Line 366 in d3434cb

if (index !== 0 || !isActionableWhisper) {

and for actionable whispers prevActionResolution value is taken as undefined with no fallback

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

Line 223 in d3434cb

const prevActionResolution = usePrevious(isActionableWhisper ? action.originalMessage.resolution : null);

So, this dropping of null here

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

Line 370 in d3434cb

if (prevActionResolution !== (action.originalMessage.resolution ?? null)) {

should not break anything.

What alternative solutions did you explore? (Optional)

While the above change fixes the issue, we should also wrap this with a useCallback to ensure it is recreated only when the dependencies (the flatlist ref here) change

App/src/hooks/useReportScrollManager/index.native.ts

Line 11 in d3434cb

const scrollToIndex = (index: number) => {

This change without the above mentioned change (that is removal of null) also fixes the bug.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01853953eae54c5779

[melvin-bot]

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

[akinwale]

We can move forward with @c3024's proposal here.

🎀👀🎀 C+ reviewed.