Skip to content

Split the renderRibbon to 2 render functions changed the logic #2173

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 4 commits into from
Aug 11, 2022

Conversation

adids1221
Copy link
Contributor

Description

Fixed customRibbon props render in Avatar component

Changelog

Fixed customRibbon props render in Avatar component

Copy link
Collaborator

@ethanshar ethanshar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
wrote a small comment

@@ -384,7 +389,8 @@ class Avatar extends PureComponent<AvatarProps> {
</View>
{this.renderImage()}
{this.renderBadge()}
{this.renderRibbon()}
{hasCustomRibbon && this.renderCustomRibbon()}
{!hasCustomRibbon && hasRibbonLabel && this.renderRibbon()}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove here only the condition of !hasCustomRibbon
Basically it means it will move the responsibility to the user.
If the user pass both ribbonLabel and customRibbon they will have a UI bug and it should be their responsibility to handle it.

@adids1221 adids1221 requested a review from ethanshar August 10, 2022 11:12
@ethanshar ethanshar merged commit 12e132b into master Aug 11, 2022
@adids1221 adids1221 deleted the fix/Avatar_customRibbon branch August 24, 2022 07:33
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