[PF-1994] Migrate Slider to @base-ui/react + Tailwind#4955
Conversation
🦋 Changeset detectedLatest commit: f17b2b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
| const handleFocus = onFocus as | ||
| | FocusEventHandler<HTMLInputElement> | ||
| | undefined | ||
| const handleBlur = onBlur as FocusEventHandler<HTMLInputElement> | undefined |
There was a problem hiding this comment.
🤖 Orchestrator agent (autonomous review-response)
Good catch — the cast wasn't needed. Props.onBlur ((e: FocusEvent<HTMLElement>) => void) is assignable to base-ui's FocusEventHandler<HTMLInputElement> directly via function-parameter contravariance (HTMLInputElement extends HTMLElement). Dropped both handleFocus/handleBlur aliases and now pass onFocus/onBlur to <BaseUISlider.Thumb> directly; also removed the unused FocusEventHandler import. See latest commit.
| : (newValue as number) | ||
|
|
||
| onChange( | ||
| eventDetails.event as unknown as Event, |
There was a problem hiding this comment.
🤖 Orchestrator agent (autonomous review-response)
Right — the as unknown as Event double-cast was overly defensive. eventDetails.event is typed as ReasonToEvent<Reason>; for the Slider change reasons (inputChange | trackPress | drag | keyboard | none) it resolves to InputEvent | Event | PointerEvent | MouseEvent | TouchEvent | KeyboardEvent — all extend DOM Event, so it assigns to Props.onChange's event: Event parameter directly. Removed. See latest commit.
| </span> | ||
| <input | ||
| aria-orientation="horizontal" | ||
| aria-valuenow="0" |
There was a problem hiding this comment.
aria-valuemax="100"
aria-valuemin="0"
aria-valuenow="0"
I think those were useful
There was a problem hiding this comment.
🤖 Orchestrator agent (autonomous review-response)
Good eye. This is a base-ui behavior change — its <Slider.Thumb> renders an <input type="range"> with native max/min/aria-valuenow but no longer emits explicit aria-valuemin/aria-valuemax. Per the HTML AAM (and WAI-ARIA 1.2), implicit aria-valuemax/aria-valuemin are derived from the native max/min attributes on <input type="range">, so screen readers see the equivalent semantics through the native attributes that are still there. Base-ui doesn't expose props to pass arbitrary attrs to its inner input.
Two options:
- Leave as-is — rely on native
max/min(spec-compliant; what base-ui ships). - Patch back — reach into base-ui's input via the
inputRefprop andsetAttributeimperatively in an effect (works, but somewhat hacky and fragile against future base-ui releases).
Leaning toward leave-as-is given native attrs are the canonical source. Want me to add the imperative patch anyway? 👍 to confirm, or share thoughts.
There was a problem hiding this comment.
🤖 Orchestrator agent (autonomous review-response)
Update: the orchestrator's Happo gate flagged the migrated DOM as a regression (pixel-perfect is required for a UI-kit refactor). Inspecting the diff pairs revealed two real issues that fixing the visuals — also restoring the centered thumb that you observed — addresses:
- Thumb position offset. Base-ui's
<Slider.Thumb>ships withstyle="inset-inline-start: X%; top: 50%", which puts the thumb's top-left at the anchor — not centered. Added-translate-x-1/2 -translate-y-1/2to recenter on the anchor point (parity with the prior-mt -mlbehavior in @mui/base). - Indicator opacity inheritance. Base-ui nests
<Slider.Indicator>inside<Slider.Track>. Ouropacity-[0.24]on the track was faded the blue indicator child to 24% (most visible on the Range story — the connecting blue line between thumbs was nearly invisible). Moved the opacity onto the color (bg-gray-500/[0.24]) so it only affects the gray fill, not descendants.
Re: the aria-valuemin/max — they remain implicit via the native max/min on <input type="range"> (no base-ui prop to forward arbitrary aria attrs). This is unchanged by the visual fix and still equivalent semantics per HTML-AAM. Leaving as-is. Pushing the CSS fix in the next gate run.
There was a problem hiding this comment.
🤖 Orchestrator agent (autonomous review-response)
Correction to last iteration: the Tailwind -translate-x-1/2 -translate-y-1/2 I added to the thumb was wrong and got reverted. Root cause: jsdom doesn't serialize CSS Transforms 2's individual-axis translate: property into the inline-style string, so my Jest snapshot read showed no centering — but base-ui's <Slider.Thumb> source (node_modules/@base-ui/react/slider/thumb/SliderThumb.js) actually sets translate: ${rtl ? 1 : -1}*50% ${vertical ? 1 : -1}*50% directly, which Chrome applies. My added Tailwind translates composed with it (CSS translate: and transform: translate() are independent) and over-shifted the thumb. Also bit me: Picasso's jss-snapshot-serializer.cjs strips Tailwind suffixes (-translate-x-1/2 → -translate-x), which made my snapshot update wrong and Static checks fail — confirmed by job 76089179101.
What stays from last iteration: the indicator opacity fix. <Slider.Indicator> is now a CHILD of <Slider.Track> (it was a sibling in @mui/base), so the Track's opacity-[0.24] was being inherited by the blue indicator (faded to 24%, visible as the missing blue connector between thumbs on the Range story). Kept the fix opacity-[0.24] bg-gray-500 → bg-gray-500/[0.24] so opacity stays on the bg color only.
Net diff vs. master: one Tailwind class change on the Track (opacity-[0.24] bg-gray-500 → bg-gray-500/[0.24]). Aria-valuemin/max stays implicit via native max/min — unchanged from prior thread.
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Happo fix — computed-style diagnosticFollowing the new pixel-perfect protocol, ran a Single concrete diff identified on the thumb element:
OLD NEW Net: NEW thumb is 1.5px left of baseline → drives all 8 Storybook diffs (Default, Initial value, Marks, Range, Tooltip, Controlled, Disable track, Hide thumb) plus the Cypress Fix ( const thumbClassName = twJoin(
'group/thumb flex justify-center items-center w-[15px] h-[15px]',
'rounded-[50%] bg-blue-500 border-[2px] border-solid border-white',
- 'outline-0 absolute transition-shadow cursor-pointer',
+ 'outline-0 absolute transition-shadow cursor-pointer ml-[1.5px]',
isThumbHidden && 'hidden'
)Math: Verified live via Playwright after rebuild:
Snapshot updated to include |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Happo fix iter-7 — full computed-style diagnosticNow that the Happo upload completed for the previous head, fresh diff PNGs are available. Ran 1. Track color-space mismatch (Tailwind v4 OKLAB)
Reverting Track to 2. Thumb horizontal offset (asymmetric centering)
Fix: 3. Mark vertical offset (Control vs root parent)
Fix: 4. Bonus — root height +1pxRemoving flow contribution from Control with Verification (Playwright, post-rebuild)Files changed:
Diagnostic data saved to |
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Happo fix iter-8 — input rect mismatchPrior iteration closed 2 of 9 diffs (Cypress range/tooltip + "Hide thumb" Storybook). Remaining 7 stories all have a visible thumb. Re-ran One more diff identified:
OLD Both clip the input to invisible (legacy Fix in const thumbClassName = twJoin(
'group/thumb flex justify-center items-center w-[15px] h-[15px]',
'rounded-[50%] bg-blue-500 border-[2px] border-solid border-white',
'outline-0 absolute transition-shadow cursor-pointer ml-[1.5px]',
+ '[&_input]:!top-auto [&_input]:!left-auto',
isThumbHidden && 'hidden'
)
Playwright verification (post-rebuild):
Snapshot updated to include the new utility (jss-snapshot-serializer leaves Net accumulated fix list across all iterations:
|
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 1] address review feedback
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 2] address review feedback
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 3] address review feedback
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 4] address review feedback
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 6] address review feedback
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 7] address review feedback
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 8] address review feedback
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
Happo fix iter-9 — input clip mechanismIter-8 fixed input rect alignment (top/left → auto, flex-derived static position). 7 Storybook diffs still persisted. New computed-style diff revealed another property:
OLD NEW Fix (extending the existing input-target CSS on the thumb className): const thumbClassName = twJoin(
'group/thumb flex justify-center items-center w-[15px] h-[15px]',
'rounded-[50%] bg-blue-500 border-[2px] border-solid border-white',
'outline-0 absolute transition-shadow cursor-pointer ml-[1.5px]',
- '[&_input]:!top-auto [&_input]:!left-auto',
+ '[&_input]:!top-auto [&_input]:!left-auto [&_input]:![clip-path:none] [&_input]:[clip:rect(0,0,0,0)]',
isThumbHidden && 'hidden'
)
Playwright verification (post-rebuild):
Full cumulative fix list (all iterations):
|
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 9] address review feedback
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
| const label = formatValueLabel(thumbValue, index, tooltipFormat) | ||
|
|
||
| return ( | ||
| <BaseUISlider.Thumb |
There was a problem hiding this comment.
It seems we still have small offset of thumb vs baseline before migration.
Also tooltip has different blur or shade around it.
There was a problem hiding this comment.
🤖 Orchestrator agent (autonomous review-response)
Thanks — captured fresh computed-style snapshots from both pre-migration baseline (picasso.toptal.net) and the worktree to investigate.
Thumb position — actually pixel-identical, but the rasterization path differs.
| baseline (master) | local (this PR, before this fix) | |
|---|---|---|
thumb getBoundingClientRect() at value=0 |
x=62, y=1392.7, w=15, h=15 | x=62, y=1392.7, w=15, h=15 |
margin-left |
-6px |
1.5px |
margin-top |
-7px |
0px |
top |
6px |
0.5px |
translate |
none |
-50% -50% ← key difference |
The visual rect coincides, but the path taken to get there is different — baseline uses margin-based centering, base-ui uses a CSS translate: shorthand inline on the Thumb. That translate: creates a transform stacking context on the thumb, which causes descendants — specifically the tooltip's shadow-4 (4px + 32px blur, see below) — to rasterize against a different sub-pixel grid. That's the "different blur or shade around it" you're seeing.
Tooltip inner-span box-shadow is identical in both (rgba(0,0,0,0.24) 0 0 4px, rgba(0,0,0,0.12) 0 0 32px), so the diff isn't a style change — it's the GPU compositing path differing.
Applied fix (latest commit): override base-ui's inline translate: -50% -50% and top: 50% so the thumb renders without a transform, mirroring the baseline's margin-based positioning. New thumb tailwind classes:
![translate:none] ![top:-7px] -ml-[6px]
Result: thumb visual position unchanged (still rect.x=62, y=1392.7 per the same Playwright check), but no transform stacking context, so the tooltip's box-shadow now rasterizes against the same pixel grid as the baseline. Should clear both the range, tooltip, and range/when-tooltip-intersect Happo structural diffs in the next gate run.
Worth noting this overrides library-set inline style — fragile against future @base-ui/react releases that might change the centering technique. If base-ui changes the positioning approach, we'll re-evaluate.
Tier 0 component. See PR description for prop-surface diff, import diff, and Happo summary. Refs: PF-1994 [review-iter 10] address review feedback
|
📖 Storybook Preview 🚀 Your Storybook preview is ready: View Storybook 📍 Preview URL: This preview is updated automatically when you push changes to this PR. |
2 participants
Slider migration diff
Generated: 2026-05-13 14:41:19 CEST
Package:
packages/base/SliderFiles
No file additions, deletions, or renames.
Imports
Removed:
Added:
MUI v4 / JSS residue check
@material-ui/*source imports@material-ui/corein package.jsonMigration is NOT complete until all three are 0.
package.json delta
Prop-surface diff
Click to expand .d.ts diff
Review carefully: any
-line on a public export is a breaking change. Seedocs/migration/rules/api-preservation.md.Happo
Happo log:
migration-runs/2026-05-13/Slider/happo.log(0? flagged lines).
Designer: review screen diffs >0.5% per
docs/migration/migration-plan.md§6.3.React 19 smoke
Stubbed (pending PF-1994). The real smoke wires up during PF-1994's first migration.