-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: multichain popular network dapp connection confirmation bottomsheet #10519
feat: multichain popular network dapp connection confirmation bottomsheet #10519
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Bitrise✅✅✅ Commit hash: 27661cc Note
|
Bitrise✅✅✅ Commit hash: a1f417f Note
|
app/components/UI/NetworkVerificationInfo/NetworkVerificationInfo.tsx
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
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.
Looks good.
I think adding a test would help
Hi Mircea, for this feature, like for the network UI redesign me and Salim from assets worked on last month, David Walsh wants us to write e2e and unit test at the end, before removing the the feature flag, this way the code is done and stable and any unecessary e2e tests and unit tests are removed/replaced at the same time. And the added advantage is that any existing unit tests, including snapshots, ensure that the UI when feature flag is off, does not change until we are ready to move from the old code to the new. David is OOO this week and not there to explain this reasoning more in detail, but we did this for the network ui redesign also, still behind feature flag. |
Bitrise✅✅✅ Commit hash: f3e5853 Note
|
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.
looks good
Description
It's rather simple all it does is modify the content of an existing screen.
And it does so behind a feature flag, meaning it keeps the previous UI when the flag is false, and shows the modified version if the flag is true.
Related issues
Contributes to solve issue https://github.com/MetaMask/MetaMask-planning/issues/2803
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist