[HOLD for payment 2023-07-14] [$1000] Placeholder not moving back to it's place after selecting a contact

[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 FAB
2. Click on "Split bill"
3. Type an email and select the contact that shows up in the below suggestion
4. Notice the placeholder text "Name, email or phone number"

Expected Result:

The placeholder text should return back to it's place after user selects on a contact

Actual Result:

Placeholder stays in typing state and doesn't return back

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
• [x] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3.16.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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/43996225/9f767ec5-2b7d-4295-9d53-cd50a5e1f51f

https://github.com/Expensify/App/assets/43996225/802d475f-c737-4743-b533-30b44ccff1ec

Expensify/Expensify Issue URL: Issue reported by: @nathan-mulugeta Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1684261779675679

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0196a89d054bb9c9e0
• Upwork Job ID: 1660771960364109824
• Last Price Increase: 2023-06-12

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] 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
• [x] 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.
• [x] 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.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[PrashantMangukiya]

Proposal

Posting proposal early as per new guidelines

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

Placeholder not moving back to it's place after selecting a contact. i.e. Placeholder not focus after selecting contact.

What is the root cause of that problem?

During Split Bill it it rendering participant selection list via this https://github.com/Expensify/App/blob/78ec5316c36c380ab64041a4b87144ae02cf80f3/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsSplitSelector.js#L218-L233

OptionsSelector has shouldFocusOnSelectRow prop is there with default value false. It can be used to set focus to text input after an option is selected. https://github.com/Expensify/App/blob/78ec5316c36c380ab64041a4b87144ae02cf80f3/src/components/OptionsSelector/optionsSelectorPropTypes.js#L71-L72

Within MoneyRequestParticipantsSplitSelector.js we are not passing shouldFocusOnSelectRow props to OptionSelector at line 218, so it will not set focus to text input after option is selected. This is the root cause of the problem.

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

Within MoneyRequestParticipantsSplitSelector.js we have to pass shouldFocusOnSelectRow to OptionSelector (starts from line 218) as shown code below. So it will focus to text input after option is selected.

 <OptionsSelector 
     ....
     shouldFocusOnSelectRow        // *** Add this
 /> 

This will solve the issue as shown in result video.

What alternative solutions did you explore? (Optional)

None

Result

https://github.com/Expensify/App/assets/7823358/83678391-fb6f-4a56-a9d3-f9418cbb932b

[strepanier03]

I am able to recreate this behavior, but something odd also happened.

When testing, I recreated the behavior right away. But then I stepped away and left my screen open on the contact select screen where this issue was happening.

When I came back 5-10 minutes later, the behavior didn't happen any longer. The placeholder went back to it's place every time and very quickly. Now, I can't recreate the issue again.

I'm going to pause until tomorrow and then move this forward then if I can recreate.

[strepanier03]

Tested this again this morning and I the behavior hasn't returned.

@kavimuru, can you still recreate this behavior? I can no longer recreate it. How about you @Nathan-Mulugeta??

[Nathan-Mulugeta]

Yes @strepanier03, I am still able to reproduce this. Here are some things to notice. After clicking split bill from the FAB menu instead of selecting the contact lists from the recent list, type out a new email to add to the split bill money request. After typing the email in the input field select the email that shows up. Then you can notice the place holder doesn't return back to its place. I will attach a video demonestrating this.

https://github.com/Expensify/App/assets/111440031/57b02fa2-645b-493c-80dc-cabfeda6f801

[bernhardoj]

Proposal

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

Text input label does not deactivate (move back as the placeholder) when we clear the input value. To reproduce it, make sure type something first.

What is the root cause of that problem?

Actually, we already have the code that SHOULD deactivate the label when we clear the input value here: https://github.com/Expensify/App/blob/737862677811acc2b05c68209a56ecd58bee59f3/src/components/TextInput/BaseTextInput.js#L71-L89

