UX: added badge status in account list #23006
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NidhiKJha
added
team-wallet-ux
needs-assets-ux-review
darkwing
reviewed
Feb 17, 2024
| ) { | ||
| badgeBorderColor = BorderColor.successDefault; | ||
| badgeBackgroundColor = BackgroundColor.backgroundDefault; | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we create a helper files in ui/helpers called connectivity.js where we have a few helper functions to re-use?
export function hasAnyAccountsConnected(accounts) {
}
export function getConnectivityColors(status) {
// return { borderColor: '???', backgroundColor: '???' }
}
There was a problem hiding this comment.
Added this functionality in ConnectedStatus to separate the BadgeStatus here d8f85f2
Builds ready [071d4d5]
Page Load Metrics (1054 ± 50 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
darkwing
changed the title
darkwing
requested changes
Feb 20, 2024
| @@ -0,0 +1,87 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
We should use typescript unless a compelling reason not to.
There was a problem hiding this comment.
This component is used to set the color for the badge. I have converted the badge-status to ts. But if I try to convert this one to ts. It doesn't allow me to use the Color prop for BackgroundColor. And background color doesn't have this color by default. So, that's why kept this component in JS
NidhiKJha
force-pushed
the
fix-1942-b
branch
from
071d4d5 to
d3706dd
Compare
NidhiKJha
force-pushed
the
fix-1942-b
branch
from
c111f8f to
fbed3db
Compare
|
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. |
|
This is excellent work! Wow! Needs merge conflict updates but works great! Can we get a test here? |
NidhiKJha
force-pushed
the
fix-1942-b
branch
from
fa2b09c to
dff48ce
Compare
NidhiKJha
force-pushed
the
fix-1942-b
branch
from
dff48ce to
fc280b1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #23006 +/- ##
===========================================
- Coverage 68.60% 68.59% -0.01%
===========================================
Files 1096 1097 +1
Lines 43251 43287 +36
Branches 11525 11538 +13
===========================================
+ Hits 29670 29690 +20
- Misses 13581 13597 +16 ☔ View full report in Codecov by Sentry. |
Builds ready [9584470]
Page Load Metrics (1778 ± 66 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
13 tasks
Builds ready [eae16e8]
Page Load Metrics (2827 ± 247 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
vthomas13
previously approved these changes
Feb 23, 2024
Builds ready [78480d6]
Page Load Metrics (2371 ± 103 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
darkwing
requested changes
Feb 23, 2024
| ); | ||
| }; | ||
|
|
||
| ConnectedStatus.propTypes = { |
There was a problem hiding this comment.
Can we define types instead?
ui/components/multichain/pages/send/components/account-picker.test.tsx
Outdated
Show resolved
Hide resolved
NidhiKJha
force-pushed
the
fix-1942-b
branch
from
78480d6 to
04d8a10
Compare
NidhiKJha
commented
Feb 26, 2024
| @@ -1043,6 +1074,7 @@ describe('Selectors', () => { | |||
| balance: '0x0', | |||
| pinned: true, | |||
| hidden: false, | |||
| active: false, | |||
There was a problem hiding this comment.
I have updated the updated the test, by checking the active false. For the connected one, it's true else false
Builds ready [58f9090]
Page Load Metrics (906 ± 399 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
darkwing
approved these changes
Feb 26, 2024
vthomas13
approved these changes
Feb 26, 2024
metamaskbot
added
the
release-11.13.0
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is to add badge status in the Account List
Related issues
Fixes: 1942
Manual testing steps
Screenshots/Recordings
Before
After
Popup View
Full Screen View
Pre-merge author checklist
Pre-merge reviewer checklist