-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix: Delay in updating green dot and total amount #34866
Conversation
@hoangzinh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@thienlnam it looks like Melvin assigned wrong C+. Could you help to assign @shubham1206agra and unassign me? Thanks |
src/libs/actions/IOU.js
Outdated
@@ -2646,6 +2654,13 @@ function deleteMoneyRequest(transactionID, reportAction, isSingleTransactionView | |||
}, | |||
] | |||
: []), | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For K2 to pick up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdym?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry not for you. Its for me.
@shubham1206agra any updates? |
@shubham1206agra Pls help review my PR when you have a chance. Thanks |
Sorry for the delay. Will review this today. |
@tienifr Can you check this case too?
This case seems to be not working as expected. |
@shubham1206agra This bug happens when users update amount. So I just updated the PR to handle this case. Screen.Recording.2024-02-05.at.17.37.58.mp4 |
@tienifr Still not working (also delete). Can you pleae check again? |
@tienifr Bump on the above. |
There were some changes that caused my PR to not work. I just resolved the conflicts and test well |
@shubham1206agra Here's the evidence Screen.Recording.2024-02-15.at.11.19.15.mp4 |
@tienifr The green dot is not updating whenever you use other currency than your default currency. |
@shubham1206agra I think we can't. When using the different currency, we need to use data from BE to know the request is lower than other. Is that your mean? |
@shubham1206agra any updates? |
@shubham1206agra wdyt^? |
Just resolve the conflicts and I will do the review today. |
@shubham1206agra Can you help review this PR soon? We're fixing in IOU.ts so it's prone to get conflicts |
Reviewer Checklist
Screenshots/VideosAndroid: NativeNA Android: mWeb ChromeNA iOS: NativeNA iOS: mWeb SafariNA MacOS: Chrome / SafariScreen.Recording.2024-02-25.at.7.33.05.PM.movMacOS: DesktopScreen.Recording.2024-02-25.at.7.37.09.PM.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good to me
@tienifr can you merge main |
@mountiny I just merged main. We're good to go |
@mountiny Everything's good. We can merge this PR |
@mountiny Can you help process this PR soon to void some kind of conflicts? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.45-0 🚀
|
🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.45-6 🚀
|
This PR caused a regression: #38425. |
Details
Fixed Issues
$ #33774
PROPOSAL: #33774 (comment)
Tests
Offline tests
Same as above
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
Screen.Recording.2024-01-22.at.11.15.59.mov
iOS: Native
Screen.Recording.2024-01-22.at.10.59.30.mp4
iOS: mWeb Safari
Screen.Recording.2024-01-22.at.10.55.43.mp4
MacOS: Chrome / Safari
Screen.Recording.2024-01-22.at.10.36.01.mov
MacOS: Desktop
Screen.Recording.2024-01-22.at.10.58.21.mp4