[HOLD for payment 2023-10-23][$2000] Choosing group members change the search input and it's label style

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1.Click on the FAB and create New group
2.Start adding the group members and notice the search input and it's label

Expected Result:

Choosing group members shouldn't change the search input and it's label style for better UX.

Actual Result:

Every time we choose a member we change the search input and it's label, because of the search input refocus

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.40-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screencast.from.13-07-23.05_23_15.webm

Recording.1268.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Ahmed-Abdella
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689216766939829

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f1e32514bd8c7aa9
• Upwork Job ID: 1681446377913671680
• Last Price Increase: 2023-08-07
• Automatic offers:
• situchan | Reviewer | 27097976
• s-alves10 | Contributor | 27097978
• Ahmed-Abdella | Reporter | 27097981

[melvin-bot]

Triggered auto assignment to @michaelhaxhiu (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[s-alves10]

Proposal

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

Search input loses focus when toggling members in new chat page and split bill page unlike as on native platforms

What is the root cause of that problem?

When user selects the members, search input lose the focus, and the focus is set here again

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 347 in 7f063c7

this.textInput.focus();

App/src/components/SelectionList/BaseSelectionList.js

Line 215 in 98f3b43

textInputRef.current.focus();

App/src/components/SelectionList/BaseSelectionList.js

Line 222 in 98f3b43

textInputRef.current.focus();

Refocusing is the root cause

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

We can prevent the lost of focus by preventing default action of mouse events

1. Add the new props called shouldTakeFocus with default value true to the BaseOptionsList, OptionRow, and BaseListItem components
2. Pass the shouldTakeFocus props down OptionsList -> OptionRow

    shouldTakeFocus={props.shouldTakeFocus}

3. Add the following code above this line and this line

    onMouseDown={this.props.shouldTakeFocus ? undefined : (e) => e.preventDefault()}

4. Pass shouldTakeFocus={!this.props.shouldFocusOnSelectRow} props to OptionsList in BaseOptionsSelector
5. Pass shouldTakeFocus={!shouldFocusOnSelectRow} props to BaseListItem in BaseSelectionList. And add the following props to select all button in BaseSelectionList

        onMouseDown={shouldFocusOnSelectRow ? (e) => e.preventDefault() : undefined}

This works as expected.

Note: We disabled the focusing on selection for mobile browsers intentionally

App/src/pages/NewChatPage.js

Line 201 in 7f063c7

shouldFocusOnSelectRow={!Browser.isMobile()}

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsSelector.js

Line 271 in 7f063c7

shouldFocusOnSelectRow={!Browser.isMobile()}

App/src/pages/workspace/WorkspaceMembersPage.js

Line 420 in 98f3b43

shouldFocusOnSelectRow={!Browser.isMobile()}

I'm keeping them as is for now

Web

22812_mac_chrome.mp4

iOS

22812_ios_native.mp4

Android

22812_android_native.mp4

Desktop

22812_mac_desktop.mp4

What alternative solutions did you explore? (Optional)

[Ahmed-Abdella]

Also happens in split bill
Screencast from 15-07-23 05:02:59.webm

[melvin-bot]

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

[michaelhaxhiu]

Reproduced easily. And it has the same effect on desktop, so I'm gonna check that box in the Platforms: section of the GH.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[michaelhaxhiu]

Lol why does it say I added the Overdue label back... Melvin you lie.

[Talha345]

What is the root cause of that problem?

Your RCA is correct but only onMouseDown will suffice.

[s-alves10]

@Talha345
Maybe. But it might cause other issues as in #21727

[melvin-bot]

@michaelhaxhiu, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[melvin-bot]

@michaelhaxhiu, @0xmiroslav Still overdue 6 days?! Let's take care of this!

[michaelhaxhiu]

@0xmiroslav can you please review the proposal provided?

[michaelhaxhiu]

If none are applicable, I will double the price of this job.

[MonilBhavsar]

Bumping to weekly as payment is due next week

[bfitzexpensify]

Flicking this one back to daily so that it's in the right spot in my k2 dashboard

[bfitzexpensify]

Due 10/23

[MonilBhavsar]

Payment is due today. @situchan could you please take a look at the checklist

[MonilBhavsar]

False comment, Melvin

[sonialiap]

@situchan review + bonus = $3000 - paid ✔️ #22812 (comment)
@s-alves10 fix + bonus = $3000 - paid ✔️
@Ahmed-Abdella report = $250 - paid ✔️

[sonialiap]

@situchan last step is to do the checklist. I scrolled through the issue and I don't think it's been filled out yet but let me know if I missed it

[MonilBhavsar]

@situchan could you please take a look at the checklist. Thanks!

[situchan]

There were several changes on this behavior but the original root cause existed from the beginning as it's web's default behavior to blur element when another one is clicked. So no regression.

This is minor UI issue so I don't think it's worth adding to regression test which should run on every deploy.
And the refactor will not likely to happen again so the fix won't be reverted.

[MonilBhavsar]

I would say we should add regression test, as we usually update callback functions with lists and could probably cause regression

[situchan]

Regression Test Proposal

Case 1:

1. Go to FAB > Start chat
2. Focus on search input if not focused
3. Select multiple users by tapping "Add to group" button
4. Deselect users again by tapping ✅ button
5. Verify that the style of search input and label doesn't change

Case 2:

1. Go to FAB > Request money
2. Input any amount in manual tab and tap Next
3. Focus on search input if not focused
4. Select multiple users by tapping "Split" button
5. Deselect users again by tapping ✅ button
6. Verify that the style of search input and label doesn't change

Case 3:

1. Go to Settings > Workspaces > any workspace > Members
2. Focus on search input if not focused
3. Select/deselect users by tapping on each row or its ✅
4. Verify that the style of search input and label doesn't change
5. Click on Invite button to go Invite members page
6. Select/deselect users by tapping on each row or its ✅
7. Verify that the style of search input and label doesn't change

[MonilBhavsar]

And also Workspace settings > Members page please

[situchan]

Updated.

[melvin-bot]

@sonialiap, @MonilBhavsar, @bfitzexpensify, @situchan, @s-alves10 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sonialiap]

New regression test addition submitted