As you can see, we update the value state with inputValue and call deactivateLabel if inputValue is empty. Then, inside deactivateLabel, we check if the state of value is empty or not. https://github.com/Expensify/App/blob/737862677811acc2b05c68209a56ecd58bee59f3/src/components/TextInput/BaseTextInput.js#L164-L171

If it's empty, then we proceed to deactivate the label. The problem is, the value state is not updated yet inside deactivateLabel, which is basically a race condition.

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

Call the deactivateLabel inside the setState callback.

More specific, move this code to be inside the setState callback. https://github.com/Expensify/App/blob/737862677811acc2b05c68209a56ecd58bee59f3/src/components/TextInput/BaseTextInput.js#L85-L89

I guess we also can change inputValue to this.state.value instead.

Result

https://github.com/Expensify/App/assets/50919443/1ddc61e5-9f65-465c-a202-9fe608ea0f3e

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[strepanier03]

Thank you for the extra details @Nathan-Mulugeta! Moved this on.

[dukenv0307]

Proposal

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

Placeholder stays in typing state and doesn't return back after selecting a contact.

What is the root cause of that problem?

There're 2 problems:

1. First, the expected result might not be enough. Here's what happens when a contact is selected in Android/iOS:

https://github.com/Expensify/App/assets/129500732/fb33e2da-f0aa-4917-9402-3192cec6234a

As we can see, on native platforms, when a contact is selected, the search field will remain focused. The same should happen for web as well for consistency, while currently for web the search input will be blurred. Also there's no reason we should blur the input after selecting a contact, because the user might want to search and add a new contact immediately.

Also worthy to note that the search field will remain focused is different from the search field will be blurred then focused again which is what the shouldFocusOnSelectRow is for. the search field will be blurred then focused again will make the input blink when selecting contact, also it's different UX compared to Android/iOS.

So the expectation should be "the search input should remain focused on Web after selecting a contact" (currently it's like that for Android/iOS).

The root cause is that currently we're not supporting this behavior.

2. 1. should be enough to fix this issue, but there's a small issue in here https://github.com/Expensify/App/blob/1aa30d1ecfa8a69275b49f051ad65c392ae4e985/src/components/TextInput/BaseTextInput.js#L88 which might cause problems in the future. In there the deactivateLabel is called if the inputValue becomes empty, inside deactivateLabel itself we're using the state.value here https://github.com/Expensify/App/blob/1aa30d1ecfa8a69275b49f051ad65c392ae4e985/src/components/TextInput/BaseTextInput.js#L165. It causes race condition where inputValue becomes empty but the state.value doesn't update to empty yet inside deactivateLabel, causing the label not to be deactivated and moved back to its place properly.

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

1. We need to listen to mouse down event of the option, and preventDefault if the TextInput is currently focused, so that it will not blur the TextInput on selecting option.

We already use the same approach to make sure the Composer is not blurred when pressing the send button here.

Steps: a. Pass a new onRowMouseDown to <OptionsList here

onRowMouseDown={(e) => {
    if (!e || !this.textInput.isFocused()) {
        return;
    }

    e.preventDefault();
}}

b. Pass onRowMouseDown={this.props.onRowMouseDown} to OptionRow, then in OptionRow here https://github.com/Expensify/App/blob/1ac9964003fcb61d6107c205ce7573d67e170ec3/src/components/OptionRow.js#L151, assign the onRowMouseDown to the onMouseDown:

onMouseDown={this.props.onRowMouseDown}

2. To fix this we should make sure the deactivateLabel call here happens inside this setState callback

What alternative solutions did you explore? (Optional)

In 1, we can optionally introduce a variable to OptionsSelector to control if we want to disable the behavior "the search input should remain focused on Web after selecting a contact" in some cases

[strepanier03]

@sobitneupane - Friendly bump for the proposal submitted yesterday. Thank you!

[sobitneupane]

I believe the behavior of OptionsSelector in split bill page should be similar to that of OptionSelector in New group page.

https://github.com/Expensify/App/assets/25876548/0cf71b83-fee5-4d61-af12-fe4171f29ebd

In New group page,

• Text Input refocuses on select row with input text selected.

@strepanier03 @tylerkaraszewski Do we want similar behavior in Split bill page as well?

[dukenv0307]

@sobitneupane @tylerkaraszewski just to note both Split bill page and New group page behavior after selecting the option in desktop are inconsistent with that same page in mobile. In mobile the input will not be blurred (blinked) after the option is selected. We can make all those pages consistent with each other and also platforms to be consistent as well.

[melvin-bot[bot]]

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

[strepanier03]

@dukenv0307 - Looks like Sobit left a comment on your proposal.

[sobitneupane]

@strepanier03 @tylerkaraszewski Do we agree on these points?

[bernhardoj]

Giving my opinion.

I think it's better for us to solve the initial reported bug first, rather than covering it up by refocusing the text input.

[dukenv0307]

@dukenv0307 - Looks like Sobit left a comment on your proposal.

@strepanier03 sorry I don't see any comment from @sobitneupane on my proposal, am I missing something?

[strepanier03]

I think we need to raise this for discussion on the best path forward so I'll do that first thing tomorrow when more of the team is online.

[melvin-bot[bot]]

@tylerkaraszewski @strepanier03 @sobitneupane this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

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

[tylerkaraszewski]

@strepanier03 - do you have a slack thread or something with that discussion in it?

[strepanier03]

@tylerkaraszewski - Oh damn, this fell through the cracks last week and I dropped the ball here. So sorry about that.

I started the discussion here. Thank you for the bump 🙌

[strepanier03]

Bumped the discussion in #expensify-open-source again.

[sobitneupane]

Thanks @strepanier03.

@dukenv0307 Would you like to update your proposal to match behavior in New group page. This is what happens in new group page.

In New group page, Text Input refocuses on select row with input text selected.

[dukenv0307]

@sobitneupane I just commented on Slack thread. Let's wait a bit to discuss more and make the best decision for this issue

[melvin-bot[bot]]

@tylerkaraszewski @strepanier03 @sobitneupane this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[strepanier03]

@dukenv0307 - Thanks for posting on the thread and moving things forward, I appreciate the knowledge you brought to the discussion.

Looks like your suggestion has buy in though so I think we're good to move forward.

[tylerkaraszewski]

👍 go ahead.

[melvin-bot[bot]]

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

[sobitneupane]

@dukenv0307 Does your proposal meet the latest expected output or do you want to update it?

[dukenv0307]

@sobitneupane My initial proposal already follows that expected output in the Slack thread, no updates needed

Thanks

[sobitneupane]

@dukenv0307 Similar to Create Group, both on selecting or deselecting a row, value input in the search field should be selected.

Overall, Proposal from @dukenv0307 looks good to me.

🎀👀🎀 C+ reviewed cc: @tylerkaraszewski

[strepanier03]

@tylerkaraszewski - Friendly bump on reviewing the proposal that @sobitneupane thinks looks good.

[melvin-bot[bot]]

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

[melvin-bot[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

[strepanier03]

@isabelastisser - I am heading out on sabbatical this evening and need to reassign this. There is no immediate action you need to take on this today, I bumped the eng for review and we just need to keep pressure on and this moving forward.

Thank you for handling the wrap up of this one!

[melvin-bot[bot]]

@tylerkaraszewski @sobitneupane @isabelastisser this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

Current assignee @sobitneupane is eligible for the Internal assigner, not assigning anyone new.

[isabelastisser]

Hey @tylerkaraszewski , can you please follow up on this? Thanks!

https://github.com/Expensify/App/issues/19231#issuecomment-1593636886

[isabelastisser]

Bump @tylerkaraszewski

[isabelastisser]

Assigning another team member because I will be OOO until July 5. SO for reference.

[melvin-bot[bot]]

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

[melvin-bot[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[bot]]

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

[melvin-bot[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