Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2023-06-29] [$1000] Keyboard is popping up everytime user selects a contact when creating a new group #20636

Closed
1 of 6 tasks
kavimuru opened this issue Jun 12, 2023 · 55 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Jun 12, 2023

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 "New group"
  3. Select a contact
  4. Hide the keyboard
  5. Select another contact

Expected Result:

Keyboard should not popup every time user selects a new contact as this behavior is not seen on Android app

Actual Result:

Keyboard pops up every time user selects a new contact

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

Screen_Recording_20230607_183004_Chrome.mp4
az_recorder_20230612_194921.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Nathan-Mulugeta
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686153010639509

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0100b196cb7f4e7c43
  • Upwork Job ID: 1668689630840729600
  • Last Price Increase: 2023-06-13
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2023

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

@huzaifa-99
Copy link
Contributor

huzaifa-99 commented Jun 13, 2023

Proposal

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

We want to prevent the keyboard from opening up on every option selected in new group creation

What is the root cause of that problem?

We are passing shouldFocusOnSelectRow as true in case of group chats to OptionsSelector in NewChatPage which is causing refocus on the text input and opening the keyboard

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

We should remove shouldFocusOnSelectRow from OptionsSelector in NewChatpage

<OptionsSelector
    // shouldFocusOnSelectRow={this.props.isGroupChat} - remove this
    // ... other code
/>

The WorkspaceInvitePage has the same issue which we can also fix by not passing shouldFocusOnSelectRow prop.

Alternatively, if we want to keep the focus on the input field on desktop web but not on mobile web, then we can only add shouldFocusOnSelectRow for mobile platform and when we are creating a group chat, something like this

shouldFocusOnSelectRow={this.props.isGroupChat && !Browser.isMobile()}

What alternative solutions did you explore? (Optional)

N/A

@joekaufmanexpensify
Copy link
Contributor

Reproduced on Android mobile web. Also confirmed we don't do this on android native, or iOS native. I agree we should not open the keyboard every time a new contact is selected.

2023-06-13_14-34-48.mp4

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Jun 13, 2023
@melvin-bot melvin-bot bot changed the title Keyboard is popping up everytime user selects a contact when creating a new group [$1000] Keyboard is popping up everytime user selects a contact when creating a new group Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@Santhosh-Sellavel
Copy link
Collaborator

@huzaifa-99 you mean !Browser.isMobile() right?

@Santhosh-Sellavel
Copy link
Collaborator

@marcaaron @huzaifa-99's proposal looks good to me.
🎀 👀 🎀
C+ Reviewed

@huzaifa-99
Copy link
Contributor

huzaifa-99 commented Jun 13, 2023

@huzaifa-99 you mean !Browser.isMobile() right?

yes, thanks for spotting

@huzaifa-99
Copy link
Contributor

@marcaaron @huzaifa-99's proposal looks good to me. 🎀 👀 🎀 C+ Reviewed

Thank you for the review

@marcaaron
Copy link
Contributor

if we want to keep the focus on the input field on desktop web but not on mobile web, then we can only add shouldFocusOnSelectRow for mobile platform and when we are creating a group chat, something like this

I think we do. If that's the current behavior we should not change it. So that would be the alternate proposal in this case?

@Santhosh-Sellavel you may have implied with your previous question that the alternative proposal is what we want - though, not sure. In the future, try make it clear whether you are recommending the main proposal or anything from the alternate proposal. Thank you 🙇

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

📣 @huzaifa-99 You have been assigned to this job by @marcaaron!
Please apply to this job in Upwork 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 📖

@huzaifa-99
Copy link
Contributor

if we want to keep the focus on the input field on desktop web but not on mobile web, then we can only add shouldFocusOnSelectRow for mobile platform and when we are creating a group chat, something like this

I think we do. If that's the current behavior we should not change it. So that would be the alternate proposal in this case?

@Santhosh-Sellavel you may have implied with your previous question that the alternative proposal is what we want - though, not sure. In the future, try make it clear whether you are recommending the main proposal or anything from the alternate proposal. Thank you 🙇

Thanks for the feedback, yes keeping focusing on desktop and not on web would be the alternate proposal. Pr will be up soon

@huzaifa-99
Copy link
Contributor

@Santhosh-Sellavel @marcaaron PR up for review #20731

@melvin-bot
Copy link

melvin-bot bot commented Jun 16, 2023

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

  • when @huzaifa-99 got assigned: 2023-06-13 22:50:01 Z
  • when the PR got merged: 2023-06-16 18:55:11 UTC
  • days elapsed: 3

