-
-
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: hide alert when navigating to protect wallet #7534
Conversation
…react native testing library, convert it to a functional component, and added the current adopted structure folder, fixed the bug when navigating to the protect wallet flow, the allert will not be visible
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. |
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7534 +/- ##
==========================================
+ Coverage 40.33% 40.50% +0.17%
==========================================
Files 1235 1241 +6
Lines 29949 29980 +31
Branches 2875 2881 +6
==========================================
+ Hits 12079 12144 +65
+ Misses 17175 17136 -39
- Partials 695 700 +5 ☔ View full report in Codecov by Sentry. |
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Kudos, SonarCloud Quality Gate passed! |
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cdfda616-4a73-4878-be99-1441696980e4 |
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/983ce1bd-ef0f-4581-b1ff-7928ddb7061d |
Had a chance to take a quick look here and am confident that this addresses the issue reported. Specifically, tested with the SRP backup demand flow and was pleased with the result. Tested commit 55d3aee on Samsung a515f with Android 12. Manual testing steps for SRP backup demand
|
233eb00
to
890a6a2
Compare
…alert to replace deprecated component
Actually there is a new Bell Icon already, will ask for design-qa, updated the final result on the description |
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 1 New issue |
@tommasini Looks good to me for now! Thank you for checking with me. Later we can also update this to use our DS banner alert component |
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.
Codewise it looks great Tomas! I dm'd you a couple of questions regarding the context of some choices
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
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/51d64361-f28e-47b5-914a-0c8ab7dda971 |
Description
This PR aims to solve a bug on the backup alert, when navigating to the protect your wallet screen the alert kept being visible.
This PR contains BackupAlert converted to typescript and functional component.
Manual testing steps
_1. Step1: Create a new wallet
_2. Step2: Do not protect your wallet pressing (remember later)
_3. Step3: Press protect your wallet on the backup alert
_4. Step4: The protect your wallet should not be there
Screenshots/Recordings
https://recordit.co/skV2sxdzil
new Bell Icon:
Before
[screenshot]
After
[screenshot]
Related issues
_Fixes #4400
Pre-merge author checklist
Pre-merge reviewer checklist