-
Notifications
You must be signed in to change notification settings - Fork 734
icon support in badge #2941
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
icon support in badge #2941
Conversation
@Inbal-Tish this PR is mentioned in related PR in the private. |
src/components/icon/index.tsx
Outdated
const containerStyle = badgeProps?.containerStyle; | ||
const badgePosition: StyleProp<ViewStyle> = { | ||
position: 'absolute', | ||
top: -3 |
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.
why did you set it to -3 if you're changing it later? I'd set the default here, for the badge, and if it is a pimple override.
src/components/icon/index.tsx
Outdated
top: -3 | ||
}; | ||
if (isPimple()) { | ||
badgePosition.right = -2; |
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.
these values should be a const. Are they not supposed to be dynamic or they should be the same for all icon/badge sizes?
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.
I'm referring to the -2 and the -5
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.
In the last commit I added support for dynamic badge size as we spoke.
Description
Icon
support in badge.Added 2 new props
badgeProps | hideBadge
to theIcon
component.Changelog
Icon
support in badge.Additional info