-
-
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
Component/4080-badge #4841
Component/4080-badge #4841
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. |
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.
Left some comments - LGTM after!
app/component-library/components/Avatars/AvatarBase/AvatarBase.types.ts
Outdated
Show resolved
Hide resolved
app/component-library/components/Avatars/AvatarBase/AvatarBase.tsx
Outdated
Show resolved
Hide resolved
app/component-library/components/Avatars/AvatarBase/AvatarBase.styles.ts
Outdated
Show resolved
Hide resolved
app/component-library/components/Avatars/AvatarBase/AvatarBase.types.ts
Outdated
Show resolved
Hide resolved
app/component-library/components/Avatars/AvatarBase/AvatarBase.styles.ts
Outdated
Show resolved
Hide resolved
app/component-library/components/Avatars/AvatarBase/AvatarBase.styles.ts
Outdated
Show resolved
Hide resolved
…to component/4080-badge
…tamask-mobile into component/4080-badge
I see that only the AvatarBase has the ability to position the badge. Is BadgeWrapper not allowed to position the badge other than on Top Right corner? |
As of right now, the positioning preset for the badge can only apply to avatar badges. In the design spec, the positioning of top: -4 and right: -4 for TopRight and bottom: -4 and right: -4, can ONLY apply to an XS size avatar. If you place a Tag as a badge with badgeWrapper with the preset positioning, it does not look good and requires different positioning. As such, I've decided to keep the current positioning logic with the AvatarBadge until the designers can fully figure out a positioning logic/formula that can be applied to ANY badge. |
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.
Thanks for the explanation @brianacnguyen
Looks good to me!
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.
1 or 2 comments - LGTM!
app/component-library/components/Avatars/AvatarBase/AvatarBase.styles.ts
Outdated
Show resolved
Hide resolved
app/component-library/components/Badges/Badge/variants/BadgeNetwork/README.md
Outdated
Show resolved
Hide resolved
…to component/4080-badge
e8eda9b
to
d726caf
Compare
…to component/4080-badge
Description
Screenshots/Recordings
Issue
Progresses #???
Checklist