feat(ui): Add a Mosaic scroll area as StyleX atoms - #9311
Conversation
A vertically scrolling region that fades its content at whichever edge still has something to reveal. Composed as `ScrollArea.Root` + `ScrollArea.Viewport`. The indicators are pure CSS with no runtime cost: two scroll-driven animations write a progress var per edge, and a single four-stop mask gradient computes its stops from them. Driving a number rather than the mask's own geometry means one gradient covers both edges — no `mask-composite` — and leaves the progress vars readable so a consumer can swap the treatment from a stylesheet alone. The vars are `stylex.types.number`, so StyleX emits an `@property` registration for each. That does two jobs: an unregistered custom property animates discretely and would snap at 50% instead of tracking the scroll, and `initial-value: 0` is what hides both indicators when the viewport isn't scrollable — an inactive timeline leaves the vars at their initial value, so the resting state is already the hidden one. Only `animation-name` sits behind `@supports (animation-timeline: scroll())`. A browser that ignores `animation-timeline` would otherwise run the animations on the document timeline at the default `0s` duration, land on the end frame immediately, and paint both fades permanently; with no name the rest is inert and the mask resolves to fully opaque. Because the fade is a mask rather than a sticky overlay element, it is paint-only and cannot shift the content the way sticky pseudo-elements do. `gutter` defaults to `auto`, matching CSS. `stable` is the opt-in for content that can change height in place — a filterable list, a paginated table — where crossing the overflow threshold would shift the rows sideways. Scrollbar size is a theme token (`--cl-scrollbar-width`, default `thin`) rather than a prop, since Mosaic has no reason to size scrollbars differently between components. It is keyword-only by spec: `scrollbar-width` accepts `auto | thin | none` and not a length. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the story module and MDX page for `ScrollArea`, following the archetype A compound layout: Example, Usage, Parts, Styling. Five examples. `Default` and `NotScrollable` show that absent indicators are the resting state rather than something switched off. `Gutter` toggles the content across the overflow threshold, because `stable` and `auto` are identical while the content permanently overflows — a side-by-side pair alone demonstrates nothing, and the docs say so alongside the platform caveat, since neither value does anything where scrollbars overlay. `CustomIndicators` is the CSS-only override path: `mask-image: none` plus the progress vars driving a pair of gradient overlays. Its scrim mixes from `--cl-color-card-foreground` rather than hardcoding black — a black scrim darkens a dark surface, which is indistinguishable from the mask it replaced, so the example would have taught a bug in dark mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--cl-scroll-area-fade-size`, `-fade-range` and `-scrollbar-inset` become `--cl-scroll-fade-size`, `-range` and `-inset` in `tokens.stylex.ts`. Every other token in Mosaic is global and category-named — `--cl-color-*`, `--cl-radius-*`, `--cl-duration-*`, `--cl-scrollbar-width`. These three were about to be the first `--cl-<component>-<thing>` family, and if each component followed, theming Clerk would mean enumerating N components x M knobs rather than learning one vocabulary. How soft the edge of a scrolling region is belongs to the design language, not to one component; a component that needs different values still sets the token on itself. The progress vars stay component-named on purpose. They are per-element runtime output that the animations overwrite on every scrolling element, so a `:root` value would be meaningless — promoting them would imply they are settable when setting them does nothing. Set versus read is the line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 162c5d4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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.
|
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Chrome and Firefox make an overflowing scroll container keyboard-focusable on their own; Safari does not, so a keyboard-only user there cannot scroll the region at all (WCAG 2.1.1). Leaving that to the caller meant every caller had to know about a browser gap to get a baseline accessibility guarantee, which is the component's responsibility rather than theirs. The viewport now takes a tab stop exactly when the browsers themselves would: when it overflows AND its content contains nothing focusable. The second half carries as much weight as the first — a list whose rows are buttons or links is already reachable, since tabbing into the content scrolls it, so a stop on the container would be redundant. Reproducing the browsers' rule rather than sniffing for Safari means this is a no-op wherever the browser already handles it. Both halves are observed rather than sampled once. A ResizeObserver watches the viewport and each element child, because content growing past the threshold leaves the viewport's own box unchanged and a MutationObserver wouldn't catch a purely visual change like an image loading; a MutationObserver covers rows gaining or losing interactivity. An explicit `tabIndex` always wins, and `-1` opts out. This is the component's only JavaScript. The JSDoc claim that nothing ran at runtime is corrected accordingly — the indicators still cost nothing, with no scroll listener and no measurement behind them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Mosaic ScrollArea StyleX atoms for contained scrolling, scrollbar gutters, edge fades, focus outlines, and scroll-progress variables. Adds shared scrollbar and scroll-fade tokens with public exports. Adds tests for style composition and token mappings. Adds an Item scrolling story, documentation, registry wiring, and a minor Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 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: 4
🧹 Nitpick comments (1)
packages/swingset/src/stories/scroll-area.component.mdx (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one short introduction.
Lines 5-13 contain two introductory paragraphs. Keep one short present-tense introduction here. Move the CSS implementation details to
StylingorBrowser support.As per coding guidelines, “Keep each documentation intro to one short, present-tense paragraph.”
🤖 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/swingset/src/stories/scroll-area.component.mdx` around lines 5 - 13, Condense the introductory text in the ScrollArea documentation to one short, present-tense paragraph. Move the detailed CSS implementation, masking, runtime, and content-shift information into the existing Styling or Browser support section, preserving the documented behavior.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/swingset/src/lib/registry.ts`:
- Around line 97-104: Import the `__source` export from the scroll-area stories
module and assign it to the corresponding `scrollAreaModule` object alongside
its existing story metadata. Preserve the current story component imports and
ensure the property is available to `StoryEmbed` so ScrollArea examples render
`CodeFooter`.
In `@packages/swingset/src/stories/scroll-area.component.stories.tsx`:
- Around line 9-10: Remove the duplicated explanatory comments from the story
examples at the referenced sections, retaining only a concise comment for the
non-obvious __source export requirement. Do not alter the story implementations
or behavior.
- Around line 43-49: Import StoryComponent from `@/lib/types` and add explicit
StoryComponent return annotations to the exported story functions Default,
NotScrollable, Gutter, Tuning, and CustomIndicators, preserving their existing
implementations.
In `@packages/ui/src/mosaic/components/scroll-area/scroll-area.styles.ts`:
- Around line 40-48: Update the mask positioning associated with maskImage so
the scrollbar strip uses the logical inline-end position rather than a physical
right-edge position. Replace the physical positioning keywords in the relevant
maskPosition declaration with logical keywords, preserving the existing
first-layer positioning and inset behavior for both LTR and RTL layouts.
---
Nitpick comments:
In `@packages/swingset/src/stories/scroll-area.component.mdx`:
- Around line 5-13: Condense the introductory text in the ScrollArea
documentation to one short, present-tense paragraph. Move the detailed CSS
implementation, masking, runtime, and content-shift information into the
existing Styling or Browser support section, preserving the documented behavior.
🪄 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: 85d9c3f4-0bb7-476b-a2a1-c26ad5e995bc
📒 Files selected for processing (12)
.changeset/mosaic-scroll-area.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/scroll-area.component.mdxpackages/swingset/src/stories/scroll-area.component.stories.tsxpackages/ui/src/mosaic/components/scroll-area/index.tspackages/ui/src/mosaic/components/scroll-area/scroll-area.styles.tspackages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsxpackages/ui/src/mosaic/components/scroll-area/scroll-area.tsxpackages/ui/src/mosaic/components/scroll-area/scroll-area.vars.stylex.tspackages/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)
| import { | ||
| CustomIndicators as ScrollAreaCustomIndicators, | ||
| Default as ScrollAreaDefault, | ||
| Gutter as ScrollAreaGutter, | ||
| meta as scrollAreaMeta, | ||
| NotScrollable as ScrollAreaNotScrollable, | ||
| Tuning as ScrollAreaTuning, | ||
| } from '../stories/scroll-area.component.stories'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'StoryEmbed|__source|scrollAreaModule|StoryModule' packages/swingset/srcRepository: clerk/javascript
Length of output: 36489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- StoryEmbed.tsx relevant lines ---'
sed -n '48,78p' packages/swingset/src/components/StoryEmbed.tsx | cat -n
echo '--- StoryPreview usage of StoryEmbed ---'
rg -n -C 8 '<StoryEmbed|StoryEmbed\(' packages/swingset/src
echo '--- registry lookups/imports of getModule or use storyModule ---'
rg -n -C 8 'getModule\(|getModule$|storyModule|StoryPreview|StoryEmbed' packages/swingset/src/stories packages/swingset/src/lib packages/swingset/src/components packages/swingset/src -g '!package.json'Repository: clerk/javascript
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate Story component ---'
rg -n 'function Story|const Story|export function Story|export const Story|Story = ' packages/swingset/src -g '*.{ts,tsx}'
echo '--- Story-related file snippets ---'
# Use bounded searches after filenames are known
sed -n '1,90p' packages/swingset/src/lib/exampleSnippet.ts 2>/dev/null | cat -n || true
sed -n '47,78p' packages/swingset/src/lib/extractStorySource.ts 2>/dev/null | cat -n || trueRepository: clerk/javascript
Length of output: 3570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- MDX config files ---'
fd -a 'mdx|vite|next|package.json' packages/swingset | sed 's#^\./##' | head -200
echo '--- MDX integration strings ---'
rg -n -C 4 'jsx: true|jsx: false|jsxImportSource|<Story|registerMDX|mdx|MDX|getProvider|providers' packages/swingset -g '*.{ts,tsx,json,cjs,mjs,js}'
echo '--- explicit Story imports/exports in packages/swingset src ---'
rg -n 'import .*Story|from .*Story|<Story|export .*Story' packages/swingset/src -g '*.{ts,tsx}' --max-count 200
echo '--- getModule callers ---'
rg -n -C 4 'getModule\(' packages/swingset/src -g '*.{ts,tsx}'Repository: clerk/javascript
Length of output: 50372
Include __source in scrollAreaModule.
MDX <Story> renders through StoryEmbed, and StoryEmbed only shows CodeFooter when storyModule.__source is present. Import __source from ../stories/scroll-area.component.stories and assign it at lines 180-187 so ScrollArea examples render their code footer.
Also applies to: 180-187
🤖 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/swingset/src/lib/registry.ts` around lines 97 - 104, Import the
`__source` export from the scroll-area stories module and assign it to the
corresponding `scrollAreaModule` object alongside its existing story metadata.
Preserve the current story component imports and ensure the property is
available to `StoryEmbed` so ScrollArea examples render `CodeFooter`.
| // Exposes this file's own source (via the `?raw` webpack rule) so each `<Story>` example | ||
| // renders a code footer with its function's source. See `StoryModule.__source`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove duplicated explanatory comments.
These comments restate behavior that the MDX page already documents. They also appear in the raw story source footer. Keep only a terse comment where it explains a non-obvious requirement, such as the __source export.
As per coding guidelines, “Keep code comments minimal” and “never restate code behavior.”
Also applies to: 51-69, 101-113, 124-136
🤖 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/swingset/src/stories/scroll-area.component.stories.tsx` around lines
9 - 10, Remove the duplicated explanatory comments from the story examples at
the referenced sections, retaining only a concise comment for the non-obvious
__source export requirement. Do not alter the story implementations or behavior.
Source: Coding guidelines
| export function Default() { | ||
| return ( | ||
| <ScrollArea.Root style={{ height: 220, width: 320 }}> | ||
| <ScrollArea.Viewport>{rows()}</ScrollArea.Viewport> | ||
| </ScrollArea.Root> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'type StoryComponent|interface StoryComponent|export function (Default|NotScrollable|Gutter|Tuning|CustomIndicators)' \
packages/swingset/srcRepository: clerk/javascript
Length of output: 37235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== types excerpt =="
sed -n '55,78p' packages/swingset/src/lib/types.ts
echo
echo "== scroll-area stories imports and export declarations =="
sed -n '1,35p' packages/swingset/src/stories/scroll-area.component.stories.tsx
awk '{print NR": "$0}' packages/swingset/src/stories/scroll-area.component.stories.tsx | sed -n '38,178p'
echo
echo "== component story exported functions with/without return annotations =="
python3 - <<'PY'
import pathlib, re
for p in sorted(pathlib.Path('packages/swingset/src/stories').glob('*.component.stories.tsx')):
text = p.read_text()
for m in re.finditer(r'^(?:export\s+)?function\s+(\w+)(?::\s*[\w<>.[\]?:!,|&` "]{2,})?\s*\(', text, re.M):
print(f"{p}:{m.start()): {m.group(1)}{' with annotation' if m.group(1) not in text[m.start():text.index(')')].strip() else ''}")
PY
echo
echo "== eslint/tsconfig story-specific rules =="
find packages/swingset -maxdepth 3 \( -name '.eslintrc*' -o -name 'eslint.config.*' -o -name 'tsconfig.*' -o -name 'biome.json' \) -print | sort
for f in $(find packages/swingset -maxdepth 4 \( -name '.eslintrc*' -o -name 'eslint.config.*' -o -name 'tsconfig.*' -o -name 'biome.json' \) -print | sort); do
echo "--- $f ---"
sed -n '1,220p' "$f" | rg -n "explicit|return|stories|typescript-eslint|react|eslint-plugin" -C 1 || true
doneRepository: clerk/javascript
Length of output: 8768
Add explicit return types to the exported stories.
Import StoryComponent from @/lib/types and annotate Default, NotScrollable, Gutter, Tuning, and CustomIndicators as story components. These exports are part of the story module surface, and the TypeScript guidelines require explicit function return annotations.
🤖 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/swingset/src/stories/scroll-area.component.stories.tsx` around lines
43 - 49, Import StoryComponent from `@/lib/types` and add explicit StoryComponent
return annotations to the exported story functions Default, NotScrollable,
Gutter, Tuning, and CustomIndicators, preserving their existing implementations.
Sources: Coding guidelines, Learnings
| // A single four-stop gradient covers both edges, because the animated quantity is a number | ||
| // the stops are computed from rather than the mask's own geometry. At progress 0 the stop | ||
| // collapses onto the edge it starts from, leaving a hard boundary that reads as fully | ||
| // opaque — so "no scroll yet" and "not scrollable at all" render identically, for free. | ||
| // | ||
| // The second layer is the scrollbar strip, held opaque so the fade never touches it. At the | ||
| // default `0px` inset it is zero-wide and contributes nothing. Layers composite with `add` | ||
| // by default, so no `mask-composite` declaration is needed. | ||
| const maskImage = `linear-gradient(to bottom, transparent 0, #000 calc(${progressStart} * ${fadeSize}), #000 calc(100% - ${progressEnd} * ${fadeSize}), transparent 100%), linear-gradient(#000, #000)`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate files"
fd -a 'scroll-area\.styles\.ts|tokens\.stylex\.ts' . | sed 's#^\./##'
echo "## scroll-area.styles outline"
ast-grep outline packages/ui/src/mosaic/components/scroll-area/scroll-area.styles.ts || true
echo "## relevant scroll-area.styles.ts"
sed -n '1,130p' packages/ui/src/mosaic/components/scroll-area/scroll-area.styles.ts
echo "## search scroll fade inset and scrollbar-related usages"
rg -n "`@cl-scroll-fade-inset`|--cl-scroll-fade-inset|scrollbar|fade" -S packages/ui/src packages -g '!**/node_modules' --glob '*.ts' --glob '*.tsx' | head -200Repository: clerk/javascript
Length of output: 31972
🌐 Web query:
CSS Masking mask-position logical keywords inline-start inline-end browser support
💡 Result:
As of July 31, 2026, the CSS property mask-position supports flow-relative (logical) keywords such as inline-start, inline-end, block-start, and block-end as part of the data type [1][2]. These keywords allow developers to position mask images relative to the writing mode and text direction of the element, rather than relying on fixed physical directions like top, left, right, or bottom [2]. Regarding browser support: - While the data type has been updated in specifications to include these logical keywords [2], full browser support for using them specifically within the mask-position property is generally aligned with the support for the mask-position property itself [1][3]. - As of the current date, mask-position has achieved broad support across all major modern browsers, including Chrome, Edge, Firefox, and Safari [1][3]. - Because mask-position follows the standard grammar, environments that support mask-position generally interpret these logical keywords as part of the standard position syntax [1][2]. You can use these keywords in the same manner as physical keywords within the property value, for example: mask-position: inline-start 10px block-start 20px; This will position the mask 10px from the inline-start edge and 20px from the block-start edge of the element's positioning area, adapting automatically if the writing mode changes [4][2].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/mask-position
- 2: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/position_value
- 3: https://caniuse.com/mdn-css_properties_mask-position
- 4: https://developer.mozilla.org/en-US/docs/Web/CSS/Guides/Masking/Mask_properties
Use the inline-side mask position for the scrollbar strip.
--cl-scroll-fade-inset is documented as holding the fade back from a strip at the inline end, but maskPosition: 'left top, right top' fixes the opaque strip to the physical right edge. With a non-zero inset in an RTL layout, the fade would cover the actual scrollbar space on the left while the right side has an unfaded opaque strip. Use logical keywords that match the scrollbar gutter/overlay direction.
🌐 Proposed fix using logical position keywords
mask: {
maskImage,
- maskPosition: 'left top, right top',
+ maskPosition: 'inline-start top, inline-end top',
maskRepeat: 'no-repeat',
mask
size: `calc(100% - ${fadeInset}) 100%, ${fadeInset} 100%`,
},🤖 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/scroll-area/scroll-area.styles.ts` around
lines 40 - 48, Update the mask positioning associated with maskImage so the
scrollbar strip uses the logical inline-end position rather than a physical
right-edge position. Replace the physical positioning keywords in the relevant
maskPosition declaration with logical keywords, preserving the existing
first-layer positioning and inset behavior for both LTR and RTL layouts.
@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: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.ts (1)
3-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMulti-line comments conflict with the one-terse-line comment guideline.
Both files carry multi-paragraph or multi-line comments. The repository guideline states comments should be kept to one terse line rather than a verbose multi-line block, even when the comment is warranted.
packages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.ts#L3-L34: condense the 3-line pre-selector comment and the 16-line JSDoc block into single terse lines, or move the rationale into a design doc/PR description.packages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.ts#L55-L57: condense the 3-line inline comment aboutobserveChildren.packages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsx#L110-L111: condense the two-linesetOverflowcomment.packages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsx#L117-L118: condense the two-linerenderViewportcomment.packages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsx#L143-L144: condense the two-line comment above the "already reachable" test.As per coding guidelines, "Keep code comments minimal. Add comments only when critical to explain why a non-obvious change was made; never restate code behavior, and keep warranted comments to one terse line rather than a verbose multi-line block."
🤖 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/scroll-area/use-scroller-focusable.ts` around lines 3 - 34, Condense the comments around FOCUSABLE_SELECTOR and the scroller-focusability JSDoc in packages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.ts lines 3-34 into terse single-line comments, preserving only critical rationale; also condense the observeChildren comment in use-scroller-focusable.ts lines 55-57 and the setOverflow, renderViewport, and “already reachable” test comments in packages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsx lines 110-111, 117-118, and 143-144 into single terse lines.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/scroll-area/use-scroller-focusable.ts`:
- Around line 6-17: Update FOCUSABLE_SELECTOR to exclude input elements with
type="hidden", and include "type" in the attributeFilter used by sync’s observer
so runtime hidden/visible type changes re-trigger synchronization.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.ts`:
- Around line 3-34: Condense the comments around FOCUSABLE_SELECTOR and the
scroller-focusability JSDoc in
packages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.ts lines
3-34 into terse single-line comments, preserving only critical rationale; also
condense the observeChildren comment in use-scroller-focusable.ts lines 55-57
and the setOverflow, renderViewport, and “already reachable” test comments in
packages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsx lines
110-111, 117-118, and 143-144 into single terse lines.
🪄 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: be328d0f-f4e2-41f2-b7ca-ddc085812614
📒 Files selected for processing (5)
.changeset/mosaic-scroll-area.mdpackages/swingset/src/stories/scroll-area.component.mdxpackages/ui/src/mosaic/components/scroll-area/scroll-area.test.tsxpackages/ui/src/mosaic/components/scroll-area/scroll-area.tsxpackages/ui/src/mosaic/components/scroll-area/use-scroller-focusable.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 (2)
- .changeset/mosaic-scroll-area.md
- packages/ui/src/mosaic/components/scroll-area/scroll-area.tsx
| const FOCUSABLE_SELECTOR = [ | ||
| 'a[href]', | ||
| 'button:not([disabled])', | ||
| 'input:not([disabled])', | ||
| 'select:not([disabled])', | ||
| 'textarea:not([disabled])', | ||
| 'audio[controls]', | ||
| 'video[controls]', | ||
| 'details > summary', | ||
| '[contenteditable]:not([contenteditable="false"])', | ||
| '[tabindex]:not([tabindex="-1"])', | ||
| ].join(','); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix: input:not([disabled]) matches hidden inputs, defeating the reachability check.
FOCUSABLE_SELECTOR matches any non-disabled <input>, including <input type="hidden">. Browsers never treat a hidden input as keyboard-focusable. In sync() (Line 48), contentIsReachable becomes true when a scroller's only interactive descendant is a hidden input (for example, a CSRF token field inside a scrollable form). The viewport then skips the managed tab stop even though nothing inside it is actually reachable by keyboard, which reintroduces the exact WCAG 2.1.1 gap this hook exists to close.
🐛 Proposed fix
const FOCUSABLE_SELECTOR = [
'a[href]',
'button:not([disabled])',
- 'input:not([disabled])',
+ 'input:not([disabled]):not([type="hidden"])',
'select:not([disabled])',
'textarea:not([disabled])',
'audio[controls]',
'video[controls]',
'details > summary',
'[contenteditable]:not([contenteditable="false"])',
'[tabindex]:not([tabindex="-1"])',
].join(',');Consider also adding 'type' to the attributeFilter (Line 76) so a runtime type change (hidden ↔ visible) re-triggers sync().
📝 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.
| const FOCUSABLE_SELECTOR = [ | |
| 'a[href]', | |
| 'button:not([disabled])', | |
| 'input:not([disabled])', | |
| 'select:not([disabled])', | |
| 'textarea:not([disabled])', | |
| 'audio[controls]', | |
| 'video[controls]', | |
| 'details > summary', | |
| '[contenteditable]:not([contenteditable="false"])', | |
| '[tabindex]:not([tabindex="-1"])', | |
| ].join(','); | |
| const FOCUSABLE_SELECTOR = [ | |
| 'a[href]', | |
| 'button:not([disabled])', | |
| 'input:not([disabled]):not([type="hidden"])', | |
| 'select:not([disabled])', | |
| 'textarea:not([disabled])', | |
| 'audio[controls]', | |
| 'video[controls]', | |
| 'details > summary', | |
| '[contenteditable]:not([contenteditable="false"])', | |
| '[tabindex]:not([tabindex="-1"])', | |
| ].join(','); |
🤖 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/scroll-area/use-scroller-focusable.ts`
around lines 6 - 17, Update FOCUSABLE_SELECTOR to exclude input elements with
type="hidden", and include "type" in the attributeFilter used by sync’s observer
so runtime hidden/visible type changes re-trigger synchronization.
Brings `ScrollArea` in line with the rest of Mosaic, where every part is polymorphic through `render`. A scrolling region often wants semantics of its own — a list of members is a `<ul>`, a labelled region is a `<section>` — and without this the caller had to choose between the component's styling and the right element. Both parts take it rather than just the viewport: a compound component where only one half is polymorphic is a trap. On the viewport the rendered element has to be able to establish a scroll box, since the overflow, mask and scroll timelines all apply to whatever lands there. `useRender` merges an array of refs, which also replaces the hand-rolled ref composition the viewport needed to observe its own element while still honouring the caller's ref. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nent Everything the scroll area does is CSS, so wrapping it in a component only added a DOM node to every scrolling surface and an API to version. `scrollAreaViewport` returns the atoms for the element that scrolls and `scrollAreaRoot` styles a positioned ancestor, so the styles ride on an element that already exists — an `Item.Group`, a list, a panel body — rather than introducing one. That also drops the `render` prop, since applying styles to your own element is polymorphism by construction, and it removes the `.cl-scroll-area-*` classes: the host element keeps its own slot class, and that stays the hook a theme targets. The swingset examples put the atoms on an `Item.Group`, so the override story is written against `.cl-item-group`. The managed tab stop goes with it. It was the only JavaScript here, and `tabindex` cannot be expressed as a style, so the Safari keyboard gap is now the caller's to close — documented, with the rule the browsers themselves use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n page A dedicated ScrollArea page made sense when there was a ScrollArea component. Now that it ships as atoms, a component page for a thing that isn't a component reads as a mistake, and the atoms are only meaningful applied to something. `Item` gains a `Scrolling` example: a capped-height group of organizations with the scroll surface spread onto the `Item.Group` itself, which is the shape a real call site takes. The page absorbs what the ScrollArea docs carried that has no other home — the theme tokens, the two progress vars, the override recipe, and the two caveats worth knowing (the mask covers the scrollbar until a custom scrollbar exists, and Safari's keyboard gap is the caller's to close). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A touch platform draws an overlay scrollbar there is no width or colour to apply to, and thinning a target that is already hard to hit would be actively worse. Gating appearance on `@media (pointer: fine)` leaves those platforms with the bar they already draw. Shopify's `s-scroll-box` gates the same way. `scrollbar-gutter` stays ungated: reserving space is a layout decision about the surrounding content, not an appearance one, and the value that is right for a list that can change height doesn't change with the pointer. Forced-colors still wins inside the gate — StyleX nests the two at-rules and triples the class, so the system scrollbar comes back where it has to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maxyinger
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/scroll-area/scroll-area.test.ts (1)
15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCheck the gutter declaration after
stylex.props.
not.toEqualonly shows that thestableandautoStyleX atom arrays are different. They could still misspell the property, use a different value, or compile to an unintended class. Convert each atom withstylex.props(...)and assert that the emittedclassName/__stylexPropsshape contains the expectedscrollbar-gutterdeclaration.🤖 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/scroll-area/scroll-area.test.ts` around lines 15 - 21, Update the scrollAreaViewport tests to pass each returned atom through stylex.props and assert the emitted className/__stylexProps shape contains the expected scrollbar-gutter declaration, including stable versus auto values and the default auto behavior. Replace the current array inequality/equality assertions while keeping coverage of the explicit and default arguments.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.
Nitpick comments:
In `@packages/ui/src/mosaic/components/scroll-area/scroll-area.test.ts`:
- Around line 15-21: Update the scrollAreaViewport tests to pass each returned
atom through stylex.props and assert the emitted className/__stylexProps shape
contains the expected scrollbar-gutter declaration, including stable versus auto
values and the default auto behavior. Replace the current array
inequality/equality assertions while keeping coverage of the explicit and
default arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e381f4f-31c5-45cb-a267-5849fb2b3079
📒 Files selected for processing (9)
.changeset/mosaic-scroll-area.mdpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/item.mdxpackages/swingset/src/stories/item.stories.tsxpackages/ui/src/mosaic/components/scroll-area/index.tspackages/ui/src/mosaic/components/scroll-area/scroll-area.styles.tspackages/ui/src/mosaic/components/scroll-area/scroll-area.test.tspackages/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 (2)
- packages/ui/src/mosaic/tokens.stylex.ts
- .changeset/mosaic-scroll-area.md
1 participant
Description
Adds a scroll surface to Mosaic: a scrolling region that fades its content at whichever edge
still has something to reveal.
It ships as StyleX atoms rather than a component, since everything it does is CSS — a
component would only add a DOM node to every scrolling surface and an API to version. Spread
scrollAreaViewport()onto an element you already render;scrollAreaRootstyles apositioned ancestor for the cases where an overlay has to anchor against the scroll box. The
atoms carry no class of their own, so the host element keeps its existing
.cl-<slot>, andthat stays the hook a theme targets.
Two scroll-driven animations write a progress var per edge and a mask reads them: no scroll
listener, no measurement, zero JavaScript. The vars are registered via @Property, which
makes them interpolate and — via initial-value: 0 — hides the indicators when there's
nothing to scroll, since an inactive timeline leaves them at their initial value. Only
animation-name is gated on @supports (animation-timeline: scroll()), so unsupported
browsers get a plain scrolling region rather than two permanent fades. The mask is paint-only
and can't shift content the way sticky overlay elements do.
Four theme tokens apply to every scrolling surface at once rather than to one component.
Scrollbar appearance is gated on @media (pointer: fine) so touch keeps its native overlay
bar.
Documented on the Item page under Scrolling, since the atoms are only meaningful applied
to something.
Known limitation: the mask covers the native scrollbar and we can't reliably avoid it.
--cl-scroll-fade-inset exists to hold the fade back, but CSS can't measure a scrollbar — the
width varies per platform and browser, and on macOS it changes at runtime when a mouse is
connected — so it defaults to 0px and any value is a guess. Solving it properly means a
custom scrollbar in the Base UI style, which needs JS.
Also deferred: Safari doesn't make overflowing scrollers keyboard-focusable and tabindex
isn't a style, so callers set tabIndex={0} themselves on a surface holding nothing focusable.
Checklist
Type of change