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

[$250] Distance rate - Unable to proceed with empty name via Enter key, while it works with green CTA #40945

Closed
2 of 6 tasks
lanitochka17 opened this issue Apr 24, 2024 · 16 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Monthly KSv2 Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Apr 24, 2024

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: 1.4.65-5
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to Collect workspace settings
  3. Go to Distance rates
  4. Click Add rate
  5. Click Name
  6. Hit Enter without entering the name
  7. Return to previous page

Expected Result:

In Step 6, Enter key should save the empty name (because it works by clicking on the Save button)

Actual Result:

In Step 6, Enter key does not save the empty name
When returning to previous page in Step 7, error appears below the empty fields, which means Enter key is working elsewhere instead

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6460735_1713988573289.20240425_035123.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~018c1c78753474f238
  • Upwork Job ID: 1785417584710262784
  • Last Price Increase: 2024-04-30
  • Automatic offers:
    • akinwale | Reviewer | 0
    • abzokhattab | Contributor | 0
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 24, 2024
Copy link

melvin-bot bot commented Apr 24, 2024

Triggered auto assignment to @kevinksullivan (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.

@lanitochka17
Copy link
Author

@kevinksullivan FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@abzokhattab
Copy link
Contributor

abzokhattab commented Apr 25, 2024

Proposal

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

Unable to proceed with empty name via Enter key in Distance rate modal

What is the root cause of that problem?

The problem originates from how the Enter key event is managed across multiple interactive components within the Create Tax page setup.

Here's what's happening:

  • The page uses a FormProvider component configured with disablePressOnEnter={false}, indicating that pressing the Enter key will submit the form.
  • within the FormProvider children, the TextPicker is used as an InputComponent.
  • When the TextPicker is opened, it opens a modal that includes text input and a save button, which also has the pressOnEnter enabled.

The core of the issue is that the Enter key press is activated in both the main form and the children modals, so when a user is in the TextPicker modal and presses Enter, FormAlertWithSubmitButton of the main will execute the onPress not the Text selector save button

This overlap in how the Enter key is handled between nested components causes confusion and incorrect behavior.

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

we can use the enterKeyEventListenerPriority to assign the enter key event priority inside the FormAlertWithSubmitButton and set it to 1 (lower priority) so that in such cases the child component submit button will have a higher priority

  • to achieve that we need to add the enterKeyEventListenerPriority as new prop with a default value of 0 inside the FormAlertWithSubmitButton (also we need to add it into the FormAlertWithSubmitButtonProps types):
    enterKeyEventListenerPriority = 0,
  • then this value should be passed here:
enterKeyEventListenerPriority={enterKeyEventListenerPriority}
  • finally we need to set the enterKeyEventListenerPriority to 1 inside the FormAlertWithSubmitButton component call in the FormWrapper:
enterKeyEventListenerPriority={1}

POC

Screen.Recording.2024-04-25.at.4.32.16.AM.mov

@melvin-bot melvin-bot bot added the Overdue label Apr 29, 2024
Copy link

melvin-bot bot commented Apr 30, 2024

@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

@kevinksullivan
Copy link
Contributor

Hm it's only for a brand new field but I was able to reproduce.

@melvin-bot melvin-bot bot removed the Overdue label Apr 30, 2024
@kevinksullivan
Copy link
Contributor

Going to add to Collect wave and make external, but I'd consider this a low priority.

@kevinksullivan kevinksullivan added the External Added to denote the issue can be worked on by a contributor label Apr 30, 2024
Copy link

melvin-bot bot commented Apr 30, 2024

Job added to Upwork: https://www.upwork.com/jobs/~018c1c78753474f238

@melvin-bot melvin-bot bot changed the title Distance rate - Unable to proceed with empty name via Enter key, while it works with green CTA [$250] Distance rate - Unable to proceed with empty name via Enter key, while it works with green CTA Apr 30, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 30, 2024
Copy link

melvin-bot bot commented Apr 30, 2024

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

@akinwale
Copy link
Contributor

akinwale commented May 3, 2024

@abzokhattab's proposal makes sense here.

🎀👀🎀 C+ reviewed.

Copy link

melvin-bot bot commented May 3, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label May 3, 2024
Copy link

melvin-bot bot commented May 3, 2024

📣 @akinwale 🎉 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

Copy link

melvin-bot bot commented May 3, 2024

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

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels May 29, 2024
Copy link

melvin-bot bot commented May 29, 2024

This issue has not been updated in over 15 days. @akinwale, @blimpich, @kevinksullivan, @abzokhattab eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@akinwale
Copy link
Contributor

This was deployed to production on 2023-05-23 which makes payment due 2023-05-30.

cc @kevinksullivan

@blimpich
Copy link
Contributor

I think we can close this? @kevinksullivan @akinwale @abzokhattab is payment complete?

@blimpich
Copy link
Contributor

Closing, if this is wrong then please feel free to ping me in Slack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Monthly KSv2 Reviewing Has a PR in review
Projects
Archived in project
Development

No branches or pull requests

5 participants