-
Notifications
You must be signed in to change notification settings - Fork 5k
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: confirmation badge - max unlock request badge size #26162
fix: confirmation badge - max unlock request badge size #26162
Conversation
I don't see a reason for the unlock counter to be >1. All unlock counts will vanish as soon as the wallet is unlocked
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. |
Quality Gate passedIssues Measures |
const unlockCount = Math.min( | ||
controller.appStateController.waitingForUnlock.length, | ||
1, | ||
); |
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.
Context: This waitingForUnlock
is an array of promises from requests internally (any maybe externally) that require the wallet to be unlocked. As soon as the wallet is unlocked, all of these promises get resolved.
From a UI perspective, I don't see why we need to have more than 1 unlock count - especially as all the requests will get resolved once the wallet is unlocked.
Otherwise we can add a larger bound here. This will prevent the unlock requests count from growing (e.g. prevent 1000+ count on the confirmations badge)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #26162 +/- ##
===========================================
- Coverage 69.94% 69.94% -0.00%
===========================================
Files 1409 1409
Lines 49794 49795 +1
Branches 13769 13769
===========================================
Hits 34825 34825
- Misses 14969 14970 +1 ☔ View full report in Codecov by Sentry. |
Builds ready [537d8a5]
Page Load Metrics (317 ± 299 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
This might already be part of the PR, but adding this here in case it's not: we should also prevent the notification window from popping up to users requesting them to unlock when those calls are made while the wallet is locked. |
Good callout - particularly for the issue line:
This may be pre-installed snaps related - this will come under a separate notifications related PR. Will investigate to see if it resolves the issue. |
Description
I don't see a reason for the unlock counter to be >1. All unlock counts will vanish as soon as the wallet is unlocked.
Otherwise I am open to adding a larger bounds (but this will ideally prevent the counter from having 1000+ count on the fox counter for confirmations)
Related issues
Fixes: #26064
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist