-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-10-10] [$500] Request money flow is not proceeded to the next page #28022
Comments
Current assignee @dylanexpensify is eligible for the Bug assigner, not assigning anyone new. |
Job added to Upwork: https://www.upwork.com/jobs/~0126a0498c5bbce06a |
Bug0 Triage Checklist (Main S/O)
|
Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak ( |
related #26588 |
On the latest version main and on staging do not reproduce |
The issue details are a little off as we added a patch to solve the previous issue which caused a new issue. The issue would be>
Steps:
Expected: Actual: cc: @dylanexpensify |
ProposalPlease re-state the problem that we are trying to solve in this issue.Pressing the Enter key on the Distance Amount page does not take the user to the next page after the form is enabled. What is the root cause of that problem?we have disabled the The real root cause of #26588 is explained below: I analyzed and it seems we are preventing the bubbling of the Enter key event on the Button component. App/src/components/Button/index.js Line 186 in 52b1e3e
As all 3 tabs are loaded together, the last mounted button component on DistanceRequest will take precedence in event callbacks when the event is fired from the browser. Because bubbling is disabled, only one handler is called when the same component is mounted multiple times on the page. We never expected multiple CTA buttons on a single page so bubbling was prevented but the tab behave differently thus we need to handle that. What changes do you think we should make in order to solve the problem?The easiest solution to keep pressonEnter as well as fix the issue without causing regression on other pages where Button is used would be to create a new prop on App/src/components/Button/index.js Line 186 in 52b1e3e
false by default as before and just enable it for two pages DistanceRequest & MoneyRequestAmountForm . App/src/components/DistanceRequest.js Line 254 in 52b1e3e
What alternative solutions did you explore? (Optional)None |
thanks @parasharrajat, updated! @eVoloshchak let's get the proposal reviewed please! |
Bump! |
@parasharrajat's #28022 (comment) looks good to me! 🎀👀🎀 C+ reviewed! |
Triggered auto assignment to @johnmlee101, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @eVoloshchak Please request via NewDot manual requests for the Reviewer role ($500) |
📣 @parasharrajat Please request via NewDot manual requests for the Contributor role ($500) |
@johnmlee101, @eVoloshchak, @parasharrajat, @dylanexpensify Huh... This is 4 days overdue. Who can take care of this? |
Update: Looking into the regression issue ATM. |
PR Created #30259 |
This comment was marked as duplicate.
This comment was marked as duplicate.
Not overdue, PR is in production |
@eVoloshchak thanks for the update! |
@johnmlee101, @eVoloshchak, @parasharrajat, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Not overdue, PR with the fix was deployed to prod on Oct 30 |
Should be good now? |
@eVoloshchak to confirm |
Yes, the payment date should be 6 Nov here. @dylanexpensify |
@johnmlee101, @eVoloshchak, @parasharrajat, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
1 similar comment
@johnmlee101, @eVoloshchak, @parasharrajat, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Regression Test Proposal
Do we agree 👍 or 👎 Also noting that this has caused a regression in #29051 |
Payment summary:
Please apply in Upwork or create a NewDot request! |
$250 payment approved for @eVoloshchak based on the comment above. |
Payment requested as per #28022 (comment) |
$250 approved for @parasharrajat |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Issue found when executing PR #26520
Action Performed:
Expected:
Pressing Enter should submit the Distance request.
Actual:
Enter key submission does not work on Distance Page.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.62-1
Reproducible in staging?: Yes
Reproducible in production?: No
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
Bug6186267_20230902_234752.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: