Conversation
|
Spotted a bug while working on it (also reproducible on main): Screen.Recording.2025-05-21.at.16.32.11.mp4 |
|
@getusha friendly bump |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-06-02.at.12.37.43.in.the.afternoon.movAndroid: mWeb ChromeScreen.Recording.2025-06-02.at.10.26.52.in.the.morning.moviOS: HybridAppScreen.Recording.2025-06-02.at.11.51.38.in.the.morning.moviOS: mWeb SafariScreen.Recording.2025-06-02.at.10.24.55.in.the.morning.movMacOS: Chrome / SafariScreen.Recording.2025-06-02.at.10.23.25.in.the.morning.movMacOS: DesktopScreen.Recording.2025-06-02.at.10.29.00.in.the.morning.mov |
src/libs/actions/IOU.ts
Outdated
| InteractionManager.runAfterInteractions(() => removeDraftTransactions()); | ||
| if (!requestMoneyInformation.isRetry) { | ||
| dismissModalAndOpenReportInInboxTab(backToReport ?? activeReportID); | ||
| dismissModalAndOpenReportInInboxTab(backToReport ?? activeReportID, !!transaction.isFromReportsPageDragAndDrop); |
There was a problem hiding this comment.
transaction wouldn't have isFromReportsPageDragAndDrop at this point @koko57
As we are saving it to a draft transaction.
There was a problem hiding this comment.
@koko57 I might be wrong, but I assume it seems to work because !!transaction.isFromReportsPageDragAndDrop is always false. which means we wouldn't be redirected if we initiate the request on drop.
There was a problem hiding this comment.
@getusha you're not wrong - my bad, I haven't thought it through. Maybe it's just better to revert the changes I made for the Stage 2 Multi Drop. I would just leave the changes for refactoring initMoneyRequest in this PR, so I don't have to cherry-pick this commit to the Stage 4 #62650 (comment)
Now I don't have a better idea to handle this, I also see some new problems related to having several transactionDrafts - so my suggestion here - revert the change (as we're not using the Search Page drag-and-drop) and leave only the changes with initMoney refactor. I would fix this problem later, when finishing the whole feature. The original problem would be solved. WDYT?
| const prevTransactionReportID = usePrevious(transaction?.reportID); | ||
|
|
||
| // Clear out the temporary expense if the reportID in the URL has changed from the transaction's reportID. | ||
| useFocusEffect( |
There was a problem hiding this comment.
we removed this here https://github.com/Expensify/App/pull/61829/files#diff-58df38b4de0f1004e4272ddff9c794051956250a26c86027addb1a6763b09237L73-L83
could you pull main?
|
@getusha friendly bump on this one |
|
oh no, waited too long - looks like there are conflicts |
|
looks like we need a |
|
@dangrous done! |
|
✋ 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/dangrous in version: 9.1.59-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.1.59-7 🚀
|
Explanation of Change
Fixed Issues
$ #62370
PROPOSAL: -
Tests
Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2025-05-21.at.16.42.05.mp4
MacOS: Desktop