feat(ButtonGroup): add XDSButtonGroup component - #2202
Conversation
|
@czarandy must be a member of the Meta Open Source team on Vercel to deploy. Learn more about collaboration on Vercel and other options here. |
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
cixzhang
left a comment
cixzhang
left a comment
There was a problem hiding this comment.
Solid architecture — context-based composition over cloneElement is the right call. A few things:
-
Add explicit
refprop — not currently declared onXDSButtonGroupProps. -
Consider
aria-orientationon the group div — quick win for screen readers with vertical groups. -
No
isDisabledon the group — XDSRadioList and XDSCheckboxList both have group-level disable. Worth adding for consistency, or documenting why it's omitted. -
Keyboard nav — roving tabindex (arrow keys between buttons) is nice-to-have for toolbar-like groups. Fine to ship without and add later, but worth tracking.
14b1458 to
2961285
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Addressed comments, including adding the keyboard nav |
Groups multiple buttons with connected styling using a context-based approach. XDSButton consumes the group context and applies position-aware border-radius and divider styles via CSS :first-child/:last-child pseudo-classes. - ref prop forwarded to root element - aria-orientation for screen readers - isDisabled cascades to all child buttons via context - Roving tabindex keyboard navigation via useListFocus - Variant-aware divider color (--color-on-accent for primary/destructive)
2961285 to
b6408b6
Compare
cixzhang
left a comment
cixzhang
left a comment
There was a problem hiding this comment.
All addressed — ref, isDisabled, aria-orientation, and roving tabindex via useListFocus. Nice.
Groups multiple buttons with connected styling using a context-based approach. XDSButton consumes the group context and applies position-aware border-radius and divider styles via CSS :first-child/:last-child pseudo-classes. - ref prop forwarded to root element - aria-orientation for screen readers - isDisabled cascades to all child buttons via context - Roving tabindex keyboard navigation via useListFocus - Variant-aware divider color (--color-on-accent for primary/destructive)
Groups multiple buttons with connected styling using a context-based approach. XDSButton consumes the group context and applies position-aware border-radius and divider styles via CSS :first-child/:last-child pseudo-classes. - ref prop forwarded to root element - aria-orientation for screen readers - isDisabled cascades to all child buttons via context - Roving tabindex keyboard navigation via useListFocus - Variant-aware divider color (--color-on-accent for primary/destructive)
2 participants
Summary
XDSButtonGroupcontainer component that groupsXDSButton/XDSIconButtonwith connected styling (shared borders, outer-only border-radius)XDSButtonGroupContextprovides orientation info, andXDSButtonconsumes it to apply position-aware styles via CSS:first-child/:last-childpseudo-classes--color-on-accentfor primary/destructive)Closes #210
Files
packages/core/src/ButtonGroup/— component, context, tests, docs, indexpackages/core/src/Button/XDSButton.tsx— group-aware styles viauseXDSButtonGroupcontextapps/storybook/stories/ButtonGroup.stories.tsx— 8 storiespackages/cli/templates/blocks/components/ButtonGroup/— showcase blockTest plan
yarn buildpassesyarn test— 8 new ButtonGroup tests pass, no regressionsyarn lint— 0 errors