[HOLD for payment 2023-10-10] [$500] Web - App doesn't re-focus on the text input when clicking on add split button

[kbecciv]

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

---

Action Performed:

1. Go to FAB > Request money
2. Enter a valid amount and continue
3. Click on add split button

Expected Result:

Text input should re-focus

Actual Result:

Text input doesn't re-focus

Workaround:

Unknown

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.73.0
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.21-09-2023.14_29_58.webm

Recording.4716.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695281387919179

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ce294c171ac5fb3a
• Upwork Job ID: 1705603569935224832
• Last Price Increase: 2023-09-23
• Automatic offers:
• jjcoffee | Reviewer | 26933741
• dukenv0307 | Contributor | 26933742

[melvin-bot]

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

[melvin-bot]

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

[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

[melvin-bot]

Triggered auto assignment to @alexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[kbecciv]

Proposal by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695281387919179

Proposal

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

App doesn't re-focus on the text input when clicking on add split button

What is the root cause of that problem?

We add shouldFocusOnSelectRow here

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

Line 290 in 04215cb

shouldFocusOnSelectRow={!Browser.isMobile()}

But when we click on the add split button, we only call this.props.onAddToSelection and don't re-focus the text input

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 380 in 04215cb

onAddToSelection={this.props.onAddToSelection}

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

In BaseOptionsSelector, we should create a function that will be triggered when we click on the add split button. And then in this function, we will call this.props.onAddToSelection function and re-focus the text input.

addToSelection(option) {
    if (this.props.shouldShowTextInput && this.props.shouldFocusOnSelectRow) {
        this.textInput.focus();
        if (this.textInput.isFocused()) {
            setSelection(this.textInput, 0, this.props.value.length);
        }
    }
    this.props.onAddToSelection(option);
}
onAddToSelection={addToSelection}

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 380 in 04215cb

onAddToSelection={this.props.onAddToSelection}

What alternative solutions did you explore? (Optional)

NA

Result

Screencast.from.21-09-2023.14.54.03.webm

[jjcoffee]

Unsure if the expected behaviour here is correct, we're defocusing when selecting Split, so I'm not sure why we'd refocus on the text input 🤔

[dukenv0307]

@jjcoffee In the past, when we clicked on the circle, we would call props.onSelectedRow, and in this function, we will re-focus the text input. After this PR https://github.com/Expensify/App/pull/25564/files is merged when we click on the add split button, we call onSelectedStatePressed which doesn't refocus the text input and I think we should refocus in this function to make the behavior is the same as what we did in the past.

[jjcoffee]

@dukenv0307 Fair enough, I guess it makes more sense if you click split and then want to type to search for another person. Happy to go with your proposal in that case!

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @dukenv0307 🎉 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 📖

[dukenv0307]

@jjcoffee The PR is ready for review.

[melvin-bot]

🎯 ⚡️ Woah @jjcoffee / @dukenv0307, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @dukenv0307 got assigned: 2023-09-29 00:26:21 Z
• when the PR got merged: 2023-10-02 16:08:47 UTC

On to the next one 🚀

[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.3.76-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Refocus when clicking on add split button #28423

If no regressions arise, payment will be issued on 2023-10-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @jjcoffee requires payment offer (Reviewer)
• @dukenv0307 requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

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

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

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - behaviour has always been there as far as I can tell
• 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: N/A
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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 Proposal

1. Go to FAB > Request money
2. Enter any valid amount and continue
3. Click on the split button next to any user
4. Verify that the search input is refocused

Do we agree 👍 or 👎

[jjcoffee]

@sophiepintoraetz Checklist complete!

[melvin-bot]

@tgolen, @jjcoffee, @sophiepintoraetz, @dukenv0307 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[tgolen]

It looks like this issue is waiting on @sophiepintoraetz to wrap up.

[tgolen]

bump @sophiepintoraetz

[sophiepintoraetz]

Sorry, been OOO (flight cancellations has me a bit behind). Created the regression GH here https://github.com/Expensify/Expensify/issues/327015 and have paid @jjcoffee!)