[Due for payment 2025-04-23] [$250] mWeb - Reports - With multiple expenses long pressing 1st expense page moves & select option shown

[jponikarchuk]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: V9 1.21-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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): Slottwo1 [email protected]
Issue reported by: Applause Internal Team
Device used: Redminote 10s android 13
App Component: User Settings

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Login with account with many expenses
3. Tap reports
4. Long press the first expense
5. Note expense is moved up and select option is not displayed
6. Try to long press & select the last expense by scrolling down the page
7. Note select option is displayed

Expected Result:

In reports page with multiple expenses, long pressing the 1st expense, deselect option must be displayed without moving up the page.

Actual Result:

In reports page with multiple expenses, long pressing the 1st expense, deselect select option is not shown. Page is moved up and long pressing on second time only select option is displayed.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6784750_1743110346795.Dvnb8264_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021907125974219770626
• Upwork Job ID: 1907125974219770626
• Last Price Increase: 2025-04-01
• Automatic offers:
• linhvovan29546 | Contributor | 106871839

Issue Owner

Current Issue Owner: @lschurr

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

[melvin-bot]

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

[melvin-bot]

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

[linhvovan29546]

🚨 Edited by proposal-police: This proposal was edited at 2025-04-02 08:48:14 UTC.

Proposal

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

mWeb - Reports - With multiple expenses long pressing 1st expense page moves & select option shown

What is the root cause of that problem?

When long-pressing an item on the search page, the onFocus event is triggered, calling setFocusedIndex(). This causes onFocusedIndexChange to fire and invoke scrollToIndex, leading to an unintended scroll when the selection modal appears.

App/src/components/Search/SearchList.tsx

Lines 205 to 207 in c3451e1

	onFocusedIndexChange: (index: number) => {
	scrollToIndex(index);
	},

We had logic in SelectionList to prevent this issue on mobile using isMobileChrome(), but in PR #57549, we migrated to FlatList and missed that logic, which led to the issue in the OP.

App/src/components/SelectionList/index.tsx

Lines 74 to 76 in c3451e1

	// Ignore the focus if it's caused by a touch event on mobile chrome.
	// For example, a long press will trigger a focus event on mobile chrome.
	shouldIgnoreFocus={isMobileChrome() && isScreenTouched}

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

We can bring back the missing logic to SearchList and only apply for web platform:

App/src/components/Search/SearchList.tsx

Line 118 in c3451e1

   const [isScreenTouched, setIsScreenTouched] = useState(false);
    
        const touchStart = () => setIsScreenTouched(true);
        const touchEnd = () => setIsScreenTouched(false);

           const shouldIgnoreFocus= isMobileChrome() && isScreenTouched
           useEffect(() => {
            if (!canUseTouchScreen()) {
                return;
            }
    
            // We're setting `isScreenTouched` in this listener only for web platforms with touchscreen (mWeb) where
            // we want to dismiss the keyboard only when the list is scrolled by the user and not when it's scrolled programmatically.
            document.addEventListener('touchstart', touchStart);
            document.addEventListener('touchend', touchEnd);
    
            return () => {
                document.removeEventListener('touchstart', touchStart);
                document.removeEventListener('touchend', touchEnd);
            };
        }, []);
const shouldIgnoreFocus=isMobileChrome() && isScreenTouched

Then, in the onFocus event, we return early if shouldIgnoreFocus is true:

App/src/components/Search/SearchList.tsx

Lines 282 to 288 in c3451e1

	onFocus={(event: NativeSyntheticEvent<ExtendedTargetedEvent>) => {
	// Prevent unexpected scrolling on mobile Chrome after the context menu closes by ignoring programmatic focus not triggered by direct user interaction.
	if (isMobileChrome() && event.nativeEvent && !event.nativeEvent.sourceCapabilities) {
	return;
	}
	setFocusedIndex(index);
	}}

                    onFocus={(event: NativeSyntheticEvent<ExtendedTargetedEvent>) => {
                        if (shouldIgnoreFocus) {
                            return; 
                        }
... other
                    }}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None – this is a UI bug specific to mobile Chrome.

What alternative solutions did you explore? (Optional)

We could simplify the solution like this (based on firesTouchEvents)

                    onFocus={(event: NativeSyntheticEvent<ExtendedTargetedEvent>) => {
                        // Prevent unexpected scrolling on mobile Chrome after the context menu closes by ignoring programmatic focus not triggered by direct user interaction.
                        if (isMobileChrome() && event.nativeEvent) {
                            if( !event.nativeEvent.sourceCapabilities){
                                return
                            }
                            if(event.nativeEvent.sourceCapabilities.firesTouchEvents){
                                return
                            }
                        }
                        setFocusedIndex(index);
                    }}

[ntdiary]

@linhvovan29546, your RCA LGTM, and I feel we could simplify the solution like this (based on firesTouchEvents)

[linhvovan29546]

@ntdiary Thank you for the suggestion! That works well. I've updated the proposal to include alternative solutions #59420 (comment)

[ntdiary]

@linhvovan29546, are you saying the focus event isn't triggered by keyboard navigation? The changes are limited to the onFocus callback, so not sure how this might affect keyboard navigation, can you please share a demo video?

[linhvovan29546]

@linhvovan29546, are you saying the focus event isn't triggered by keyboard navigation? The changes are limited to the onFocus callback, so not sure how this might affect keyboard navigation, can you please share a demo video?

@ntdiary I've retested it, and it works well. Please ignore my first message 😊

[linhvovan29546]

so not sure how this might affect keyboard navigation, can you please share a demo video?

That is a different issue because I can reproduce that on the web as well (though not consistently) without your code.

Screen.Recording.2025-04-04.at.17.34.20.mp4

[ntdiary]

@linhvovan29546 's proposal LGTM, and I think it's fine to use a simple firesTouchEvents check here.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[lschurr]

Bump on this one @thienlnam

[melvin-bot]

@thienlnam Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

📣 @linhvovan29546 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[linhvovan29546]

The PR is ready!
cc @ntdiary

[linhvovan29546]

FYI: The PR was deployed to production 4 days ago, and the due date for payment is 2025-04-23.

[lschurr]

Looks like the automation didn't work. Thanks @linhvovan29546

[ntdiary]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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/57549/files#r2056196736

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
  No need, as we should already have a regression test

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[mallenexpensify]

Contributor: @linhvovan29546 paid $250 via Upwork
Contributor+: @ntdiary due $250 via NewDot

@ntdiary can you propose the regression test steps plz

[ntdiary]

No need, as we should already have a regression test

@ntdiary can you propose the regression test steps plz

@mallenexpensify, do you think it's better to create a new regression test? I noticed there's already a similar one earlier, It's actually the same, just described differently. 😂

[lschurr]

I think we can close if we already have a regression test for this.

[JmillsExpensify]

$250 approved for @ntdiary