-
-
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
fix: notifications first round of tests #10668
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: 59f2c59 Note
|
Unit tests are failing! Can you update the description with what the issues that were discovered and what the PR is aiming to fix? It will help the reviews |
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.
LGTM!
6d498e1
44a1a2a
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@metamask/snaps-execution-environments@6.5.0 |
Quality Gate passedIssues Measures |
Description
This PR introduces smalls fixes on Notifications feature after the first round of tests.
[x] - Error when enabling Notifications due embedded Snap Execution Environment. Changed to use external URL.
[x] - Misalignment on UI components like Loading and Notifications Header.
[x] - Bug
Cannot update a component while rendering a different component
due unnecessary usage of navigationsOptions on Notifications Details page.[x] - Fix async dependency on Notification Enable toggle & Alert.
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist