Update badge component #1019
Update badge component #1019
Conversation
almsh
requested review from
andrashee,
mkue,
ssandino and
socialincome-dev
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces comprehensive updates to the Changes
Suggested Labels
Suggested Reviewers
Possibly Related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
ui/src/components/badge.tsx (1)
7-34: LGTM! Consider standardizing opacity values.The variants are well-structured with consistent interactive states. However, opacity values vary between variants (10%, 50%, 100%).
Consider standardizing to:
- interactive: 'border-transparent bg-primary bg-opacity-10 text-primary hover:bg-opacity-100 hover:text-white focus:ring-2 focus:ring-ring focus:ring-offset-2', - 'interactive-accent': 'border-transparent bg-accent bg-opacity-50 text-primary hover:bg-opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2', + interactive: 'border-transparent bg-primary bg-opacity-20 text-primary hover:bg-opacity-100 hover:text-white focus:ring-2 focus:ring-ring focus:ring-offset-2', + 'interactive-accent': 'border-transparent bg-accent bg-opacity-20 text-primary hover:bg-opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2',website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerBadges.tsx (2)
Line range hint
45-59: Consider using interactive variants for consistency.The badges inside HoverCardContent could use interactive variants for consistent behavior.
- <Badge variant='faded' className="text-popover-foreground"> + <Badge variant='interactive' className="text-popover-foreground"> - <Badge variant="accent" className="bg-opacity-10 text-popover-foreground"> + <Badge variant="interactive-accent" className="text-popover-foreground"> - <Badge variant='secondary' className="bg-opacity-10 text-popover-foreground"> + <Badge variant='interactive-secondary' className="text-popover-foreground">
Line range hint
105-113: TODO comment needs attention.The TODO comment about HoverCardContent should be addressed.
Would you like me to help implement the HoverCardContent for the FundraiserBadge?
ui/src/stories/badge.stories.tsx (1)
165-168: Document icon size constraints.Consider adding documentation about recommended icon sizes for each badge size variant.
website/src/app/[lang]/[region]/(website)/transparency/recipient-selection/[currency]/(sections)/selection-process.tsx (1)
111-111: Specify badge variant explicitly.Consider adding a variant prop to make the styling intention clear. For example:
-<Badge size="md"> +<Badge variant="default" size="md">Also applies to: 145-145
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ui/src/components/badge.tsx(1 hunks)ui/src/stories/badge.stories.tsx(2 hunks)website/src/app/[lang]/[region]/(website)/about-us/(sections)/landing-page.tsx(2 hunks)website/src/app/[lang]/[region]/(website)/our-work/(sections)/our-work.tsx(1 hunks)website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerBadges.tsx(4 hunks)website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerHome.tsx(2 hunks)website/src/app/[lang]/[region]/(website)/transparency/recipient-selection/[currency]/(sections)/selection-process.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test website
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (9)
ui/src/components/badge.tsx (2)
38-40: LGTM! Well-typed interface.The BadgeProps interface correctly extends HTMLAttributes and includes proper typing for the size prop.
42-43: LGTM! Clean implementation.The Badge component implementation is concise and follows React best practices.
website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerBadges.tsx (1)
24-24: LGTM! Clean size handling.Good use of the new size prop with conditional logic.
Also applies to: 30-34
website/src/app/[lang]/[region]/(website)/about-us/(sections)/landing-page.tsx (1)
Line range hint
30-34: LGTM! Consistent use of interactive variants.Both badges correctly use the interactive-muted variant with consistent styling.
Also applies to: 55-59
website/src/app/[lang]/[region]/(website)/our-work/(sections)/our-work.tsx (1)
Line range hint
30-35: LGTM! Appropriate use of interactive-outline variant.The badge correctly uses the interactive-outline variant for the video trigger.
ui/src/stories/badge.stories.tsx (3)
9-20: Well-structured argTypes configuration!The configuration provides clear descriptions and appropriate controls for both variant and size props.
53-53: Complete the Figma design link.The Figma link appears to be truncated. Please provide the complete URL.
Line range hint
61-150: Excellent story coverage!The stories effectively demonstrate all badge variants, sizes, and common use cases.
website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerHome.tsx (1)
104-104: Correct implementation of the new Badge API!The badge correctly uses the new variant and size props while maintaining the space-x-2 utility class.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
ui/src/stories/badge.stories.tsx (4)
66-66: Complete the Figma design link.The Figma link appears to be truncated.
101-109: Consider using a more semantic className for the container.The flex container could benefit from a more descriptive className.
- <div className="flex flex-wrap items-center gap-4"> + <div className="flex flex-wrap items-center gap-4 badge-sizes-demo">
140-171: Add aria-labels for better accessibility.Consider adding aria-labels to the badge groups to improve screen reader experience.
- <div className="flex gap-2"> + <div className="flex gap-2" aria-label="Status indicators">
219-226: Make icon sizes dynamic based on badge size.Currently, icons have fixed sizes. Consider adjusting icon sizes based on the badge size prop.
- <CheckCircleIcon className="mr-1 h-4 w-4" /> + <CheckCircleIcon className={`mr-1 ${args.size === 'lg' ? 'h-5 w-5' : args.size === 'sm' ? 'h-3 w-3' : 'h-4 w-4'}`} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ui/src/components/badge.tsx(1 hunks)ui/src/stories/badge.stories.tsx(2 hunks)website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerBadges.tsx(4 hunks)website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerHome.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerHome.tsx
- website/src/app/[lang]/[region]/(website)/partners/(components)/PartnerBadges.tsx
- ui/src/components/badge.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test website
- GitHub Check: Prettify
🔇 Additional comments (5)
ui/src/stories/badge.stories.tsx (5)
9-32: Well-structured argTypes configuration with clear descriptions.The configuration provides good control options for both variant and size props.
68-71: LGTM! Clean implementation of the Default story.
74-96: Good documentation and implementation of interactive variants.
Line range hint
114-129: Excellent showcase of all available variants.
237-244: Good demonstration of custom styling possibilities.
|
nice👌 @ssandino, should we update the styling of the colors on the yellow badge? Both, black on yellow and blue on yellow are imo not ideal for readability/accessibility. |
|
@mkue yes, I think we can optimize various smaller elements, badges included. Would do this in a future commit (needs some playing around). Now with Storybook this becomes much easier, as we have a view of all different styles and variants. |
Summary by CodeRabbit
Release Notes: Badge Component Update
New Features
faded,interactive, and interactive state variants.Improvements
Changes