feat(headless): add a Button primitive with focusableWhenDisabled - #9319
feat(headless): add a Button primitive with focusableWhenDisabled#9319alexcarpenter wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: b2fd4f9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughAdded a client-side Estimated code review effort: 3 (Moderate) | ~30 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/hip-moles-jam.md:
- Around line 1-2: Update the changeset frontmatter in
.changeset/hip-moles-jam.md to include a patch release entry for `@clerk/headless`
and add a concise release description for the new public ./button export; do not
leave the changeset empty.
In `@packages/headless/src/primitives/button/button.tsx`:
- Around line 30-32: Update suppressEvent to call event.stopPropagation()
alongside preventDefault(), ensuring disabled Button interactions do not reach
ancestor handlers. Add a regression test with a parent handler covering click,
keyboard, or pointer interaction on a focusable disabled Button.
- Around line 55-58: Update the non-native button props in
packages/headless/src/primitives/button/button.tsx:55-58 so disabled elements
use tabIndex -1 when focusableWhenDisabled is false, while preserving tabIndex 0
otherwise. In packages/headless/src/primitives/button/button.test.tsx:252-266,
render a disabled anchor with an href and verify keyboard navigation does not
focus it.
- Line 46: The Button component’s forwarded ref is restricted to
HTMLButtonElement even when nativeButton is false and useRender produces an
anchor or span. Update Button and its ref-related types to support the
non-native HTMLElement render path, and add type coverage confirming an anchor
ref is accepted.
In `@packages/headless/src/primitives/button/README.md`:
- Around line 13-16: Update the Button import in the README usage example to use
the published package subpath `@clerk/headless/button` instead of the internal
`@/primitives/button` alias, leaving the example’s component usage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b019049-b545-44db-85f8-0dddb87e2402
📒 Files selected for processing (7)
.changeset/hip-moles-jam.mdpackages/headless/package.jsonpackages/headless/src/primitives/button/README.mdpackages/headless/src/primitives/button/button.test.tsxpackages/headless/src/primitives/button/button.tsxpackages/headless/src/primitives/button/index.tspackages/headless/vite.config.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/headless/src/primitives/button/button.tsx`:
- Around line 99-104: Update the disabled handling in the Button primitive’s
merged event handlers to assign suppressEvent to merged.onPointerDown,
preventing ancestor pointerdown actions for aria-disabled buttons; add the
requested ancestor onPointerDown regression assertion in
packages/headless/src/primitives/button/button.test.tsx at lines 193-210.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 610adb7b-e5d8-49a4-81f1-7ec1744c6aae
📒 Files selected for processing (3)
packages/headless/src/primitives/button/README.mdpackages/headless/src/primitives/button/button.test.tsxpackages/headless/src/primitives/button/button.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/headless/src/primitives/button/README.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/headless/src/primitives/button/button.tsx (1)
74-85: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard non-native keyboard activation.
When
nativeButton={false}, a key event from a focusable descendant can bubble to the Button handler and activate the Button viaevent.currentTarget.click(). Return unlessevent.target === event.currentTargetin both handlers. Also skip the synthetic click whenevent.defaultPreventedis true so consumers cannot cancel activation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/button/button.tsx` around lines 74 - 85, Update the onKeyDown and onKeyUp handlers in the Button component to ignore bubbled key events unless event.target equals event.currentTarget. In onKeyUp, also skip synthetic activation when event.defaultPrevented is true, while preserving existing Space and Enter behavior for the Button itself.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/headless/src/primitives/button/button.tsx`:
- Around line 74-85: Update the onKeyDown and onKeyUp handlers in the Button
component to ignore bubbled key events unless event.target equals
event.currentTarget. In onKeyUp, also skip synthetic activation when
event.defaultPrevented is true, while preserving existing Space and Enter
behavior for the Button itself.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ed728ecd-47fa-4088-b55b-021b33ba0ae3
📒 Files selected for processing (3)
packages/headless/src/primitives/button/README.mdpackages/headless/src/primitives/button/button.test.tsxpackages/headless/src/primitives/button/button.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/headless/src/primitives/button/README.md
- packages/headless/src/primitives/button/button.test.tsx
1 participant
Description
Adds a
Buttonprimitive to@clerk/headless, exported from@clerk/headless/button.It covers the disabled behaviour a native
<button>cannot express:focusableWhenDisabled— a button that disables itself mid-interaction (while a form submits, while a request is in flight) drops focus to the body, and the user loses their place on the page. With this prop the button keeps its position in the tab order: thedisabledattribute is omitted in favour ofaria-disabled, and inertness comes from suppressingclick,keydown, andkeyupinstead. The suppressing handlers overwrite rather than chain, so a consumer'sonClicknever runs while disabled.mousedownis also suppressed, so a pointer press does not pull focus off wherever it currently sits — matching what thedisabledattribute does natively.nativeButton={false}— for use with the sharedrenderescape hatch when the rendered element is not a real<button>. Appliesrole="button", tab order, and Enter/Space activation. Without it,<Button render={<span />} />would emit atype="button"attribute on a span and be non-interactive. Enter on a link is left to the browser so activation does not fire twice.The API follows Base UI's button.
@clerk/ui's MosaicButtonis not wired up to this yet; that is stacked on top in a follow-up.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change