Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.29k stars 2.72k forks source link

[HOLD for payment 2023-07-17] [$1000] Android - Keyboard doesn't open automatically while verifying contact method #21415

Closed kbecciv closed 1 year ago

kbecciv commented 1 year ago

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. Go to settings -> Profile -> contact method
  2. Go to new contact method
  3. Enter email/phone and add
  4. Now click the recently added mail/number to verify
  5. Form field to enter magic code appears. However, Notice that there appears no keyboard to type .

Expected Result:

Keyboard should be shown on default . like on magic code entry page in signing page.

Actual Result:

Keyboard appears only after user click in verify new contact method page.

Workaround:

Unknown

Platforms:

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

Version Number:1.3.27-7

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/93399543/b22d21fe-66b7-4d28-840d-cce75c284d9c

https://github.com/Expensify/App/assets/93399543/205de6c0-5627-471a-8a27-2c77c5ee08f7

Expensify/Expensify Issue URL:

Issue reported by: @ashimsharma10

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686916708232399

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0148ec17c8af49cf8c
  • Upwork Job ID: 1672352190027980800
  • Last Price Increase: 2023-06-23
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

situchan commented 1 year ago

Proposal

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

Keyboard doesn't open in android when magic code input gets focused

What is the root cause of that problem?

In android, keyboard doesn't open because of navigation transition.

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

Solution 1: use shouldDelayFocus

In /Contacts/BaseValidateCodeForm component:

In MagicCodeInput component:

Solution 2: set autoFocus to false here and focus manually on onEntryTransitionEnd callback of ScreenWrapper in ContactMethodDetailsPage

melvin-bot[bot] commented 1 year ago

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

lukemorawski commented 1 year ago

Hey this is Lucas from callstack, I think I have a solution for that.

flaviadefaria commented 1 year ago

Hey @lukemorawski feel free to submit your proposal and it will be reviewed by the C+ like all others. If you have the best proposal you will be assigned to the GH.

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

allroundexperts commented 1 year ago

Waiting for @lukemorawski to send his proposal.

While we wait, @situchan, can you please verify that your solution 2 really works? Please post a video if possible.

lukemorawski commented 1 year ago

@allroundexperts my solution is almost identical with an addtional removal of autoFocus prop from inputs as it's already handled by the programatically in on of the useEffect hooks. I can confirm that this solution works.

situchan commented 1 year ago

While we wait, @situchan, can you please verify that your solution 2 really works? Please post a video if possible.

yes it works but ref needs to be passed multiple times as BaseValidateCodeForm is not a direct children of ContctMethodDetailsPage. So I prefer Solution 1. If TextInput and ScreenWrapper are in the same component, maybe Solution 2 is prefered. I don't think it's a problem whether to pick up Solution 1 or 2 because app already has those 2 inconsistent approaches of auto-focus across all the pages. i.e. #15184 (shouldDelayFocus), #18889 (onEntryTransitionEnd)

allroundexperts commented 1 year ago

Thanks for the confirmation @lukemorawski & @situchan. Since @situchan proposed this first, let's go with there proposal.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed

melvin-bot[bot] commented 1 year ago

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

flaviadefaria commented 1 year ago

Waiting for PR

allroundexperts commented 1 year ago

Waiting for PR

@flaviadefaria We're waiting for @techievivek to review and assign this task to @situchan.

melvin-bot[bot] commented 1 year ago

πŸ“£ @allroundexperts πŸŽ‰ An offer has been automatically sent to your Upwork account πŸŽ‰

Reviewer - [$1000] Android - Keyboard doesn't open automatically while verifying contact method

melvin-bot[bot] commented 1 year ago

πŸ“£ @situchan You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

melvin-bot[bot] commented 1 year ago

πŸ“£ @ashimsharma10 You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

techievivek commented 1 year ago

Assigned the task to @situchan, thanks.

situchan commented 1 year ago

@allroundexperts PR is ready for review

melvin-bot[bot] commented 1 year ago

🎯 ⚑️ Woah @allroundexperts / @situchan, 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 πŸŽ‰

On to the next one πŸš€

melvin-bot[bot] commented 1 year ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 year ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.38-7 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-17. :confetti_ball:

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.

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

melvin-bot[bot] commented 1 year ago

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:

flaviadefaria commented 1 year ago

Payment to be processed:

@ashimsharma10 = $250 (UW) @allroundexperts $1000 + $500 bonus = $1500 (Expensify) @situchan $1000 + $500 bonus = $1500 (UW)

@allroundexperts can you please fill out the checklist above so that I can process payment? Thanks!

allroundexperts commented 1 year ago

This is a well known issue throughout our app where we're focusing inputs without waiting for the screen transition to finish. The team which initially implemented this seems to have not considered the page transition. As such, there is not a single PR which can be pointed to.

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: https://expensify.slack.com/archives/C049HHMV9SM/p1689772875086199

I think this does not effect the user experience too much. As such, a regression test might not be needed here.

anmurali commented 1 year ago

Approved payment to @allroundexperts based on https://github.com/Expensify/App/issues/21415#issuecomment-1639950981

allroundexperts commented 1 year ago

@flaviadefaria Can you please withdraw the offer on Upwork?

flaviadefaria commented 1 year ago

Oops, I was meant to send it to @situchan. Withdrawing it and sending it correctly to @situchan.

flaviadefaria commented 1 year ago

Offer sent to @situchan @ashimsharma10 I can see two "Ashim S." in Upworks, can you send me your profile link or clarify if you're the one based in Nepal or Bangladesh? Thanks!

ashimsharma10 commented 1 year ago

@flaviadefaria Nepal it is . https://www.upwork.com/freelancers/~018a92cf13e1e88eed

flaviadefaria commented 1 year ago

Offers sent! @situchan and @ashimsharma10 just need you to accept them so that I can pay you and close this :)

flaviadefaria commented 1 year ago

@ashimsharma10 paid

flaviadefaria commented 1 year ago

@situchan paid. Closing this.