[RFC] ButtonGroup has no horizontal overflow treatment, and the workaround costs roving focus
- Dominant language
- TypeScript
- Stars
- 13.1k
- Forks
- 1.1k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
## Problem Statement
A horizontal `ButtonGroup` has no answer for running out of room. The group is `display: inline-flex` with `align-items: stretch` and nothing else — no wrap, no scroll, no collapse ([`ButtonGroup.tsx#L94-L97`](https://github.com/facebook/astryx/blob/bc39db431/packages/core/src/ButtonGroup/ButtonGroup.tsx#L94-L97)) — so a group whose buttons exceed the available width simply overruns its container and the trailing actions are unreachable. There is no prop, no documented guidance, and no escape hatch short of the consumer measuring the row themselves.
This is the ordinary case for the surfaces button groups are for: a table row's actions in a resizable column, a toolbar above a pane that a user can drag narrower, a card header on a narrow viewport. The actions that fall off the end are not decorative — they are the ones a keyboard or screen-reader user can no longer reach at all.
Every other layout row in Astryx that can outgrow its space has an answer. `OverflowList` exists and is good: it measures, collapses from either end, honours `minVisibleItems`/`maxVisibleItems`/`maxRows`, and observes either itself or its parent. `Tokenizer` already composes it. `ButtonGroup` — the one component whose entire job is a row of side-by-side controls — is the gap.
## Evidence of Demand
**We shipped a replacement rather than do without it.** I maintain `@nest/eps-core`, the EPS theme and component layer over Astryx (an internal Meta design system migrating onto Astryx). EPS's `ButtonGroup` requires overflow-to-menu, so rather than use Astryx's `ButtonGroup` we ship our own component that wraps `OverflowList` and renders the overflowing buttons into a trailing `DropdownMenu`. Its own docblock records what that cost, under a heading we called `APPROXIMATIONS`:
> - No Astryx ButtonGroup context, so its roving arrow-key focus and size-context propagation are dropped — children stay individually tabbable and set their own size.
> - Overflow items without a `label` are skipped in the menu (they'd otherwise render a menu item with no accessible name).
So the workaround trades a documented WAI-ARIA APG behaviour for a layout behaviour. That is the wrong trade to force on a consumer, and it is the demand signal: a product team chose to lose roving focus rather than lose overflow handling.
**Where the pattern appears elsewhere.** Overflow-to-menu on an action row is a standard toolbar treatment — Fluent UI ships `OverflowSet` and Fluent v9 an `Overflow`/`OverflowItem` pair explicitly for command bars; Atlaskit's `ButtonGroup` documents overflow handling; Material's toolbars specify an overflow menu as the canonical response to a narrow command row. It is a general-purpose UI pattern, not an EPS recipe.
**Frequency.** Inside EPS this is the default for horizontal groups, not an opt-in: every horizontal `ButtonGroup` call site gets the collapse behaviour.
## Why Existing Components Don't Cover This
`` inside `` is the obvious composition, and it does not work: `OverflowList` collapses *its own children*, so it would collapse the whole group as one item. Inverting it — `OverflowList` as the group, with Buttons as children — is what we actually shipped, and that is where it breaks down:
**1. It cannot be Astryx's ButtonGroup at all.** `ButtonGroupContext` is what gives children their position-aware seam and radius in pure CSS. Once `OverflowList` owns the child list, that context is gone, and with it the single-tab-stop roving focus (`useListFocus`), Home/End, disabled-skipping, and size propagation. We re-implemented the connected seam in ~60 lines of hand-written CSS keyed on `:first-child`/`:last-child`, which is precisely the structural reach [RFC #5417](https://github.com/facebook/astryx/issues/5417) is about.
**2. The trailing menu breaks `:last-child`.** The overflow trigger's `DropdownMenu` renders a native-`popover` element that stays in the DOM as the real `:last-child`, so the visible trigger is `:not(:last-child)` and gets its trailing corners squared with the interior buttons. We needed an extra class on the trigger purely to put its radius back.
**3. `OverflowList`'s container clips the focus ring — measured, and I think this one is an upstream bug in its own right.** The visible container is `overflow: hidden` ([`OverflowList.tsx#L40-L45`](https://github.com/facebook/astryx/blob/bc39db431/packages/core/src/OverflowList/OverflowList.tsx#L40-L45)). Astryx's focus ring is `2px` wide at `3px` offset (`focusDefaults`), so it paints 5px outside the element box — entirely outside a container that hugs its children. In Chromium, focusing a button inside such a container and counting ring-coloured pixels in a 12px band around it:
| container | ring pixels painted |
|---|---:|
| `overflow: hidden` (today) | **0** |
| `overflow: clip` + `overflow-clip-margin: 8px` | 344 |
The ring is not clipped to a sliver; it does not paint at all. That is a WCAG 2.4.7 failure for **any interactive `OverflowList` child**, and the only reason it has not surfaced upstream is that the sole in-repo consumer, `Tokenizer`, renders a non-interactive `+N more` span as its indicator and tokens that do not take focus. We work around it with `overflow: clip` plus `overflow-clip-margin`, which keeps the collapse clipping while letting the ring paint through. Happy to split this out as its own bug report if you'd rather keep the two apart.
## Rough Approaches Considered
Not implementation proposals — the point is to show the design space, since the interesting question here is focus, not layout.
**Option A — an `overflow` prop on `ButtonGroup`.** The group keeps owning its children and context, and internally routes through the same measurement `OverflowList` uses.
```tsx
```
Reads as a caller-owned distinction under [AST-002 DEC-1](https://github.com/facebook/astryx/blob/bc39db431/docs/specs/AST-002/spec.md#dec-1--public-props-require-a-non-derivable-caller-distinction): two identical groups genuinely need different outcomes (a 3-action row that must collapse vs. one the caller has sized and wants left alone), and the component cannot derive which. The open question is what the group puts in the menu — a `Button`'s `label` and `onClick` are enough for the common case, but not for a member that is itself a `DropdownMenu` or carries a `tooltip`.
**Option B — make the members declare their own overflow behaviour**, so the group is not guessing:
```tsx
```
**Option C — leave `ButtonGroup` alone and teach `OverflowList` to host a group**, e.g. an opt-in that forwards `ButtonGroupContext` through to both the visible and collapsed sets. Keeps the seam logic in one place, at the cost of coupling two components.
I have deliberately not built any of these. If A is the direction, the mechanism question is whether the collapse can live inside `ButtonGroup` without duplicating `OverflowList`'s measurement.
## Accessibility Considerations
This is the part I think decides the shape, and the reason this is an RFC rather than a PR.
`ButtonGroup` today is a WAI-ARIA APG roving-tabindex toolbar: one tab stop, arrows move along the orientation, Home/End jump to the ends, disabled members are skipped. Any overflow treatment has to answer questions that pattern does not currently have:
- **Does the "…" trigger join the roving order?** It is a member of the visible row, so it should — which means the group's focus model has to include a member it did not receive as a child.
- **What happens to focus when the focused button collapses?** A user arrows to "Archive", the pane is dragged narrower, and that button is now inside a closed menu. Focus cannot silently land on ``. The plausible answer is that it moves to the overflow trigger, but that needs deciding, not assuming.
- **Are collapsed members announced?** A screen-reader user should be able to tell the actions still exist. The trigger needs an accessible name that says so.
- **Reduced motion / no measurement.** Collapse must be stable when `ResizeObserver` never fires or the row has no width yet, rather than flickering members in and out.
Per [AST-002 DEC-2](https://github.com/facebook/astryx/blob/bc39db431/docs/specs/AST-002/spec.md#dec-2--caller-need-does-not-admit-an-unclear-or-unenforceable-api), an overflow prop justified with toolbar semantics has to actually deliver them — roving focus across visible *and* collapsed members, or it should not claim the pattern. Our EPS component does not clear that bar today, which is exactly why I would rather it be solved here than copied.
Also, per the measurement above: whatever hosts the collapse must not clip its children's focus rings.
## Performance Considerations
`OverflowList` renders a hidden measurement copy of every child ([`measureContainer`](https://github.com/facebook/astryx/blob/bc39db431/packages/core/src/OverflowList/OverflowList.tsx#L58-L67)), so an N-member group renders ~2N buttons. For a button group N is small (typically 2–6), and the copy is `visibility: hidden`, which keeps it out of the tab order and the accessibility tree — so no duplicate focus stops. Worth stating only because a group inside a virtualised table row multiplies it by the row count.
## Pre-submission Checklist
- [x] I have read the Contributing guide
- [x] I have read the API Conventions
- [x] I have checked that existing Astryx components cannot compose to solve this
- [x] This is a general-purpose UI pattern (not specific to one product)
Contributor guide
Research direction
Start with packages/core/src/ButtonGroup/ButtonGroup.tsx around lines 94-97 and packages/core/src/OverflowList/OverflowList.tsx around lines 40-67. Read the existing ButtonGroup focus and context behavior alongside OverflowList measurement, then resolve the RFC's API and accessibility questions, including collapsed-focus handling and focus-ring clipping. Done means an agreed design that preserves the required toolbar semantics without forcing consumers to reimplement the components.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- accessibility, design, frontend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100