Closed izarutskaya closed 10 months ago
Job added to Upwork: https://www.upwork.com/jobs/~01f4cfb0b4e5279620
Triggered auto assignment to @mallenexpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Platforms
in OP are ✅)Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External
)
I'm trying to reproduce the issue but I can't, it seems like when I get to the split bill action we have an exception in where we try to parse the participant login but is undefined
https://github.com/Expensify/App/assets/1194982/d77c86f6-ceed-46d4-beda-f6d2e6d1ccbe
I believe we need to first fix this issue?
When editing the amount on the request confirmation page, hitting Enter
submits the request instead of just updating the amount.
The reason for this is that when we are editing the amount, we have 2 buttons with pressOnEnter
:
Since we explicitly allow bubbling on the button in MoneyRequestAmountForm
, these 2 buttons are pressed on hitting Enter
at once.
We need to make the allowBubble
conditional and disable it when in editing mode:
<Button
success
- allowBubble
+ allowBubble={!isEditing}
pressOnEnter
medium={isExtraSmallScreenHeight}
style={[styles.w100, canUseTouchScreen ? styles.mt5 : styles.mt2]}
onPress={submitAndNavigateToNextPage}
text={buttonText}
/>
https://github.com/Expensify/App/assets/12595293/e54caf1a-e59a-491f-9384-16918ddd52f8
Pressing enter on the edit amount view when focused on the amount text field creates the split bills instead of going back to the confirmation page
The footer button on <MoneyRequestAmountForm>
bubbles the enter event, causing a double action enter action, subsequently creating the bill when landing on the confirmation view.
By removing the allowBubble
from the button we will prevent the event to happen twice
Pressing enter on the edit amount view when focused on the amount text field creates the split bills instead of going back to the confirmation page
The footer button on <MoneyRequestAmountForm>
bubbles the enter event, causing a double action on enter action.
https://github.com/Expensify/App/blob/e448a7e912231ee47fd384ec73e9f3396c596871/src/pages/iou/steps/MoneyRequestAmountForm.js#L284-L292
We need to remove the allowBubble from the button.
<Button
success
pressOnEnter
medium={isExtraSmallScreenHeight}
style={[styles.w100, canUseTouchScreen ? styles.mt5 : styles.mt2]}
onPress={submitAndNavigateToNextPage}
text={buttonText}
/>
NA
@eVoloshchak can you please review the proposals above?
I think this is regression from https://github.com/Expensify/App/issues/28022 and PR author/reviewer should handle this as it's within regression period.
Thanks @mkhutornyi , put on hold.
This was supposed to work based on what we implemented in the bubbling for Main Button. Looks like when the event bubbles, the page behind the screen gets focused just in time thus it ends up firing the event.
I was expecting that isFocused
on the Button would prevent hidden buttons from firing.
Unfortunately, none of the proposals above make a best solution.
@parasharrajat do you agree that this is a regression from https://github.com/Expensify/App/issues/28022 that you worked on? Trying to figure out what the next step is here
Yes, I am looking into this.
@eVoloshchak, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@parasharrajat can you provide an update when you have a minute? Thx
Update: Tried a few tricks but all seem to be workarounds. I will try to get some help from Slack to solve it. ETA: Friday for the PR.
@eVoloshchak, @mallenexpensify Eep! 4 days overdue now. Issues have feelings too...
Will be tackling this today.
📣 @eVoloshchak Please request via NewDot manual requests for the Reviewer role ($500)
📣 @parasharrajat Please request via NewDot manual requests for the Contributor role ($500)
@parasharrajat also assigning this to you, so that when you provide updates, it'll remove Overdue
I rechecked and https://github.com/Expensify/App/issues/29051#issuecomment-1751679482 is working great. I was somehow confused about this while reading it earlier.
Because it is a regression from my PR, I have created a PR to fix this. There shouldn't be any need to make this change based on isFocused
check but it is not working https://github.com/Expensify/App/issues/29051#issuecomment-1757438456. I do not want to implement any hacks so preventing bubbling on the edit page seems fine.
Triggered auto assignment to @tylerkaraszewski, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
Fix is merged.
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.92-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-11-06. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
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:
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:
Do you agree 👍 or 👎 ?
@mallenexpensify Please remove [HOLD Possible regression https://github.com/Expensify/App/issues/28022] from the title.
Is compensation due here? Wasn't sure because it looks like this was related to a regression.
Yeah, No compensation on this.
And... lastly(?) @eVoloshchak, are you owed any compensation?
No compensation for me either, this issue is just a regression from another PR
K, thanks! Closing (since I'm assuming, since this is a regression, any regression tests would be created as part of the original issue, comment if ya disagree).
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
The new amount is saved and user lands in confirmation page.
Actual Result:
The new amount is not saved and Enter key triggers the creation of split bill with the previous amount.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.3.79-3
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
Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/115492554/b5d35db4-cf5c-47ce-8190-87fc10ea83e3
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @
View all open jobs on GitHub
Upwork Automation - Do Not Edit