Skip to content

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

Merged
merged 7 commits into from
Feb 12, 2024
Merged

icon support in badge #2941

merged 7 commits into from
Feb 12, 2024

Conversation

adids1221
Copy link
Contributor

Description

Icon support in badge.
Added 2 new props badgeProps | hideBadge to the Icon component.

Changelog

Icon support in badge.

Additional info

@adids1221 adids1221 requested a review from Inbal-Tish February 8, 2024 13:21
@adids1221 adids1221 marked this pull request as ready for review February 8, 2024 13:21
@adids1221
Copy link
Contributor Author

adids1221 commented Feb 8, 2024

@Inbal-Tish this PR is mentioned in related PR in the private.

const containerStyle = badgeProps?.containerStyle;
const badgePosition: StyleProp<ViewStyle> = {
position: 'absolute',
top: -3
Copy link
Collaborator

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.

top: -3
};
if (isPimple()) {
badgePosition.right = -2;
Copy link
Collaborator

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?

Copy link
Collaborator

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

Copy link
Contributor Author

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.

@adids1221 adids1221 requested a review from Inbal-Tish February 11, 2024 18:03
@Inbal-Tish Inbal-Tish merged commit 6e28830 into master Feb 12, 2024
@Inbal-Tish Inbal-Tish deleted the feat/Icon_badge_support branch February 12, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants