-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Migrate useBeforeRemove to usePreventRemove in DiscardChangesConfirmation
#78291
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@DylanDylann 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] |
|
@DylanDylann oops looks like melvin bumped the wrong guy 💀 @sobitneupane should be reviewing this. |
|
Tried to assign @sobitneupane but they're OOO til Monday (and removed from the C+ GH team) so I can't. If this is timely, post in #contributor-plus for help |
|
No product review required. Removing myself and unsubscribing |
|
Sorry for the delay. I was OOO. I’m back now and will try to review the PR by tomorrow. |
|
Asking for confirmation here before resolving conflicts |
|
Conflicts resolved. |
|
Reviewing the PR... |
sobitneupane
left a comment
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.
Could you please remove the console logs and any unnecessary changes?
|
@mkzie2 Could you please add tests to cover all the changes introduced in the PR? Current tests only cove.r the change in description. |
|
@sobitneupane I updated the test steps. |
Explanation of Change
This PR replaces the usage of
useBeforeRemovewithusePreventRemoveinDiscardChangesConfirmationto prevent issue where the logics is not properly executed with IOS swipe back gestureFixed Issues
$ #76326
PROPOSAL: #76326 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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
Screen.Recording.2025-12-23.at.00.24.47.mov
Android: mWeb Chrome
Screen.Recording.2025-12-23.at.00.25.09.mov
iOS: Native
Screen.Recording.2025-12-23.at.00.23.46.mov
iOS: mWeb Safari
Screen.Recording.2025-12-23.at.00.25.31.mov
MacOS: Chrome / Safari
Screen.Recording.2025-12-23.at.00.22.11.mov