On to the next one 🚀

@Santhosh-Sellavel
Copy link
Collaborator

@joekaufmanexpensify It got merged within 3 biz days. Melvin is confused here.

@Natnael-Guchima
Copy link

Natnael-Guchima commented Jun 26, 2023

@Natnael-Guchima I'd say no. We closed the issue you reported because we were not able to reproduce it. Whereas the bug in this issue was consistently reproducible, which is why we took action here.

Given the behavior you reported was not consistently reproducible, we wouldn't issue a reporting bonus for it.

@joekaufmanexpensify If you saw my conversations with @stephanieelliott you can see I have been reproducing the issue consistently and I have been providing a clear steps to reproduce the issue. There are 4 videos reproducing the issue perfectly on the closed GH. If you are convinced that the steps were not clear on the action performed section, it is alright.

Screenshot_20230626_163254_GitHub.jpg

@huzaifa-99
Copy link
Contributor

@huzaifa-99 offer sent for $1,000! ($500 will be issued as bonus)

Accepted, Thank you!

@Santhosh-Sellavel
Copy link
Collaborator

@joekaufmanexpensify I will get done with the checklist this week. Regarding payment I'm just waiting for the internal process, please check here

@joekaufmanexpensify
Copy link
Contributor

Sounds good, thanks!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 28, 2023
@joekaufmanexpensify
Copy link
Contributor

@Santhosh-Sellavel we are all set to test your payment. Could you please complete your portion of the BZ checklist, and then I'll post new steps for payment in Slack.

@Santhosh-Sellavel
Copy link
Collaborator

Pinged you on a Slack thread about payment. Caught up on other priority items, I'll complete the checklist this week in a day or two!

@joekaufmanexpensify
Copy link
Contributor

Sounds good! As soon as the BZ checklist is complete, we'll issue the payments on this issue (besides yours, which is held for the new process).

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Jul 2, 2023

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:

cc: @marcaaron Let me know if you differ on any of the above. What do you think about adding regression tests here?

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Jul 2, 2023

Regression Steps

  1. Go to the new chat page from the fab icon
  2. In desktop/web Verify if the search input is focused then it should remain focused when any member is selected.
  3. In native & mWeb, Verify that selecting a member is not focusing on the search input again.
  4. Follow steps 2 and 3 for the workspace invite member page.

👍 or 👎

@marcaaron
Copy link
Contributor

Verify that selecting a member is not focusing on the search input again.

I would rephrase this like:

Verify if the search input is focused then it should lose focus when any member is selected.

But otherwise 👍

@Nathan-Mulugeta
Copy link

Hey guys payment has not been processed for this issue. @joekaufmanexpensify

@huzaifa-99
Copy link
Contributor

Hey guys payment has not been processed for this issue. @joekaufmanexpensify

bump ^

@joekaufmanexpensify
Copy link
Contributor

joekaufmanexpensify commented Jul 5, 2023

Thanks! I was OOO the past two days, and previously we were waiting for the BZ checklist to be completed (which is required before we issue payments). I will finish this up and issue payment today.

@joekaufmanexpensify
Copy link
Contributor

Added regressions test issue above. Only change I made is that I made step 1 specific to new group, rather than new chat as I think new chat was a typo (given this issue only applies when there is the ability to select multiple invitees).

@joekaufmanexpensify
Copy link
Contributor

BZ checklist is now complete! All set to issue payment.

@joekaufmanexpensify
Copy link
Contributor

@huzaifa-99 $1,500 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

@Nathan-Mulugeta $250 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed.

@joekaufmanexpensify
Copy link
Contributor

This one is all set, besides paying @Santhosh-Sellavel, which is on hold for the new payment process.

@huzaifa-99
Copy link
Contributor

@joekaufmanexpensify TYSM, also apologies I didn't knew you were OOO.

@joekaufmanexpensify
Copy link
Contributor

Of course, and no worries at all!

@joekaufmanexpensify
Copy link
Contributor

Still held on paying Santhosh

@anmurali
Copy link

anmurali commented Jul 7, 2023

Santhosh has been paid on New Dot

@joekaufmanexpensify
Copy link
Contributor

Great, thanks!

@joekaufmanexpensify
Copy link
Contributor

@Santhosh-Sellavel closing for now as payment has been issued. LMK if there are any issues receiving it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

8 participants