-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add Badge component to UI package #73875
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
base: trunk
Are you sure you want to change the base?
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
packages/ui/src/badge/badge.tsx
Outdated
| export const Badge = forwardRef< HTMLSpanElement, BadgeProps >( function Badge( | ||
| { children, intent = 'none', render = DEFAULT_RENDER, ...props }, | ||
| ref | ||
| ) { |
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.
Nice work on this! Quick question: would it be worth considering an icon prop? I know you can achieve this with children or some other different ways, but having a dedicated prop might make it a bit more convenient for common use cases (for extenders mainly, but maybe also handy to have it for the project)
If not, maybe we could add a gap property to the styles? That way, if folks do add an icon alongside text, there'd be some opinionated spacing between them without needing custom styles. Since Badge is using inline-flex, gap should work nicely here.
What do you think? Happy to keep it simple if this feels like over-engineering!
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 think we should, but I'd prefer to handle it separately as there's some nuance to consider. Specifically; I'm unsure if there should be a system that ties specific icons (or shapes) to the different tones (like IBM Carbon), or if consumers should be allowed to supply their own icons, or both :)
My concern with custom icons is that we risk the innocent introduction of inconsistent patterns at the macro level. Also because we need to keep color blindness top of mind. Consistent icons will be an helpful status indicator for folks who cannot interpret the different colors.
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 thoughtful reply. That makes a lot of sense. I brought it up because many libraries do offer an icon option (or at least some opinionated spacing), and it can help maintain cohesion when people extend components.
From my perspective, giving extenders a bit of “opinionated freedom” could support a more unified WordPress experience, especially as we shape the future admin design language. But completely agree this is something we can iterate on separately, and I appreciate the nuance around consistency and accessibility. Nice work!
Co-authored-by: Andrew Duthie <[email protected]>
Co-authored-by: Andrew Duthie <[email protected]>
Co-authored-by: Andrew Duthie <[email protected]>
aduth
left a comment
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.
LGTM 👍 Love this as a validation of Box and, inversely, as a checklist of things we should aim to target with typography tokens.
I'm also eager to see this as a replacement for the private Badge component in @wordpress/components (i.e. replace existing usage and remove that component from @wordpress/components). It'd be good to track that in an issue.
aduth
left a comment
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.
We need to add an export to packages/ui/src/index.ts to make the component available for usage from the package.
Related: #73928, since I only just noticed this after realizing we're already not exporting the |
| padding={ { inline: '2xs' } } | ||
| borderRadius="md" | ||
| render={ render } | ||
| style={ { |
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 feel like we should use a stylesheet instead of inlining all these?
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 expected we'd be able to get rid of most of them through e.g. the Text component in #73931.
There's a few that wouldn't be covered:
displayandalignItems: I'm not sure we need this anymore after the explicitmin-heightwas removed in 46aa19eboxSizing: This feels like it could be common enough that maybe we'll create a shared utility for it
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.
display, alignItems, and boxSizing are gone. Any preference on moving the remaining styles to a stylesheet, leaving them as-is, or waiting for Text (#73931)?
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 agree with @mirka in having a light preference to a stylesheet since it's a simple shift and general preference, but honestly I think it's fine to just leave these since we'll refactor them away after Text in the next few days.
| backgroundColor: 'neutral-strong', | ||
| color: 'neutral', | ||
| borderColor: 'neutral', | ||
| borderWidth: 'xs', |
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.
This selective border is making the badge's height change depending on whether it has an intent or not (20 vs 22px). Usually we add a transparent border to all the variants in these cases.
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.
Oh boy this is a fun one. Transparent borders will make all badges 22px tall which we don't want as it's not aligned with the 4px baseline. We might have to set a fixed height, which could actually act as a helpful forcing function given they shouldn't ever really contain enough content to wrap anyway.
Edit: Although if we set a fixed height then density will be ignored, unless we add height to the dimension tokens.
Yeah, it's an interesting problem. We could probably solve this with I think a fundamental issue is the combination of border tokens (1px) and spacing tokens (4px-based) resulting in heights that aren't going to align to the 4px base unit. Couple further thoughts:
|
|
Flaky tests detected in 5ed8020. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20173987367
|
|
I'd welcome @poligilad-auto's thoughts on this detail; the no-tone badge is 2px larger than the others due to its border. Do you think this is problematic? If so how do you feel about removing the border like the mockup in this comment? I can go either way to be honest. @aduth You're right about the fundamental issue, and I suspect we'll bump into this again later :) It might be something we want |

Why
We need a standardized Badge component for displaying labels with semantic intent across the WordPress admin interface. Badges help communicate status, priority, and meaning through color-coded visual indicators, improving information hierarchy and user comprehension.
Why
This implementation builds on the
Boxprimitive to ensure consistency with the design token system and maintain alignment with the broader UI architecture. By leveragingBox, the Badge automatically inherits design token support for colors, spacing, and borders, ensuring it stays in sync with design system updates.The implementation uses directional padding (
inline: 'xs') to apply horizontal spacing only, and maps semantic intent values (high,medium,low,stable,informational,draft,none) to appropriate design token colors, ensuring accessibility and visual consistency.How
Badgecomponent inpackages/ui/src/badge/that wraps theBoxprimitiveintentprop with seven semantic values, each mapping to appropriatebackgroundColor,color,borderColor, andborderWidthvalues from the design token systemxstoken applied inline-only via directional paddinglgtokenhigh→error,stable→success)spanelement (inline) but supports custom rendering viarenderpropComponentProps<'span'>for HTML attribute supportThe component is exported from the main UI package index and follows the same patterns as other UI package components like
Stack.Testing
npm run storybook