-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 2024-03-11] [HOLD for #36401] [$500] Reimburse - Pressing 'Enter' key on 'Rate' page refreshes the page #36439
Comments
Triggered auto assignment to @Christinadobrzyn ( |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
Triggered auto assignment to @deetergp ( |
@shubham1206agra Similar to #36363 |
ProposalPlease re-state the problem that we are trying to solve in this issue.Reimburse - Pressing 'Enter' key on 'Rate' page refreshes the page What is the root cause of that problem?The main root cause of the problem is that App/src/pages/workspace/reimburse/WorkspaceRateAndUnitPage/RatePage.tsx Lines 85 to 105 in 19e2849
What changes do you think we should make in order to solve the problem?The main change that will stop this issue is, we need to stop the What alternative solutions did you explore? (Optional)N/A |
Job added to Upwork: https://www.upwork.com/jobs/~01255e944bed02c24a |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws ( |
I don't think this is a blocker. I have confirmed that the bug exists in I was going to say that I'd expect hitting |
Oh I am working on this on separate PR. This is from another PR I raised earlier. |
I understand that PR #36401 fixes this issue, right? |
@deetergp or @Ollyws could you confirm this for us? #36439 (comment) |
Let's hold this one for #36401 |
This issue is now being handled in the regression path, and PR #36401 has been created to solve it. I'm not sure if we call such cases "holding". Anyway, I'll make sure to make that PR ready for merging ASAP. |
Clarification here: The expected behaviour should be that the page should not refresh. We do not submit form on pressing Enter now on this specific page. |
Unless there's any objection, we'll assume the expected behavior @shubham1206agra mentioned. |
Is there any way to allow submitting the form on enter yet preventing the refresh? |
To prevent refresh, I already have a PR in place. For submitting the form on Enter, I need to change so many components to achieve the same, so it might be a feature request for me. (This is context to this component only). |
I feel like this should be handled here? or as a follow up? @shubham1206agra |
Let me see if there's a simple fix to this. If there is, I will add it in the same PR. |
thanks @shubham1206agra, and the expected result is Amount change should be saved 😄 |
@getusha Please check now. This is now same as Expected behavior. |
Where are we at with this one? |
Good question 🤔 PR #36401 is still open, but it's both C+ and internally approved. Not merged yet, though. There was some post-approval discussion between @shubham1206agra and @getusha. |
Nice! Looks like it got merged yesterday. |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.46-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-03-11. 🎊 For reference, here are some details about the assignees on this issue:
|
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:
|
|
I think that means we can close this one out then, right? |
Yep! closing! |
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: v1.4.41-2
Reproducible in staging?: y
Reproducible in production?: n (new feature)
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: Applause internal team
Slack conversation:
Action Performed:
Expected Result:
Amount change should be saved
Actual Result:
Page refreshes
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6378609_1707856570560.Screen_Recording_2024-02-13_at_11.34.36_at_night.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: