Uh oh!
There was an error while loading. Please reload this page.
feat(ui): add Mosaic SubmitButton - #9342
Conversation
A `Button` that defaults `type` to `submit` and takes `isPending`. While pending it fades its label to zero opacity and centers a spinner over it, so the button keeps its width and nothing around it reflows. The pending state applies immediately; only the spinner is delayed, so a fast action never flashes one. `spinDelay` tunes both ends of that window. The button goes inert via `aria-disabled` rather than the `disabled` attribute, so it stays focusable and its progressbar stays announced. Brings `Spinner` and `useSpinDelay` over from carp/account-button-switcher.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 86554aa 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 |
@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/vuecommit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
The wrapper is where a consumer's children actually land, so it carries `cl-button-content` rather than being reachable only through StyleX atoms.
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdded Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/tender-months-attack.md:
- Around line 1-2: Replace the empty Changeset front matter with a minor release
entry for the `@clerk/ui` package, preserving the standard Changesets format so
the new public SubmitButton and Spinner exports produce a package version and
changelog entry.
In `@packages/ui/src/mosaic/components/button/submit-button.test.tsx`:
- Around line 8-9: Update the comment above the spinner helper in the
submit-button tests to reflect that SubmitButton’s spinner exposes
role="progressbar" and the pending label, while the helper still queries its
slot class. Do not describe the spinner as decorative or lacking an accessible
role or name.
In `@packages/ui/src/mosaic/components/button/submit-button.tsx`:
- Around line 101-115: Move the pending Spinner announcement out of the button
into a sibling live region, while retaining aria-busy on the button. Update the
isPending/showPending rendering in SubmitButton so the pendingLabel and
progressbar role are exposed by the sibling region rather than nested under the
native button, preserving the existing pending visibility and spinner styling
behavior.
🪄 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: 26b1dbe3-6b22-4919-8b5f-496dc33a61e6
📒 Files selected for processing (16)
.changeset/tender-months-attack.mdpackages/swingset/src/stories/button.mdxpackages/swingset/src/stories/button.stories.tsxpackages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/button/button.tsxpackages/ui/src/mosaic/components/button/index.tspackages/ui/src/mosaic/components/button/submit-button.styles.tspackages/ui/src/mosaic/components/button/submit-button.test.tsxpackages/ui/src/mosaic/components/button/submit-button.tsxpackages/ui/src/mosaic/components/spinner/index.tspackages/ui/src/mosaic/components/spinner/spinner.styles.tspackages/ui/src/mosaic/components/spinner/spinner.test.tsxpackages/ui/src/mosaic/components/spinner/spinner.tsxpackages/ui/src/mosaic/hooks/__tests__/useSpinDelay.test.tspackages/ui/src/mosaic/hooks/useSpinDelay.tspackages/ui/src/mosaic/styles/index.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)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
| --- | ||
| --- |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add a @clerk/ui release entry.
This PR adds public SubmitButton and Spinner exports. The empty Changeset creates no package version or changelog entry. Add a minor @clerk/ui Changeset.
As per coding guidelines, “Use Changesets for version management and changelogs.” Based on learnings, an empty Changeset is acceptable only when no published package release is involved.
Proposed Changeset
---
+'`@clerk/ui`': minor
---
++Add Mosaic SubmitButton and Spinner components.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- | |
| --- | |
| --- | |
| '`@clerk/ui`': minor | |
| --- | |
| Add Mosaic SubmitButton and Spinner components. |
🤖 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 @.changeset/tender-months-attack.md around lines 1 - 2, Replace the empty
Changeset front matter with a minor release entry for the `@clerk/ui` package,
preserving the standard Changesets format so the new public SubmitButton and
Spinner exports produce a package version and changelog entry.
Sources: Coding guidelines, Learnings
| /** The spinner is decorative, so it has no role or name to query — only its slot class. */ | ||
| const spinner = () => document.querySelector('.cl-spinner'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the stale helper comment.
The comment states that the spinner is decorative and has no role or name. In SubmitButton the spinner sets role='progressbar' and aria-label={pendingLabel}, and the tests at lines 75 and 90 query it by role and name. The comment is copied from spinner.test.tsx, where it is accurate.
📝 Proposed comment fix
-/** The spinner is decorative, so it has no role or name to query — only its slot class. */+/** Queried by slot class, since the assertions below cover both its hidden and revealed states. */
const spinner = () => document.querySelector('.cl-spinner');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** The spinner is decorative, so it has no role or name to query — only its slot class. */ | |
| constspinner=()=>document.querySelector('.cl-spinner'); | |
| /** Queried by slot class, since the assertions below cover both its hidden and revealed states. */ | |
| constspinner=()=>document.querySelector('.cl-spinner'); |
🤖 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/ui/src/mosaic/components/button/submit-button.test.tsx` around lines
8 - 9, Update the comment above the spinner helper in the submit-button tests to
reflect that SubmitButton’s spinner exposes role="progressbar" and the pending
label, while the helper still queries its slot class. Do not describe the
spinner as decorative or lacking an accessible role or name.
Source: Coding guidelines
| {isPending || showPending ? ( | ||
| <Spinner | ||
| // The indicator has to be in the accessibility tree the moment the button goes | ||
| // pending, so it opts out of `Spinner`'s decorative default and carries an | ||
| // indeterminate progressbar role. It is named in its own right rather than folded | ||
| // into the button's name: `progressbar` is a range role, so the name computation | ||
| // takes its value — absent, since it is indeterminate — over its label, and a | ||
| // descendant one contributes nothing to the button above it. | ||
| role='progressbar' | ||
| aria-hidden={undefined} | ||
| aria-label={pendingLabel} | ||
| size={spinnerSizes[size]} | ||
| {...stylex.props(styles.spinner, !showPending && styles.spinnerHidden)} | ||
| /> | ||
| ) : null} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
ARIA button role presentational children: are descendant roles like progressbar exposed inside a button in browsers?
💡 Result:
No, descendant roles like progressbar are not exposed inside a button to assistive technologies [1][2]. The button role is defined in WAI-ARIA as having children that are strictly presentational [1][2]. To compensate for the fact that accessibility APIs generally cannot represent semantic elements (such as a progressbar, heading, or link) inside a button, browsers automatically treat all descendant elements of a button as having role=presentation [1][2]. When a descendant element is treated as presentational, its semantic role and ARIA attributes are hidden from the accessibility tree, effectively rendering it as if it were simple text or a generic container [1][3]. While the text content of these descendants remains visible to screen reader users, their structural and interactive roles do not [1][4]. It is considered an authoring error to nest interactive elements or elements with significant semantics (like a progressbar) inside a button [5][6]. If an element inside a button is focusable (for example, if it has a tabindex), browsers may sometimes inconsistently expose the role due to conflict resolution rules, but this behavior is discouraged and can lead to severe accessibility issues, including broken keyboard navigation and confusion for assistive technology users [5][4][6]. If you need to indicate a loading state within a button, it is standard practice to use techniques like aria-busy="true" on an appropriate container, rather than nesting a semantic progressbar role inside the button [7][8][9].
Citations:
- 1: https://www.w3.org/WAI/ARIA/apg/practices/hiding-semantics/
- 2: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/button_role
- 3: https://lists.w3.org/Archives/Public/public-aria/2016Jan/0000.html
- 4: Children presentational true element nesting another control injected via JavaScript w3c/aria#2406
- 5: Descendants of buttons with "Children Presentational: True"? w3c/aria#1174
- 6: https://www.w3.org/WAI/standards-guidelines/act/rules/307n5z/proposed/
- 7: https://stackoverflow.com/questions/78839671/which-aria-role-should-a-loading-button-have
- 8: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/progressbar_role
- 9: https://www.w3.org/WAI/WCAG22/Techniques/aria/ARIA25
🏁 Script executed:
#!/bin/bashset -eu
printf'%s\n''--- submit button ---'
cat -n packages/ui/src/mosaic/components/button/submit-button.tsx | sed -n '1,160p'printf'%s\n''--- related tests ---'
fd -i 'submit-button.test.tsx'. -x sh -c 'cat -n "$1" | sed -n "1,140p"' sh {}
printf'%s\n''--- button call sites and accessibility patterns ---'
rg -n --glob '*.{ts,tsx}'"SubmitButton|aria-busy|role=['\"]progressbar['\"]|aria-live" packages/ui/src/mosaic | head -200Repository: clerk/javascript
Length of output: 20751
🏁 Script executed:
#!/bin/bashset -eu
printf'%s\n''--- button implementation ---'
cat -n packages/ui/src/mosaic/components/button/button.tsx | sed -n '1,220p'printf'%s\n''--- spinner implementation ---'
cat -n packages/ui/src/mosaic/components/spinner/spinner.tsx | sed -n '1,180p'printf'%s\n''--- pending-related tests ---'
cat -n packages/ui/src/mosaic/components/button/submit-button.test.tsx | sed -n '140,230p'Repository: clerk/javascript
Length of output: 10038
🌐 Web query:
site:w3.org WAI-ARIA button role descendants presentational progressbar live region button loading pendingLabel
💡 Result:
In WAI-ARIA, the button role is defined as having presentational children, meaning that all DOM descendants of an element with role="button" are treated as presentational [1][2][3]. User agents are required to hide the semantics of these descendant elements from accessibility APIs [1][2]. Key implications of this behavior include: 1. Presentational Children and Semantics: When an element has the button role, its descendants (such as a nested div, span, or complex structure) are effectively stripped of their own semantic roles [2]. For example, a heading or a list inside a button will not be announced as such by screen readers; they are treated as plain text or part of the button's accessible name [2]. 2. Focusable Descendants: Because descendants are presentational, they must not be focusable [4]. Including interactive elements (like a link or another button) inside a button is a violation of accessibility standards because the inner elements cannot be exposed to the accessibility tree or reliably focused [4]. 3. Accessible Name Computation: The button's accessible name is typically computed from its descendant content [5][6]. If you use aria-label or aria-labelledby on the button, it overrides the default behavior and hides the descendant content from accessibility APIs [6]. 4. Live Regions and Progressbars: You cannot meaningfully nest a live region or a progressbar inside a button [2][4]. Because the button forces its children to be presentational, a progressbar (or any live region) placed as a descendant will lose its semantic role and its functional ability to communicate updates to assistive technology [2]. If a button needs to indicate a loading state (e.g., using a progressbar or pending label), the recommended pattern is to manage the state externally or update the button's label text dynamically (e.g., changing "Submit" to "Loading...") and utilizing aria-busy="true" or an external status element [5][7][8].
Citations:
- 1: https://www.w3.org/TR/aria-role/roles
- 2: https://www.w3.org/WAI/ARIA/apg/practices/hiding-semantics/
- 3: https://www.w3.org/TR/wai-aria-1.2/
- 4: https://www.w3.org/WAI/standards-guidelines/act/rules/307n5z/
- 5: https://www.w3.org/WAI/ARIA/apg/patterns/button/
- 6: https://www.w3.org/WAI/ARIA/apg/practices/names-and-descriptions/
- 7: https://www.w3.org/TR/wai-aria/states%5Fand%5Fproperties
- 8: https://www.w3.org/WAI/WCAG22/Techniques/client-side-script/SCR14
🏁 Script executed:
#!/bin/bashset -eu
python3 - <<'PY'from pathlib import Pathimport resource = Path("packages/ui/src/mosaic/components/button/submit-button.tsx").read_text()button = re.search(r"<Button\b(?P<body>.*?)</Button>", source, re.S)assert button, "SubmitButton must render a Button"body = button.group("body")assert "aria-busy={isPending || undefined}" in bodyassert "role='progressbar'" in bodyassert "aria-label={pendingLabel}" in bodyassert body.index("role='progressbar'") > body.index("<Spinner")print("SubmitButton renders a native Button with a nested, named progressbar and aria-busy.")PYRepository: clerk/javascript
Length of output: 239
Move the pending announcement outside the button. A native button makes its descendants presentational, so the nested progressbar and pendingLabel are not exposed reliably. Use a sibling live region and retain aria-busy on the button; the Testing Library role test does not model this browser behavior.
🤖 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/ui/src/mosaic/components/button/submit-button.tsx` around lines 101
- 115, Move the pending Spinner announcement out of the button into a sibling
live region, while retaining aria-busy on the button. Update the
isPending/showPending rendering in SubmitButton so the pendingLabel and
progressbar role are exposed by the sibling region rather than nested under the
native button, preserving the existing pending visibility and spinner styling
behavior.
Source: Coding guidelines
| // `isPending` drives the semantics, this drives the pixels. The split is deliberate: the button | ||
| // has to go inert and start announcing the moment the action does, or a fast action gets | ||
| // submitted twice and assistive tech misses it — but drawing a spinner that fast only produces | ||
| // a flash, so the visual waits out the delay and then sticks around long enough to be read. | ||
| const showPending = useSpinDelay(isPending || null, { delay, minDuration }) !== null; |
| // rather than on `useSpinDelay` itself: a button is pressed and watched, so it wants a tighter | ||
| // window than a hook shared with background loads. `minDuration` has no such tension, so it takes | ||
| // the hook's default. | ||
| const DEFAULT_SPIN_DELAY = 300; |
There was a problem hiding this comment.
I am curious if this delay could be a little shorter, but I think we will discover that through usage
There was a problem hiding this comment.
yeah, I am inclined to reduce it as well! will tweak values once we start implemnting in the user button 👍🏼
Description
Preview: https://swingset-git-carp-mosaic-submit-button.clerkstage.dev/components/button
Adds
SubmitButtonto Mosaic: aButtonthat defaultstypetosubmitand takes anisPendingprop.While pending, the label fades to zero opacity and a spinner centers over it. The label stays mounted rather than being swapped out, so the button holds the width its content gives it and nothing around it reflows when the state flips. Every child sits in one box, so an icon fades with its label instead of hanging on beside the spinner. Text runs still get their own truncatable box inside that wrapper.
The pending state is immediate —
aria-busy,aria-disabled,data-pending, pointer events dropped, press cancelled — but the spinner waits 300ms before it's drawn and stays up at least 200ms once it is, so an action that resolves quickly never flashes one.spinDelaymoves both numbers, or removes the wait entirely with{ delay: 0 }.Accessibility:
progressbarthe momentisPendingflips, including during the delay when it's mounted but not yet drawn. That's why the delay isopacityand not conditional rendering — not rendering it removes it from the tree just asdisplay: nonewould.aria-disabledrather than thedisabledattribute, so it stays focusable and doesn't drop focus mid-action just as the progressbar is announced. The press is cancelled in JS instead. Marked with a TODO to fold into the headless button'sfocusableWhenDisabledfrom feat(headless): add a Button primitive with focusableWhenDisabled #9319 / feat(ui): wire Mosaic Button to the headless Button with focusableWhenDisabled #9320 once those land.pendingLabelrather than folded into the button's name:progressbaris a range role, so name computation reads its value — absent, since it's indeterminate — over its label, and a descendant one contributes nothing to the button above it.Buttongates its hover and pressed fills on:enabled, which a pending button still is. Pointer events are dropped for the pointer half; the 9 active-fill selectors now exclude[data-pending]for the keyboard half, since a focused button takes:activefrom space and enter with no pointer involved.SpinneranduseSpinDelaycome over fromcarp/account-button-switcher. One divergence to reconcile when that branch merges:spinner.styles.tsderives both colors fromcurrentColorinstead of the fixed--cl-color-card-foregroundpair, which would be invisible on afilled-primarybutton.Swingset's button page documents all of it, with a pressable example and a slow/fast pair showing the delay.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change