[Hold #50639] [$250] Category - Long press a category&tap outside highlights the category in mWeb & not in hybrid

[IuliiaHerets]

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. 0.53-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N
Issue reported by: Applause Internet Team

Action Performed:

1. Launch site in both mweb and hybrid app
2. Tap profile icon -- Workspaces-- workspace
3. Tap category
4. Long press on a category and tap outside
5. Note the category is highlighted in mweb and not in hybrid app

Expected Result:

Long pressing on a category and tapping outside must show same behavior in mweb and hybrid app.

Actual Result:

Long pressing on a category and tapping outside highlights the category in mweb and not in hybrid app.
Issue reproduced only in redmi devices.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6643788_1729720924313.Screenrecorder-2024-10-24-03-23-11-698_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021853406887958563486
• Upwork Job ID: 1853406887958563486
• Last Price Increase: 2024-11-18

[melvin-bot]

Triggered auto assignment to @trjExpensify (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.

[IuliiaHerets]

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

[trjExpensify]

Issue reproduced only in redmi devices.

Given this, I'm inclined to close this issue. @Expensify/design for a quick hip check before I do.

[dubielzyk-expensify]

Yeah I can't reproduce this

[trjExpensify]

Great stuff, closing then.

[shahinyan11]

@trjExpensify @dubielzyk-expensify The problem is still reproducible on last main

Untitled.mov

[trjExpensify]

@shahinyan11 on a Redmi device only?

[shahinyan11]

@shahinyan11 on a Redmi device only?

On this simulator

[trjExpensify]

Oh hm, @dubielzyk-expensify what Android device did you test?

[dubielzyk-expensify]

Tested it on a Pixel 7

[dubielzyk-expensify]

My bad. I can actually reproduce it now. I'm still not overly convinced we should fix it, but I'll leave that decision to you @trjExpensify

[trjExpensify]

Ah okay, so yeah.. I think we should at least investigate the root cause then if it's even reproducible on a Pixel.

[melvin-bot]

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

[melvin-bot]

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

[c3024]

I will check the proposal and update.

[c3024]

Waiting for valid proposals!

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[QichenZhu]

Proposal

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

After long-pressing a selection list item and dismissing the context menu, the item is highlighted.

What is the root cause of that problem?

The item is focused by long-pressing, but the conditions below detect that the focus is triggered by touch and ignore it, so the item is not highlighted immediately.

App/src/components/SelectionList/index.tsx

Line 48 in e4969b2

shouldIgnoreFocus={Browser.isMobileChrome() && isScreenTouched}

App/src/components/SelectionList/BaseSelectionListItemRenderer.tsx

Lines 77 to 79 in 7256ad6

	if (shouldIgnoreFocus || isDisabled) {
	return;
	}

Instead, it is highlighted only after the context menu is closed, when the focus trap programmatically returns focus to the item.

https://github.com/focus-trap/focus-trap/blob/6c757fa2a90bb818b7f702ba4129d14cdc733f7e/index.js#L769

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

In addition to ignoring focus triggered by touch, we should also ignore programmatic focus.

import * as Browser from '@libs/Browser';

onFocus={(event) => {

App/src/components/SelectionList/BaseSelectionListItemRenderer.tsx

Lines 77 to 79 in 7256ad6

	if (shouldIgnoreFocus || isDisabled) {
	return;
	}

    // Insert this after the above code.
    if (Browser.isMobileChrome() && event && event.nativeEvent && !event.nativeEvent.sourceCapabilities) {
        return;
    }

What alternative solutions did you explore? (Optional)

N/A

[wildan-m]

It has the same root cause with #50639

My proposal for the issue: #50639 (comment)

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[gev990]

Proposal

Note I have noticed a proposal here from another member that does not currently exist, and since I have liked the main concept of that proposal I decided to post a new proposal that may be similar to the proposal that was there previously.

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

Category - Long press a category&tap outside highlights the category in mWeb & not in hybrid

What is the root cause of that problem?

Here we ignore focus when shouldIgnoreFocus is true. And here you can see a description in which cases shouldIgnoreFocus will be true
Now the problem occurs because onFocus event is also triggered after closing modal when the shouldIgnoreFocus is false.

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

Move the ignore focus logic inside BaseListItem component and blur the focused item before return

1. Pass shouldIgnoreFocus as a prop to ListItem component.
2. Add shouldIgnoreFocus in TableListItem component's props and pass following prop shouldIgnoreFocus={shouldIgnoreFocus} to BaseListItem
3. Add shouldIgnoreFocus in BaseListItem component's props and update this onFocus to bellow

onFocus={()=>{
    if(shouldIgnoreFocus || isDisabled) {
        pressableRef.current?.blur()
        return;
    }
    onFocus()
}}

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[trjExpensify]

It has the same root cause with #50639

My proposal for the issue: #50639 (comment)

Given that, @c3024 do you think this is a dupe?

[c3024]

Yes, I think fixing that issue should fix this too. We can hold off this issue for #50639.

[trjExpensify]

Sounds good to me, putting it on hold.

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[melvin-bot]

This issue has not been updated in over 15 days. @trjExpensify, @c3024 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[trjExpensify]

Still held on this PR: #53120

[trjExpensify]

Same as above, Melv.

[trjExpensify]

Same, melv.