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

lanitochka17 commented 3 weeks 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: 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:

Go to staging.new.expensify.com
Go to Collect workspace settings
Go to Distance rates
Click Add rate
Click Name
Hit Enter without entering the name
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
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/a2d6b58f-378e-46cc-ad84-c0ca8a1db7d1

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

melvin-bot[bot] commented 3 weeks ago

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 commented 3 weeks ago

@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 commented 3 weeks ago

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

https://github.com/Expensify/App/assets/59809993/1d070cf6-0cbe-4a03-acf3-dcbda97cf7b5

melvin-bot[bot] commented 2 weeks ago

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

kevinksullivan commented 2 weeks ago

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

kevinksullivan commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

akinwale commented 2 weeks ago

@abzokhattab's proposal makes sense here.

🎀👀🎀 C+ reviewed.

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

Expensify / App