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.51k stars 2.86k forks source link

[Awaiting payment on 23/08/24] [$250] Pressing `Tab` in the magic code screen does not move to next magic code input #45416

Closed m-natarajan closed 1 month ago

m-natarajan commented 3 months 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!


Version Number: 9.0.6-4 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 Expensify/Expensify Issue URL: Issue reported by: @dylanexpensify Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1721030930518949

Action Performed:

  1. Open staging.new.expensify.com
  2. Enter the email address of any account
  3. When the focus is on any of the inputs for the magic code except the last one press Tab key

Expected Result:

The cursor moves to the next input for magic code

Actual Result:

The cursor jumps to Didn't receive a magic code? if 30 sec elapses or sign in button

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/85086f25-0e28-42a9-8eb8-f73d68f6332f

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01cf3849877e81009e
  • Upwork Job ID: 1818598235444042146
  • Last Price Increase: 2024-07-31
  • Automatic offers:
    • ishpaul777 | Reviewer | 103423841
Issue OwnerCurrent Issue Owner: @ishpaul777
melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

nyomanjyotisa commented 3 months ago

Proposal

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

Pressing Tab in the magic code screen does not move to next magic code input

What is the root cause of that problem?

We don't handle Tab press on onKeyPress on MagicCodeInput here, so it still default behavior to move to the next element

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

Add Tab key press logic on onKeyPress on MagicCodeInput here to move to next part of the magic code input

else if (keyValue === 'Tab' && focusedIndex !== undefined) {
    const newFocusedIndex = focusedIndex + 1;
    if (newFocusedIndex !== maxLength) {
        if (event.preventDefault) {
            event.preventDefault();
        }
        setInputAndIndex(newFocusedIndex);
        inputRefs.current?.focus();
    }
}

RESULT

https://github.com/user-attachments/assets/398bec0c-dc57-48e6-a502-348a946b99d9

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 3 months ago

@miljakljajic Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 3 months ago

@miljakljajic Still overdue 6 days?! Let's take care of this!

melvin-bot[bot] commented 3 months ago

@miljakljajic 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot[bot] commented 3 months ago

@miljakljajic Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

melvin-bot[bot] commented 3 months ago

@miljakljajic this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 3 months ago

@miljakljajic 12 days overdue now... This issue's end is nigh!

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

daledah commented 3 months ago

Proposal

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

The cursor jumps to Didn't receive a magic code? if 30 sec elapses or sign in button

What is the root cause of that problem?

The magic code input actually has only 1 input https://github.com/Expensify/App/blob/032a8fc433aea38533ad6f99acd74f5c8f2de0a4/src/components/MagicCodeInput.tsx#L368

The 6 fields are View with manual focus highlighting behavior https://github.com/Expensify/App/blob/032a8fc433aea38533ad6f99acd74f5c8f2de0a4/src/components/MagicCodeInput.tsx#L401

So when tabbing, it will just move directly to buttons below.

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

We need to manually handle the tabbing behavior, just like we did for arrow keys. So in https://github.com/Expensify/App/blob/032a8fc433aea38533ad6f99acd74f5c8f2de0a4/src/components/MagicCodeInput.tsx#L335, we can add

else if (keyValue === 'Tab' && focusedIndex !== undefined) {
    const newFocusedIndex = event.shiftKey ? focusedIndex - 1 : focusedIndex + 1;
    if (newFocusedIndex >= 0 && newFocusedIndex < maxLength) {
        setInputAndIndex(newFocusedIndex);
        inputRefs.current?.focus();
        event?.preventDefault && event.preventDefault();
    }
}

If the user presses tab, it's forward navigation and the new focused index will be the one on the right, otherwise. If the user presses shift also, it's backward tabbing and the one we should focus next would be the one on the left.

If the new index is within range, we prevent default and focus on that index, if it's not in range, that means the user wants to navigate outside of the magic code inputs and we should do nothing and let the browser handle the rest.

What alternative solutions did you explore? (Optional)

We can check and possibly handle other key combination with Tab too, ie. Ctrl + Tab should do nothing

ishpaul777 commented 3 months ago

Thanks for your proposals @nyomanjyotisa @daledah.


Proposal from @daledah looks most complete to me, lets assign them πŸ‘

πŸŽ€ πŸ‘€ πŸŽ€

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 2 months ago

@miljakljajic, @srikarparsi, @ishpaul777 Eep! 4 days overdue now. Issues have feelings too...

ishpaul777 commented 2 months ago

waiting for final proposal review cc @srikarparsi

srikarparsi commented 2 months ago

Looks good to me as well, thanks for the bump

melvin-bot[bot] commented 2 months ago

πŸ“£ @ishpaul777 πŸŽ‰ 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[bot] commented 2 months ago

πŸ“£ @daledah You have been assigned to this job! Please apply to the Upwork job 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 πŸ“–

daledah commented 2 months ago

@ishpaul777 this PR is ready for review.

miljakljajic commented 2 months ago

Is this one coming up for payment this week? I noticed the PR was deployed to prod, but seems like the automation has failed

srikarparsi commented 2 months ago

Yes, I believe so thank you @miljakljajic

miljakljajic commented 2 months ago

@daledah it doesn't look like you've applied to the job on upwork. Can you please either apply or share your upwork profile so we can get an offer sent to you?

miljakljajic commented 2 months ago

@ishpaul777 has been paid - we can close this out when we hear from @daledah - please share your profile or apply to the job.

melvin-bot[bot] commented 2 months ago

@miljakljajic, @srikarparsi, @ishpaul777, @daledah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

ishpaul777 commented 2 months ago

Update above melvin https://github.com/Expensify/App/issues/45416#issuecomment-2312312310

daledah commented 2 months ago

@miljakljajic Here's my profile https://www.upwork.com/freelancers/~0138d999529f34d33f, could you help send the offer? Thx

miljakljajic commented 1 month ago

@daledah 's offer here: https://www.upwork.com/nx/wm/offer/103795834

daledah commented 1 month ago

@miljakljajic I accepted it

miljakljajic commented 1 month ago

Paid!