[WEB-8149] fix(badge): support icon-only badges with proper accessibility#225
[WEB-8149] fix(badge): support icon-only badges with proper accessibility#225codingwolf-at wants to merge 5 commits into
Conversation
…ries - Updated `Badge` component to support icon-only rendering by conditionally omitting the label. - Introduced new `IconOnly` and `IconOnlyInteraction` stories to demonstrate the icon-only badge behavior and ensure proper rendering. - Refactored existing badge stories to utilize a shared row layout for improved presentation. - Removed the `info` tone from badge variants to align with design specifications. - Enhanced documentation to clarify the behavior of icon-only badges and their layout.
…tency - Changed the text color for the `success` variant from `text-success-primary` to `text-success-secondary` to align with design specifications. - Updated the text color for the `warning` variant from `text-warning-primary` to `text-warning-secondary` for improved consistency across badge variants.
- Updated the `Badge` component to require an `aria-label` when using icon-only rendering, ensuring proper accessibility. - Enhanced documentation to clarify the need for an accessible name in icon-only badges. - Adjusted stories to reflect the new accessibility requirements and validate the presence of `aria-label` in tests.
- Enhanced the `Badge` component to ensure icon-only badges have a valid `role="img"` and require an `aria-label` for accessibility. - Updated documentation to clarify the accessibility requirements for icon-only badges. - Adjusted the `success` and `warning` variant text colors for consistency with design specifications, changing `text-success-secondary` to `text-success-primary` and `text-warning-secondary` to `text-warning-primary`.
codingwolf-at
changed the title
|
📚 Storybook preview: https://pr-225-propel-storybook.vamsi-906.workers.dev |
codingwolf-at
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
| grey: "bg-label-grey-bg text-label-grey-text", | ||
| brand: "bg-accent-subtle text-accent-primary", | ||
| info: "bg-info-subtle text-info-primary", | ||
| indigo: "bg-label-indigo-bg text-label-indigo-text", |
There was a problem hiding this comment.
info is an existing public BadgeTone, and the Badge guidance still lists it as a status use case. Removing it here breaks existing <Badge tone="info" /> consumers—can we retain it or make this a deliberate versioned migration?
There was a problem hiding this comment.
I'd dropped info to make the tone set exactly match the Figma spec's Color property, which lists 11 tones and doesn't include info — so it looked like a stray extra at the time.
Reverting this change to preserve the tone.
| export function Badge({ label, startIcon, endIcon, ...props }: BadgeProps) { | ||
| return ( | ||
| <BadgeElement {...props}> | ||
| <BadgeElement role={label == null ? "img" : undefined} {...props}> |
There was a problem hiding this comment.
This only documents the aria-label requirement: callers can still omit both label and aria-label, producing an unnamed role="img". Could we model the label/icon-only cases as a union so the accessible name is required here?
There was a problem hiding this comment.
Done. Modeled BadgeProps as a discriminated union so the name is required either way:
export type BadgeProps = BadgeOwnProps &
({ label: string; "aria-label"?: string } | { label?: undefined; "aria-label": string });
Labeled → named by its text; icon-only (no label) → aria-label required at compile time, so omitting both is now a type error instead of a silent unnamed role="img". Went with a union rather than the aria-hidden fallback used for Avatar because an icon-only badge is meaningful status — it should be named, not hidden. Type-check confirms all existing usages already pass a name.
|
Please update the Figma URL, as it currently points to this PR only. |
- Introduced the 'info' tone to both the `Badge` and `Badge` stories, expanding the available color options for badges. - Updated the badge variants to include the new 'info' tone, ensuring consistency with design specifications. - Enhanced documentation to reflect the addition of the new tone and its intended usage.
Done |
2 participants
Description
labelomitted): the ready-madeBadgeskips the labelslot entirely (an empty flex child would eat the pill's gap and render it lopsided) and
defaults to
role="img"so an author-suppliedaria-labelis valid ARIA — a bare<span>'sgenericrole doesn't otherwise support naming.IconOnly/IconOnlyInteractionstories (components + elements tier) covering the newaccessible-name contract, and dedupe the repeated row-layout wrapper
<div>across stories intoone shared
rowLayoutdecorator.elements/badge/variants.tsleft over from a prior refactor.Type of Change
Screenshots and Media (if applicable)
Test plan
vp check— clean (format/lint/typecheck)vp test— 116/116 story files, 796/796 tests passing (Storybook + a11y checks)vp pack/vp run -r build— library build + attw/publint cleanFigma URL