Skip to content

feat(kumo): add disabled prop to LinkButton#535

Open
andystalick wants to merge 1 commit into
cloudflare:mainfrom
andystalick:linkbutton-disabled
Open

feat(kumo): add disabled prop to LinkButton#535
andystalick wants to merge 1 commit into
cloudflare:mainfrom
andystalick:linkbutton-disabled

Conversation

@andystalick
Copy link
Copy Markdown

Summary

Adds a disabled?: boolean prop to LinkButton with parity to Button's disabled behavior, using aria-disabled rather than swapping the rendered element type.

Behavior when disabled is true

  • Renders <a aria-disabled="true" data-disabled="true">
  • preventDefault + stopPropagation on click (blocks navigation and consumer onClick)
  • preventDefault on Enter and Space keydown (blocks keyboard activation)
  • Other keys (Tab, etc.) pass through to consumer handlers so the link stays keyboard-navigable
  • Element remains focusable so assistive technology can announce its disabled state
  • Same disabled styling as Button (shared via buttonVariants())

Why aria-disabled and not "render as <button>"

Anchors are the right semantic element for navigation triggers. Swapping the DOM node based on a runtime flag would break ref typing (HTMLAnchorElement vs HTMLButtonElement) and cause focus jumps when the prop toggles. aria-disabled is the WAI-ARIA pattern for "exists but inert" controls and matches Radix, Base UI, and the W3C ARIA APG.

Styling implementation

Extended buttonVariants() to pair every existing disabled: Tailwind class with an aria-disabled: counterpart. not-disabled:hover: selectors became not-disabled:not-aria-disabled:hover: so hover is suppressed for either kind of disabled control. The aria-disabled side gains pointer-events-none because browsers do not block clicks on aria-disabled anchors the way they do on native <button disabled>.

This keeps Button and LinkButton visually identical when disabled and centralises the styling — no per-variant drift.

Spread order (defense against bypass)

{...externalProps}
{...props}
aria-disabled={disabled || undefined}
data-disabled={disabled || undefined}
onClick={handleClick}
onKeyDown={handleKeyDown}

Consumer props are spread FIRST, then the explicit disabled-related attributes and wrapped handlers — so a consumer cannot accidentally override disabled semantics. onClick and onKeyDown are also destructured out of ...props so the wrapped handlers always own those slots and remain the only path that invokes the consumer's handler.

Tests

7 new tests in button.test.tsx cover:

  • aria-disabled and data-disabled attribute emission
  • Click blocking (consumer onClick not invoked, defaultPrevented === true)
  • Enter and Space activation blocking
  • Tab NOT being blocked (link remains focusable + consumer onKeyDown fires)
  • Regression: non-disabled onClick still fires

All 1086 kumo tests pass.

Docs

  • New LinkButtonDisabledDemo in ButtonDemo.tsx
  • New "Disabled Link" subsection on /components/button explaining the contract
  • JSDoc on LinkButtonProps.disabled documents the behavior for IDE tooltips

Known limitations (not addressed in this PR)

  • LinkButton is not a separate entry in the auto-generated component registry — it's bundled under Button. The disabled prop is documented via the new "Disabled Link" prose section, the live demo, and source JSDoc, but does not get its own row in the PropsTable. Fix would require expanding scripts/component-registry/ to extract sub-components.
  • The destructive variant with disabled (or aria-disabled) still looks visually identical to its enabled state because !text-white overrides aria-disabled:text-kumo-subtle and there's no per-variant aria-disabled:bg-kumo-danger/50. This is a pre-existing issue that also affects <Button variant="destructive" disabled> and is out of scope here.

Verification

  • pnpm typecheck → 0 errors workspace-wide
  • pnpm lint → 0 errors workspace-wide
  • pnpm --filter @cloudflare/kumo test → 1086/1086 pass
  • pnpm --filter @cloudflare/kumo-docs-astro build → success
  • Manual browser smoke test confirmed: aria-disabled="true", data-disabled="true", tabIndex=0, pointer-events: none computed, click/Enter/Space all defaultPrevented, Tab NOT prevented, focus ring visible when disabled link is focused

Changeset

minor bump for @cloudflare/kumo (additive prop, no breaking change).


  • Reviews
  • bonk has reviewed the change
  • automated review not possible because: feature work on a personal fork — bonk reviews can run after merge to main
  • Tests
  • Tests included/updated
  • Automated tests not possible - manual testing has been completed as follows:
  • Additional testing not necessary because:

Adds a `disabled?: boolean` prop to `LinkButton` with parity to `Button`'s
disabled behavior, using `aria-disabled` rather than swapping the rendered
element type.

When `disabled` is true:
- Renders `<a aria-disabled="true" data-disabled="true">`
- Calls preventDefault + stopPropagation on click (blocks navigation and
  consumer onClick)
- Calls preventDefault on Enter and Space keydown (blocks keyboard activation)
- Lets other keys (Tab, etc.) pass through to consumer handlers
- Element remains focusable so assistive technology can announce its state

Styling is shared with `Button` via `buttonVariants()`: every existing
`disabled:` Tailwind class is paired with an `aria-disabled:` counterpart,
and `not-disabled:hover:` selectors become `not-disabled:not-aria-disabled:hover:`
to suppress hover on either kind of disabled control. The `aria-disabled`
side gains `pointer-events-none` since browsers do not auto-block clicks
on aria-disabled anchors the way they do on `<button disabled>`.

The spread order in the rendered JSX places `{...externalProps}` and
`{...props}` BEFORE the explicit `aria-disabled`, `data-disabled`,
`onClick`, and `onKeyDown` attributes so that consumers cannot bypass the
disabled behavior by passing those props directly. `onClick` and
`onKeyDown` are destructured out of `...props` so the wrapped versions
always win and are the only path that invokes the consumer's handler.

Tests cover: attribute emission, click blocking, Enter and Space activation
blocking, Tab non-blocking (keeps the link focusable), and a regression
test that non-disabled `onClick` still fires.

Docs add a "Disabled Link" subsection to the Button docs page and a
`LinkButtonDisabledDemo` demo function.
// Disabled state
// Disabled state — applies to native :disabled (Button) and aria-disabled (LinkButton)
"disabled:cursor-not-allowed disabled:text-kumo-subtle",
"aria-disabled:cursor-not-allowed aria-disabled:text-kumo-subtle aria-disabled:pointer-events-none",
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may roll back pointer-events-none and allow the JS to handle things.

href,
shape = "base",
size = "base",
variant = "ghost",
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a breaking change, but why is the default ghost here when Button's default is secondary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants