-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: add MultipleAlertModal component #13683
base: main
Are you sure you want to change the base?
Conversation
…tamask-mobile into feat/add-alert-modal
|
|
); | ||
}; | ||
|
||
export default MultipleAlertModal; |
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.
Hey @vinistevam : a question for my curiosity. Why is this named as MultipleAlertModal
. I was looking at it in extension and found it a bit confusing.
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.
There are 3 alert modals:
- ConfirmationAlertModal
- AlertModal
- MultipleAlertModal
I am bit confused with functionality of each.
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.
Changes look good, a query here: #13683 (comment)
Description
PR is adding the
MultipleAlertModal
component, which is part of the Alert System.Until we add an alert there is no functional change in the app.
Related issues
Fixes: https://github.com/MetaMask/mobile-planning/issues/2145
Manual testing steps
Screenshots/Recordings
multi-alert-modal.mp4
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist