feat(ui): remap Mosaic Button onto color, variant, and size - #9282
Merged
Conversation
🦋 Changeset detectedLatest commit: f213621 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: |
Contributor
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces Mosaic Button’s Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.claude/skills/mosaic/references/stylex.md (1)
326-335: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScope reduced-motion durations to motion properties only.
A single
transitionDurationvalue governs all transitionable properties intransition-property, so this example also shorts color/opacity transitions despite saying they should remain unaffected. Show separate duration rules for motion-bearing transitions, or note that this object only contains motion property transitions.🤖 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 @.claude/skills/mosaic/references/stylex.md around lines 326 - 335, The transitionDuration example conflicts with the guidance by applying the reduced-motion duration to every transitionable property. Update the example to scope reduced-motion timing to motion-bearing properties only, using separate duration rules where needed, or explicitly limit the object’s transition-property set to motion properties.
🤖 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/mosaic-button-variants.md:
- Around line 1-2: Add the appropriate `@clerk/ui` release entry to the
frontmatter in mosaic-button-variants.md, including the required version bump
and migration details for the Mosaic Button contract and Firefox build floor.
Preserve the existing changeset structure and header format.
In `@packages/ui/src/mosaic/components/button/button.styles.ts`:
- Around line 16-19: Reconcile the documented and implemented primary
pressed-state blend in the button style definitions: update primaryActive or its
accompanying comment so both agree on the intended percentage, while preserving
the existing primaryHover, negative, and neutral blend values.
---
Nitpick comments:
In @.claude/skills/mosaic/references/stylex.md:
- Around line 326-335: The transitionDuration example conflicts with the
guidance by applying the reduced-motion duration to every transitionable
property. Update the example to scope reduced-motion timing to motion-bearing
properties only, using separate duration rules where needed, or explicitly limit
the object’s transition-property set to motion properties.
🪄 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: 58c929e8-46a9-4122-99f0-277c6108216a
📒 Files selected for processing (23)
.changeset/mosaic-button-variants.md.claude/skills/mosaic/references/stylex.mdpackages/swingset/src/stories/button.mdxpackages/swingset/src/stories/button.stories.tsxpackages/swingset/src/stories/destructive.stories.tsxpackages/swingset/src/stories/dialog.component.stories.tsxpackages/ui/src/mosaic/__tests__/props.test.tspackages/ui/src/mosaic/block/destructive.tsxpackages/ui/src/mosaic/components/avatar/avatar.styles.tspackages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/button/button.test.tsxpackages/ui/src/mosaic/components/button/button.tsxpackages/ui/src/mosaic/components/icon/icon.markers.stylex.tspackages/ui/src/mosaic/components/icon/icon.tsxpackages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section.view.tsxpackages/ui/src/mosaic/organization/organization-profile-leave-section.view.tsxpackages/ui/src/mosaic/organization/organization-profile-members-panel.view.tsxpackages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsxpackages/ui/src/mosaic/styles/index.tspackages/ui/src/mosaic/tokens.stylex.tspackages/ui/tsdown.mosaic.config.mts
🔗 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)
Comment on lines
+1
to
+2
| --- | ||
| --- |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== changeset files =="
git ls-files .changeset | sort
echo
echo "== target changeset =="
if [ -f .changeset/mosaic-button-variants.md ]; then
cat -n .changeset/mosaic-button-variants.md
else
echo "missing"
fi
echo
echo "== package/ui references =="
git ls-files | rg 'packages/ui|ui/' | head -100
echo
rg -n '"name"\s*:\s*["`@-`]?[^"]*ui|packages/ui|`@clerk/ui`|`@mui/material`|firefox' package.json CHANGELOG.md packages README.md .changeset -S || trueRepository: clerk/javascript
Length of output: 5459
🏁 Script executed:
#!/bin/bash
set -euo_pipefail
echo "== root package/workspaces =="
cat -n package.json | sed -n '1,220p'
echo
echo "== packages/ui package metadata =="
cat -n packages/ui/package.json | sed -n '1,220p'
echo
echo "== related changesets =="
while IFS= read -r f; do
echo "--- $f"
cat -n "$f"
done < <(git ls-files .changeset '*.md' | grep -v config.json | grep -v changelog.js | grep -v README.md | sort)
echo
echo "== button contract references =="
rg -n "intent|color|variant|Browser\|Firefox|firefox|120|121" packages/ui -S --glob '!**/CHANGELOG.md' | head -200
echo
echo "== exports/test files touching Button/Mosaic =="
rg -n "Button|ButtonProps|color|variant|intent" packages/ui/src --glob '*.{ts,tsx}' -S | head -250Repository: clerk/javascript
Length of output: 807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== root package/workspaces =="
cat -n package.json | sed -n '1,220p'
echo
echo "== packages/ui package metadata =="
cat -n packages/ui/package.json | sed -n '1,220p'
echo
echo "== related changesets =="
while IFS= read -r f; do
echo "--- $f"
cat -n "$f"
done < <(git ls-files .changeset '*.md' | grep -v config.json | grep -v changelog.js | grep -v README.md | sort)
echo
echo "== button contract references =="
rg -n "intent|color|variant|Browser\|Firefox|firefox|120|121" packages/ui -S --glob '!**/CHANGELOG.md' | head -200
echo
echo "== exports/test files touching Button/Mosaic =="
rg -n "Button|ButtonProps|color|variant|intent" packages/ui/src --glob '*.{ts,tsx}' -S | head -250Repository: clerk/javascript
Length of output: 18637
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .changeset files and contents =="
for f in $(git ls-files .changeset '*.md' | sort); do
echo "--- $f"
cat -n "$f"
done
echo
echo "== root workspace configs =="
fd '^pnpm-workspace.yaml$|^turbo.json$|^README.md$' -t f -x sh -c 'echo "--- $1"; cat -n "$1" | sed -n "1,220p"' sh {}
echo
echo "== target package references =="
rg -n --no-heading '`@clerk/ui`|packages/ui|Firefox|firefox|12 [01]|121|120|intent|color|variant' .changeset README.md package.json turbo.json pnpm-workspace.yaml .github packages/ui -S | head -300
echo
echo "== button files in packages/ui/src =="
rg -n --no-heading 'export .*Button|interface .*ButtonProps|type .*ButtonProps|const .*Button|ButtonProps|intent|color|variant' packages/ui/src --glob '*.{ts,tsx}' -S | head -300Repository: clerk/javascript
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant .changeset files and contents =="
for f in $(git ls-files .changeset '*.md' | sort | grep -v -E '(config\.json|changelog\.js|README\.md)$'); do
echo "--- $f"
cat -n "$f"
done
echo
echo "== exact target references =="
for pat in '`@clerk/ui`' 'packages/ui' 'Firefox' 'firefox' '120' '121' 'intent' 'color' 'variant'; do
echo "--- $pat"
rg -n "$pat" .changeset README.md package.json turbo.json pnpm-workspace.yaml .github packages/ui -S --glob '!**/CHANGELOG.md' --glob '!**/README.md' --glob '!**/*.png' --glob '!**/*.svg' | head -100
done
echo
echo "== all Button-related files under packages/ui/src =="
git ls-files 'packages/ui/src/**' | rg 'button|Button|buttons' || true
echo
echo "== Button symbol/export definitions under packages/ui/src =="
rg -n --no-heading -i 'button|intent|color|variant|Firefox|120|121' packages/ui/src -S | head -300Repository: clerk/javascript
Length of output: 50375
Add the @clerk/ui release entry to this changeset.
.changeset/mosaic-button-variants.md is only the header, while @clerk/ui is a public package and this change touches its Mosaic Button contract/Firefox build floor. Add its bump/migration details so the release note and package version are not missed.
🤖 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/mosaic-button-variants.md around lines 1 - 2, Add the appropriate
`@clerk/ui` release entry to the frontmatter in mosaic-button-variants.md,
including the required version bump and migration details for the Mosaic Button
contract and Firefox build floor. Preserve the existing changeset structure and
header format.
Sources: Coding guidelines, Learnings
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/button/button.styles.ts (1)
31-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the new styling commentary.
The repeated prose substantially obscures the style recipe. Keep terse “why” notes only for non-obvious StyleX ordering/selector constraints; remove comments that restate the adjacent declarations.
packages/ui/src/mosaic/components/button/button.styles.ts#L31-L42: retain a short selector-ordering rationale.packages/ui/src/mosaic/components/button/button.styles.ts#L54-L60: remove declaration-by-declaration overflow/appearance narration.packages/ui/src/mosaic/components/button/button.styles.ts#L64-L65: remove theflexShrinkrestatement.packages/ui/src/mosaic/components/button/button.styles.ts#L75-L85: reduce transition and interaction-state prose to essential rationale.packages/ui/src/mosaic/components/button/button.styles.ts#L89-L96: retain only theminWidth: 0flexbox rationale.packages/ui/src/mosaic/components/button/button.styles.ts#L112-L123: shorten touch-target explanations.packages/ui/src/mosaic/components/button/button.styles.ts#L130-L135: remove generic variant-map narration.As per coding guidelines, “Keep code comments minimal: add comments only when critically needed to explain why a non-obvious change was made.”
🤖 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/button.styles.ts` around lines 31 - 42, Condense comments in the button.styles.ts style definitions: retain only a brief explanation of the non-obvious selector-ordering constraint at lines 31-42, remove declaration restatements at lines 54-60 and 64-65, reduce transition and interaction-state prose at lines 75-85, keep only the minWidth flexbox rationale at lines 89-96, shorten touch-target explanations at lines 112-123, and remove generic variant-map narration at lines 130-135.Source: Coding guidelines
🤖 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/ui/src/mosaic/components/button/button.tsx`:
- Around line 40-47: Update withTruncatableLabel to explicitly return
React.ReactNode and combine consecutive string/number children into a single
label span, flushing the buffer only when an element child is encountered so
adjacent text remains one flex item; preserve non-text children and add a test
covering mixed adjacent text such as “Delete {name}”.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/button/button.styles.ts`:
- Around line 31-42: Condense comments in the button.styles.ts style
definitions: retain only a brief explanation of the non-obvious
selector-ordering constraint at lines 31-42, remove declaration restatements at
lines 54-60 and 64-65, reduce transition and interaction-state prose at lines
75-85, keep only the minWidth flexbox rationale at lines 89-96, shorten
touch-target explanations at lines 112-123, and remove generic variant-map
narration at lines 130-135.
🪄 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: e016899f-aa4a-4dc2-b383-817f4fd29053
📒 Files selected for processing (5)
packages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/button/button.test.tsxpackages/ui/src/mosaic/components/button/button.tsxpackages/ui/src/mosaic/styles/index.tspackages/ui/src/mosaic/tokens.stylex.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)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/mosaic/styles/index.ts
maxyinger
force-pushed
the
max/mosaic-button-variant-remap
branch
from
July 30, 2026 15:30
54db4c8 to
f24fe25
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/ui/src/mosaic/components/button/button.tsx (1)
37-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdjacent text children still truncate/gap independently.
withTruncatableLabelwraps each string/number child in its own<span>rather than buffering consecutive text into a single label.<Button>Delete {name}</Button>still gets an internalgapbetween "Delete " and{name}, and each fragment truncates independently instead of as one label. This was flagged previously and remains unresolved. The helper also still lacks an explicit return type.♻️ Proposed fix (buffer consecutive text/number children)
-function withTruncatableLabel(children: React.ReactNode) { - return React.Children.map(children, child => - typeof child === 'string' || typeof child === 'number' ? ( - <span {...stylex.props(styles.label)}>{child}</span> - ) : ( - child - ), - ); +function withTruncatableLabel(children: React.ReactNode): React.ReactNode { + const nodes: React.ReactNode[] = []; + let label = ''; + + const flushLabel = (): void => { + if (label) { + nodes.push( + <span key={`label-${nodes.length}`} {...stylex.props(styles.label)}> + {label} + </span>, + ); + label = ''; + } + }; + + React.Children.forEach(children, child => { + if (typeof child === 'string' || typeof child === 'number') { + label += child; + return; + } + flushLabel(); + nodes.push(child); + }); + + flushLabel(); + return nodes; +}As per coding guidelines, "Always define explicit return types for functions, especially public APIs."
🤖 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/button.tsx` around lines 37 - 48, Update withTruncatableLabel to combine consecutive string and number children into one styled span, preserving element children as direct flex items so adjacent text truncates and gaps as a single label. Add an explicit React return type to the helper.Source: Coding guidelines
packages/ui/src/mosaic/components/button/button.styles.ts (1)
24-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
primaryActivestill doesn't match its documented percentage.Comment states primary blends "12%/24%" for hover/active;
primaryHoveris 12% (matches), butprimaryActiveis20%, not24%. Negative/neutral values do match their documented percentages. This was flagged in a prior review round and is still unresolved — please confirm which value (20% or 24%) is correct and reconcile code/comment.✏️ Example fix (if 24% is correct)
-const primaryActive = `color-mix(in oklab, ${colorVars['--cl-color-primary']}, ${colorVars['--cl-color-primary-foreground']} 20%)`; +const primaryActive = `color-mix(in oklab, ${colorVars['--cl-color-primary']}, ${colorVars['--cl-color-primary-foreground']} 24%)`;🤖 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/button.styles.ts` around lines 24 - 29, Reconcile the documented primary hover/active blend percentages with the implementation in primaryHover and primaryActive: confirm whether the active value should be 20% or 24%, then update the constant or explanatory comment so both agree. Leave the negative and neutral blend values unchanged.
🧹 Nitpick comments (1)
.claude/skills/mosaic/references/stylex.md (1)
326-335: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScope reduced-motion gating to motion properties.
This
transitionDurationoverride is property-wide; if the component also transitions color or opacity, it disables those transitions too, contradicting the preceding guidance. Show a motion-specific transition (for example,transform) or separate motion and color transition declarations before applying the reduced-motion override.🤖 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 @.claude/skills/mosaic/references/stylex.md around lines 326 - 335, Update the reduced-motion example in the motion-property guidance so the override applies only to a motion-specific transition such as transform, rather than the property-wide transitionDuration declaration. If retaining color or opacity transitions, separate their declarations and leave them unaffected by the reduced-motion override.
🤖 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.
Duplicate comments:
In `@packages/ui/src/mosaic/components/button/button.styles.ts`:
- Around line 24-29: Reconcile the documented primary hover/active blend
percentages with the implementation in primaryHover and primaryActive: confirm
whether the active value should be 20% or 24%, then update the constant or
explanatory comment so both agree. Leave the negative and neutral blend values
unchanged.
In `@packages/ui/src/mosaic/components/button/button.tsx`:
- Around line 37-48: Update withTruncatableLabel to combine consecutive string
and number children into one styled span, preserving element children as direct
flex items so adjacent text truncates and gaps as a single label. Add an
explicit React return type to the helper.
---
Nitpick comments:
In @.claude/skills/mosaic/references/stylex.md:
- Around line 326-335: Update the reduced-motion example in the motion-property
guidance so the override applies only to a motion-specific transition such as
transform, rather than the property-wide transitionDuration declaration. If
retaining color or opacity transitions, separate their declarations and leave
them unaffected by the reduced-motion override.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: dd78d8ee-1422-4766-9457-2ffb8524715b
📒 Files selected for processing (23)
.changeset/mosaic-button-variants.md.claude/skills/mosaic/references/stylex.mdpackages/swingset/src/stories/button.mdxpackages/swingset/src/stories/button.stories.tsxpackages/swingset/src/stories/destructive.stories.tsxpackages/swingset/src/stories/dialog.component.stories.tsxpackages/ui/src/mosaic/__tests__/props.test.tspackages/ui/src/mosaic/block/destructive.tsxpackages/ui/src/mosaic/components/avatar/avatar.styles.tspackages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/button/button.test.tsxpackages/ui/src/mosaic/components/button/button.tsxpackages/ui/src/mosaic/components/icon/icon.markers.stylex.tspackages/ui/src/mosaic/components/icon/icon.tsxpackages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsxpackages/ui/src/mosaic/organization/organization-profile-domains-section.view.tsxpackages/ui/src/mosaic/organization/organization-profile-leave-section.view.tsxpackages/ui/src/mosaic/organization/organization-profile-members-panel.view.tsxpackages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsxpackages/ui/src/mosaic/styles/index.tspackages/ui/src/mosaic/tokens.stylex.tspackages/ui/stylex-lightningcss.config.mjs
🔗 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 (6)
- packages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsx
- packages/ui/src/mosaic/organization/organization-profile-members-panel.view.tsx
- .changeset/mosaic-button-variants.md
- packages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsx
- packages/ui/src/mosaic/organization/organization-profile-domains-section.view.tsx
- packages/swingset/src/stories/button.mdx
Contributor
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/ui/src/mosaic/components/button/button.styles.ts`:
- Around line 30-32: Update neutralStep0, neutralStep1, and neutralStep2 to use
the lightDark utility with the existing black and white OKLCH values inside each
color-mix declaration, replacing the raw light-dark() expressions while
preserving the current mix percentages.
🪄 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: 8fe7b5ed-b5ab-423a-be23-b52afdd8ae3a
📒 Files selected for processing (1)
packages/ui/src/mosaic/components/button/button.styles.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)
Comment on lines
+30
to
+32
| const neutralStep0 = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 8%, transparent)`; | ||
| const neutralStep1 = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 12%, transparent)`; | ||
| const neutralStep2 = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 16%, transparent)`; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file excerpt =="
sed -n '1,90u' packages/ui/src/mosaic/components/button/button.styles.ts
echo
echo "== lightDark utility candidates =="
fd -a 'lightDark|light-dark' packages/ui/src | sed 's#^\./##'
echo
echo "== imports/usages of lightDark in packages/ui/src =="
rg -n "lightDark|createLightDark|toCSSVariable|light-dark" packages/ui/src --glob '*.{ts,tsx}' | head -200Repository: clerk/javascript
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file excerpt =="
sed -n '1,90p' packages/ui/src/mosaic/components/button/button.styles.ts
echo
echo "== lightDark utility candidates =="
fd -a 'lightDark|light-dark' packages/ui/src | sed 's#^\./##'
echo
echo "== imports/usages of lightDark in packages/ui/src =="
rg -n "lightDark|createLightDark|toCSSVariable|light-dark" packages/ui/src --glob '*.{ts,tsx}' | head -200Repository: clerk/javascript
Length of output: 13864
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lightDark implementation =="
cat -n packages/ui/src/utils/lightDark.ts
echo
echo "== cssSupports implementation =="
cat -n packages/ui/src/utils/cssSupports.ts
echo
echo "== local imports/usages around neutralStep constants =="
sed -n '80,160p' packages/ui/src/mosaic/components/button/button.styles.tsRepository: clerk/javascript
Length of output: 7114
Route neutral scrims through the lightDark utility.
packages/ui/src/mosaic/components/button/button.styles.ts constructs raw light-dark() values, which bypass the codebase fallback for unsupported environments and violate the packages/ui/src guideline to use lightDark. Since these values are inlined StyleX CSS, unsupported environments will receive an unparsed color-mix(..., light-dark(...)), leaving neutral fills and states without a valid fallback color.
As lightDark accepts light/dark strings, these can be rewritten with lightDark('oklch(0 0 0)', 'oklch(1 0 0)') inside the color-mix(...) declarations.
🤖 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/button.styles.ts` around lines 30 -
32, Update neutralStep0, neutralStep1, and neutralStep2 to use the lightDark
utility with the existing black and white OKLCH values inside each color-mix
declaration, replacing the raw light-dark() expressions while preserving the
current mix percentages.
Source: Coding guidelines
Contributor
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/ui/src/mosaic/components/button/button.styles.ts`:
- Around line 29-31: Update neutralStep0, neutralStep1, and neutralStep2 to use
the repository’s lightDark helper instead of raw light-dark() expressions,
preserving the existing 6%, 12%, and 18% color-mix ramp values.
🪄 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: a4ef16b9-1ca7-49dc-9c7a-52f597e1be02
📒 Files selected for processing (2)
packages/ui/src/mosaic/components/button/button.styles.tspackages/ui/src/mosaic/components/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/ui/src/mosaic/components/button/button.tsx
Comment on lines
+29
to
+31
| const neutralStep0 = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 6%, transparent)`; | ||
| const neutralStep1 = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 12%, transparent)`; | ||
| const neutralStep2 = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 18%, transparent)`; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use lightDark for neutral scrim values.
These raw light-dark() calls bypass the repository fallback; where light-dark() is unsupported, the surrounding color-mix() declaration becomes invalid and neutral button states lose their styling. Replace the nested expressions with lightDark(...) while preserving the 6%/12%/18% ramp. This repeats the unresolved finding from the previous review.
🤖 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/button.styles.ts` around lines 29 -
31, Update neutralStep0, neutralStep1, and neutralStep2 to use the repository’s
lightDark helper instead of raw light-dark() expressions, preserving the
existing 6%, 12%, and 18% color-mix ramp values.
Source: Coding guidelines
alexcarpenter
approved these changes
Jul 30, 2026
Add `--cl-radius-control` (6px), a step the 4/8/12 radius progression doesn't land on, for the controls that sit between an inner mark and a container. Avatar's square shape stops hardcoding the value and points at the token. Add the motion groups the interaction styles need: `--cl-duration-*`, scaled by how directly a change answers the pointer, and a single `--cl-ease-default` for properties that actually move. Both are re-exported from `styles/index.ts` with their `*VarName` unions, matching the other token groups.
Icon now applies a `defineMarker()` alongside its own styles, so a container selecting on `data-icon` matches only a Mosaic `Icon` and not a stray `data-icon` elsewhere in its subtree. The marker lives in its own `.stylex.ts` module because `@stylexjs/enforce-extension` requires the define-primitives to.
Replace the `intent` axis with `color`, matching what Badge, Heading, and Text already call it, and widen every axis: `color` gains `neutral` and renames `destructive` to `negative` to track the token names; `variant` gains `link`; `size` gains `lg`. `shape` and `fullWidth` stay as orthogonal modifiers. The emitted attribute follows the prop, so `data-intent` becomes `data-color`. The variant × color cross product is flattened into one `<variant>-<color>` map the component indexes directly, rather than one axis plumbing custom properties to the other -- vars would emit onto every instance in devtools and read as public API. Each cell is self-contained so it can be tuned against the design matrix; the values shared across cells are same-file consts that StyleX inlines. Interaction states move onto the motion tokens: an instant press so it reads as contact, a `fast` settle on release, and `linear` throughout since nothing here moves. Hover excludes `:active` explicitly rather than relying on rule order, which StyleX doesn't guarantee across at-rule boundaries. Sizes now tighten their inline padding on the side an icon sits, keyed off the `data-icon` placement Icon reflects, so an icon lands in a square cell instead of inheriting the text inset. That selector is `:has()`, which moves the Firefox build target to 121. `Button` also picks up `MosaicComponentProps`, whose `color` shadows the native `button` color attribute -- the one trigger call site forwarding native props now drops it. The `themeProps` contract tests move onto the same names, and the swingset dialog trigger drops native `color` the way the profile-section trigger does.
Point the story's variant metadata at the new axes so the prop table matches the component, and add stories for the variant set, the full color x variant matrix, and icon placement. The matrix is rendered live rather than described, since hover, press, and focus are most of what separates the cells. The icon prose covers why the padding tightens on the icon side and where the value comes from, which is otherwise only legible from the styles.
The StyleX reference had no guidance for a component with two multiplying visual axes, and the natural move -- one axis writing custom properties for the other to read -- emits vars onto every instance that read as public API. Write down the flattened variant map instead, when a var is still the right tool, and the role-atom escape hatch for a cross product too large to flatten.
…avior Four corrections to Button's base behavior, informed by reading the StyleX design systems in `references/stylex-ui`. `disabled` no longer sets `pointer-events: none`. It made the `cursor: not-allowed` alongside it dead — an element that isn't hit-tested never supplies a cursor — and it kept a disabled button from receiving the pointer at all, which a wrapping tooltip needs. Suppressing the interactive states is now the selectors' job: `:hover` and `:active` still match a disabled button, so every one of them is gated on `:enabled` and the button keeps its resting fill while `disabled` only dims it. The size axis fixes the button's height, so a label long enough to wrap used to overflow it. The label now stays on one line and truncates: text children get a span of their own to run the ellipsis against, since `text-overflow` doesn't inherit and a bare text child is laid out in an anonymous flex item no selector can reach. Element children pass through untouched and stay direct flex items, so `gap` and the icon padding still apply. Under a coarse pointer the control floors at a new `--cl-target-coarse` (44px) — off the `--cl-spacing` scale deliberately, since a fingertip doesn't rescale with a consumer's density setting. Icon buttons take it on both axes so the target doesn't end up tall and narrow; `link` opts out, being text rather than a control. Base picks up the control styles the reference implementations all carry: `appearance: none`, `flex-shrink: 0`, and `user-select: none`.
The neutral ramp -- `filled-neutral` at rest through pressed, and the hover and pressed fills every `outline-*` and `ghost-*` variant shares -- was three steps mixed off `--cl-color-neutral`. Those are opaque, so the fill painted over whatever it sat on rather than compositing with it, and a button on a non-default surface read as a neutral-colored patch instead of a lift. The three steps are now 8%/12%/16% of a fully-tilted black-on-light, white-on-dark scrim over `transparent`. Deliberately not a percentage of `--cl-color-neutral`: that token is a 900, so the same percentage of it lands lighter than the percentage of black -- 8% neutral is 0.01 L short -- and the shortfall moves with the backdrop, which would leave the step numbers describing something other than what they render. The two opaque filled fills keep blending toward their own on-fill, since they have a fill of their own to blend from; only neutral rides the opacity ramp. The outline border stays `--cl-color-border` in every state, but it no longer sits a fixed lightness gap from the fill it covers -- how close the two read now depends on the backdrop. `item` and `avatar` still mix from `--cl-color-neutral`. Unifying them with this ramp on a shared overlay token is a palette-wide change, not a Button one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The neutral ramp opens lower and tops out higher: 6%/12%/18% rather than 8%/12%/16%. The old rest step sat close enough to step 1 that a `filled-neutral` button barely moved on hover, and the pressed step didn't read as pressed against it. Widening the ends keeps the same three roles while making each transition visible. The opaque filled fills line up with that ramp instead of running their own spacing: primary and negative both blend 12%/18% toward their on-fill, where primary was 12%/20% and negative 8%/16%. Negative in particular was moving half as far as primary on hover for no reason other than the values predating the shared ramp. `styles.label` no longer redeclares `overflow` and `text-overflow`; the span picks up `truncationStyles.singleLine`, which is the same declaration plus the `display: block` the ellipsis needs anyway. What stays local is `minWidth: 0`, which releases the flex-item min-content floor -- that's specific to the label sitting in the button's row, and it's why `item` doesn't need the same treatment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`withTruncatableLabel` boxed each text child on its own, so `Delete {name}` --
two children as far as React is concerned -- came out as two spans and the
button's `gap` opened up mid-sentence. Adjacent text now accumulates into a
single run and flushes to one span when an element child interrupts it, so the
gap only ever falls between the label and an icon.
The walk moved to `React.Children.toArray` so the element children it passes
through keep the keys React assigns them, and the returned array doesn't warn.
Children that render nothing open no box at all.
Tests cover the runs, a numeric child, the empty-render case, shape reflection,
absent boolean modifiers, and that a disabled button doesn't fire `onClick` --
the last one worth asserting now that `disabled` no longer sets
`pointer-events: none` and the element stays hit-testable.
Comments throughout `button.styles.ts` are tightened to a single pass; a TODO
records that the neutral scrim scale should become shared tokens once more than
Button wants those shades.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…get behavior Three behaviors landed on Button without prose to go with them. Truncation gets an example and a section. The point the demo has to make is that a button sizes to its own content and doesn't shrink, so most of the time there is nothing to truncate -- the ellipsis only appears once the width is constrained from outside, which the story does with `fullWidth` in a narrow container. The prose covers why only text children truncate and why an icon holds its size while the label gives way. Disabled had a story but no explanation. What matters isn't visible in it: the button keeps its resting fill rather than dropping to a gray of its own, and it stays hit-testable, which is what makes `cursor: not-allowed` render and lets a wrapping tooltip say why the action is unavailable. The coarse-pointer floor is documented under Sizes, since it's a sizing rule -- 44px on both axes for icon buttons, `link` opting out, and a note that none of it shows on a mouse, which is otherwise a confusing thing to read next to a demo that doesn't do it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four additions to the Mosaic StyleX reference, each a rule Button had to work out with nothing written down to work from. Overlay fills -- a hover or pressed wash on a transparent `outline`/`ghost` control -- are a scrim: an opacity of black-on-light, white-on-dark over `transparent`, not a percentage of a gray token. A gray like `--cl-color-neutral` is a 900 rather than black, so the same percentage of it lands lighter, and by an amount that shifts with whatever the control sits on, which stops the step numbers describing what they render. Scoped to overlay fills; a fill with a color of its own still blends toward its on-fill. Radii are named by what the surface is -- `inner`, `control`, `container` -- rather than by size, so the steps nest and a role name survives a value change. An interactive control floors its hit area at `--cl-target-coarse` under a coarse pointer, as its own atom rather than a per-size override, with the both-axes rule for square controls and the reason the token sits off the `--cl-spacing` scale. And `pointer-events: none` is called out as the wrong way to suppress a disabled control, since it takes the cursor and any wrapping tooltip with it -- gate on `:enabled` instead. The hoist example also stops using the neutral `color-mix` the scrim rule now argues against; it shows a colored fill blending toward its on-fill. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maxyinger
force-pushed
the
max/mosaic-button-variant-remap
branch
from
July 30, 2026 18:28
c4403b6 to
305c380
Compare
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Remaps Mosaic
Buttononto three independent axes —color×variant×size— and restyles it against the design matrix.intentbecomescolor, matching whatBadge,Heading, andTextalready call the axis, and every axis widens:colorgainsneutraland renamesdestructivetonegativeto track the token names,variantgainslink,sizegainslg.shapeandfullWidthstay as orthogonal modifiers. The emitted attribute follows the prop, sodata-intentis nowdata-color.Supporting changes:
--cl-radius-control(6px), for controls sitting between an inner mark and a container — Avatar's square shape stops hardcoding the value and points at it. Plus the motion groups the interaction states need:--cl-duration-*, scaled by how directly a change answers the pointer, and a single--cl-ease-default.fastsettle on release, andlinearthroughout since nothing here moves. Hover excludes:activeexplicitly rather than relying on rule order, which StyleX doesn't guarantee across at-rule boundaries.data-iconplacementIconreflects, so an icon lands in a square cell instead of inheriting the text inset.Iconnow also applies adefineMarker()so a container'sdata-iconselectors can't match a straydata-iconelsewhere in the subtree. The selector is:has(), which moves the Firefox build target to 121.Follow-ups
isPendingprop that reserves the label's box and overlays a spinner, with the reveal delayed in CSS so fast actions never flash. Blocked on Mosaic having noSpinner. Async orchestration stays in the machine/controller layer.render. Either anhrefprop or a separateButtonLink. Callers must destructure the nativecolorattribute out first; only the swingset build catches that collision.<Button shape='circle'><Icon /></Button>type-checks today and ships unnamed. Encodearia-label/aria-labelledbyin the prop union, discriminated onshape.filledandghostlose their boundary in Windows high contrast, since base setsborder-color: transparent. One shared atom addingCanvasText/Highlightunder@media (forced-colors: active).border-radiusandborder-colormean per cell.[aria-expanded="true"]/[aria-pressed="true"]rather than a new prop, so visual and accessible state stay the same fact. Declare it before:enabled:active.<svg>children get no sizing — our icon rules only match a real MosaicIcon. Consider a fallback.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change