Button uses native disabled for busy state, which drops focus
- Dominant language
- TypeScript
- Stars
- 13.1k
- Forks
- 1.1k
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 687
Description
## What happens
While a `clickAction` is pending, `Button` sets the native `disabled` attribute on the ``. A natively disabled element cannot hold focus, so the browser moves focus to `` the moment the action starts, and it is not restored when the action settles.
In `packages/core/src/Button/Button.tsx`:
```ts
const isLoadingState = isLoading || isPending;
const buttonDisabled =
isDisabled || groupDisabled || (isLoadingState && !isInterruptible);
...
disabled={useAriaDisabled ? undefined : buttonDisabled}
```
`useAriaDisabled` is only true when a `tooltip` is set, so in the common case the busy button is natively disabled.
## Why it's a problem
A keyboard user who activates "Save" with Enter loses their place: focus falls to ``, so the next Tab restarts from the top of the document, and screen readers announce nothing when the action completes. This is the classic reason busy state should never be expressed as `disabled`.
It also contradicts the documented rule in the API Conventions wiki page ("Disabled vs Busy"), which says busy is visual-only — `aria-busy` plus a spinner — and that only `isDisabled` uses the native attribute. Input components already follow that rule: they render `aria-busy` and keep the control focusable, guarding re-entry in the change handler.
## Why the native disable looks redundant
`Button` already guards re-entry in the click handler with a ref:
```ts
const actionInFlightRef = useRef(false);
if (buttonDisabled || (actionInFlightRef.current && !isInterruptible)) {
e.preventDefault();
return;
}
```
That guard is what actually makes a fire-once action fire once — it dedupes even a same-tick double click, which `disabled` does not reliably do. The native attribute adds the focus loss without adding the safety.
## Suggested fix
While busy (and not otherwise disabled):
- keep the button focusable — no native `disabled`
- keep `aria-busy="true"` and the spinner
- set `aria-disabled="true"` so AT announces the state
- suppress Enter/Space activation via the existing handler path, the way the `aria-disabled` + tooltip branch already does
- leave `isDisabled` behavior unchanged: that stays native `disabled`
`isInterruptible` can stay as-is; it would simply become the variant that also lets activation through.
## Notes
Changing this affects every consumer that relies on a busy button being unclickable, so the re-entry guard needs to cover the keyboard path too before the attribute comes off. Worth a changeset and a note in the release notes.
Contributor guide
Research direction
Start in packages/core/src/Button/Button.tsx, tracing isLoadingState, buttonDisabled, the click handler, and the existing aria-disabled path. Compare the behavior with the input components and the API Conventions wiki page’s “Disabled vs Busy” rule. Done means busy buttons retain focus, expose aria-busy and aria-disabled, block unintended keyboard re-entry, and preserve native disabled behavior for isDisabled.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- accessibility, frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100