-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Revert "replace onModalHide with onDismiss for the web" #15563
Conversation
@flodnv @eVoloshchak One of you needs to 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] |
Reviewer Checklist
Screenshots/VideosWebMobile Web - ChromeMobile Web - SafariDesktopiOSAndroid |
@flodnv looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
Straight revert, no need for checks to pass. |
(cherry picked from commit 0962a8c)
…ng-15563 🍒 Cherry pick PR #15563 to staging 🍒
Performance Comparison Report 📊Significant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
|
🚀 Cherry-picked to staging by https://github.com/yuwenmemon in version: 1.2.77-3 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.77-4 🚀
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀
|
The PR which we revert PR introduced a regression. We want to revert it because this has an impact on all modals, since the changes apply to BaseModal component.
Reverts #15298
Fixed Issues
$ #15559