-
Notifications
You must be signed in to change notification settings - Fork 17
[PF-1994] Migrate Slider to @base-ui/react + Tailwind #4955
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/picasso-modernization-temp
Are you sure you want to change the base?
Changes from all commits
ba4f349
d5fa2fd
4147b54
b1644d9
f56a16d
dfee345
a86e31b
44ac229
16d3c50
f17b2b9
45ef754
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| '@toptal/picasso-slider': major | ||
| --- | ||
|
|
||
| ### Slider | ||
|
|
||
| - Migrate internals from @mui/base to @base-ui/react (behavioral parity) | ||
| - Rebuild marks and value-label rendering on top of @base-ui/react's compound parts (Slider.Root + Slider.Control + Slider.Track + Slider.Indicator + Slider.Thumb) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| "{\n \"firstThumb\": {\n \"rect\": {\n \"x\": 62,\n \"y\": 1392.7,\n \"width\": 15,\n \"height\": 15\n },\n \"style\": {\n \"margin-left\": \"1.5px\",\n \"margin-top\": \"0px\",\n \"top\": \"0.5px\",\n \"left\": \"0px\",\n \"translate\": \"-50% -50%\",\n \"transform\": \"none\",\n \"width\": \"15px\",\n \"height\": \"15px\",\n \"position\": \"absolute\",\n \"inset-inline-start\": \"0px\"\n }\n },\n \"firstThumbClassName\": \"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]:![clip-path:none] [&_input]:[clip:rect(0,0,0,0)]\",\n \"fiveThumbsRects\": [\n {\n \"x\": 62,\n \"y\": 1392.7,\n \"width\": 15,\n \"height\": 15\n },\n {\n \"x\": 147.12,\n \"y\": 1612.7,\n \"width\": 15,\n \"height\": 15\n },\n {\n \"x\": 192.81,\n \"y\": 1839.26,\n \"width\": 15,\n \"height\": 15\n },\n {\n \"x\": 62,\n \"y\": 2070.83,\n \"width\": 15,\n \"height\": 15\n },\n {\n \"x\": 102.57,\n \"y\": 2300.83,\n \"width\": 15,\n \"height\": 15\n }\n ]\n}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| "{\n \"thumb\": {\n \"rect\": {\n \"x\": 62,\n \"y\": 1392.7,\n \"width\": 15,\n \"height\": 15\n },\n \"style\": {\n \"margin-left\": \"-6px\",\n \"margin-top\": \"-7px\",\n \"padding-left\": \"0px\",\n \"padding-top\": \"0px\",\n \"left\": \"0px\",\n \"top\": \"6px\",\n \"right\": \"1055px\",\n \"bottom\": \"-2px\",\n \"transform\": \"none\",\n \"translate\": \"none\",\n \"transform-origin\": \"7.5px 7.5px\",\n \"width\": \"15px\",\n \"height\": \"15px\",\n \"box-sizing\": \"border-box\",\n \"position\": \"absolute\",\n \"inset-inline-start\": \"0px\",\n \"inset-block-start\": \"6px\",\n \"background-color\": \"rgb(32, 78, 207)\",\n \"border-left-width\": \"2px\",\n \"border-top-width\": \"2px\",\n \"border-radius\": \"50%\",\n \"box-shadow\": \"none\",\n \"filter\": \"none\",\n \"will-change\": \"auto\",\n \"isolation\": \"auto\",\n \"contain\": \"none\",\n \"opacity\": \"1\"\n }\n },\n \"thumbClassName\": \"base-Slider-thumb group/thumb flex justify-center items-center w-[15px] h-[15px] rounded-[50%] bg-blue-500 border-[2px] border-solid border-white -mt-[7px] -ml-[6px] outline-0 absolute transition-shadow cursor-pointer\",\n \"outerTooltip\": {\n \"rect\": {\n \"x\": 0,\n \"y\": 0,\n \"width\": 0,\n \"height\": 0\n },\n \"style\": {\n \"margin-left\": \"0px\",\n \"margin-top\": \"0px\",\n \"padding-left\": \"0px\",\n \"padding-top\": \"0px\",\n \"left\": \"calc(100% - 13px)\",\n \"top\": \"calc(100% + 2px)\",\n \"right\": \"auto\",\n \"bottom\": \"auto\",\n \"transform\": \"none\",\n \"translate\": \"none\",\n \"transform-origin\": \"50% 50%\",\n \"width\": \"auto\",\n \"height\": \"auto\",\n \"box-sizing\": \"border-box\",\n \"position\": \"absolute\",\n \"inset-inline-start\": \"calc(100% - 13px)\",\n \"inset-block-start\": \"calc(100% + 2px)\",\n \"background-color\": \"rgba(0, 0, 0, 0)\",\n \"border-left-width\": \"0px\",\n \"border-top-width\": \"0px\",\n \"border-radius\": \"0px\",\n \"box-shadow\": \"none\",\n \"filter\": \"none\",\n \"will-change\": \"transform\",\n \"isolation\": \"auto\",\n \"contain\": \"none\",\n \"opacity\": \"1\"\n }\n },\n \"outerTooltipClassName\": \"absolute will-change-transform transition-transform hidden top-[calc(100%+2px)] left-[calc(100%-13px)]\",\n \"innerTooltip\": {\n \"rect\": {\n \"x\": 0,\n \"y\": 0,\n \"width\": 0,\n \"height\": 0\n },\n \"style\": {\n \"margin-left\": \"4px\",\n \"margin-top\": \"4px\",\n \"padding-left\": \"8px\",\n \"padding-top\": \"2px\",\n \"left\": \"auto\",\n \"top\": \"auto\",\n \"right\": \"auto\",\n \"bottom\": \"auto\",\n \"transform\": \"none\",\n \"translate\": \"none\",\n \"transform-origin\": \"50% 50%\",\n \"width\": \"auto\",\n \"height\": \"auto\",\n \"box-sizing\": \"border-box\",\n \"position\": \"static\",\n \"inset-inline-start\": \"auto\",\n \"inset-block-start\": \"auto\",\n \"background-color\": \"rgb(38, 45, 61)\",\n \"border-left-width\": \"0px\",\n \"border-top-width\": \"0px\",\n \"border-radius\": \"4px\",\n \"box-shadow\": \"rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0.24) 0px 0px 4px 0px, rgba(0, 0, 0, 0.12) 0px 0px 32px 0px\",\n \"filter\": \"none\",\n \"will-change\": \"auto\",\n \"isolation\": \"auto\",\n \"contain\": \"none\",\n \"opacity\": \"1\"\n }\n },\n \"innerTooltipClassName\": \"shadow-4 text-sm text-white bg-graphite-800 m-1 rounded-sm py-[2px] px-2 max-w-[300px] break-words\"\n}" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems this file shouldn't be added to the PR
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Same root cause — see #discussion_r3265951267. All 5 stray JSON files batched into one operator action there. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| "{\n \"Default\": [],\n \"Initial value\": [],\n \"Tooltip\": [],\n \"Range\": [],\n \"Marks\": [],\n \"Disable track highlight\": []\n}" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems this file shouldn't be added to the PR
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Same root cause — see #discussion_r3265951267. |
||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| "{\n \"thumb\": {\n \"rect\": {\n \"x\": 62,\n \"y\": 1392.7,\n \"width\": 15,\n \"height\": 15\n },\n \"style\": {\n \"margin-left\": \"1.5px\",\n \"margin-top\": \"0px\",\n \"padding-left\": \"0px\",\n \"padding-top\": \"0px\",\n \"left\": \"0px\",\n \"top\": \"0.5px\",\n \"right\": \"1047.5px\",\n \"bottom\": \"-14.5px\",\n \"transform\": \"none\",\n \"translate\": \"-50% -50%\",\n \"transform-origin\": \"7.5px 7.5px\",\n \"width\": \"15px\",\n \"height\": \"15px\",\n \"box-sizing\": \"border-box\",\n \"position\": \"absolute\",\n \"inset-inline-start\": \"0px\",\n \"inset-block-start\": \"0.5px\",\n \"background-color\": \"rgb(32, 78, 207)\",\n \"border-left-width\": \"2px\",\n \"border-top-width\": \"2px\",\n \"border-radius\": \"50%\",\n \"box-shadow\": \"none\",\n \"filter\": \"none\",\n \"will-change\": \"auto\",\n \"isolation\": \"auto\",\n \"contain\": \"none\",\n \"opacity\": \"1\"\n }\n },\n \"thumbClassName\": \"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]:![clip-path:none] [&_input]:[clip:rect(0,0,0,0)]\",\n \"outerTooltip\": {\n \"rect\": {\n \"x\": 0,\n \"y\": 0,\n \"width\": 0,\n \"height\": 0\n },\n \"style\": {\n \"margin-left\": \"0px\",\n \"margin-top\": \"0px\",\n \"padding-left\": \"0px\",\n \"padding-top\": \"0px\",\n \"left\": \"calc(100% - 13px)\",\n \"top\": \"calc(100% + 2px)\",\n \"right\": \"auto\",\n \"bottom\": \"auto\",\n \"transform\": \"none\",\n \"translate\": \"none\",\n \"transform-origin\": \"50% 50%\",\n \"width\": \"auto\",\n \"height\": \"auto\",\n \"box-sizing\": \"border-box\",\n \"position\": \"absolute\",\n \"inset-inline-start\": \"calc(100% - 13px)\",\n \"inset-block-start\": \"calc(100% + 2px)\",\n \"background-color\": \"rgba(0, 0, 0, 0)\",\n \"border-left-width\": \"0px\",\n \"border-top-width\": \"0px\",\n \"border-radius\": \"0px\",\n \"box-shadow\": \"none\",\n \"filter\": \"none\",\n \"will-change\": \"transform\",\n \"isolation\": \"auto\",\n \"contain\": \"none\",\n \"opacity\": \"1\"\n }\n },\n \"outerTooltipClassName\": \"absolute will-change-transform transition-transform hidden top-[calc(100%+2px)] left-[calc(100%-13px)]\",\n \"innerTooltip\": {\n \"rect\": {\n \"x\": 0,\n \"y\": 0,\n \"width\": 0,\n \"height\": 0\n },\n \"style\": {\n \"margin-left\": \"4px\",\n \"margin-top\": \"4px\",\n \"padding-left\": \"8px\",\n \"padding-top\": \"2px\",\n \"left\": \"auto\",\n \"top\": \"auto\",\n \"right\": \"auto\",\n \"bottom\": \"auto\",\n \"transform\": \"none\",\n \"translate\": \"none\",\n \"transform-origin\": \"50% 50%\",\n \"width\": \"auto\",\n \"height\": \"auto\",\n \"box-sizing\": \"border-box\",\n \"position\": \"static\",\n \"inset-inline-start\": \"auto\",\n \"inset-block-start\": \"auto\",\n \"background-color\": \"rgb(38, 45, 61)\",\n \"border-left-width\": \"0px\",\n \"border-top-width\": \"0px\",\n \"border-radius\": \"4px\",\n \"box-shadow\": \"rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0.24) 0px 0px 4px 0px, rgba(0, 0, 0, 0.12) 0px 0px 32px 0px\",\n \"filter\": \"none\",\n \"will-change\": \"auto\",\n \"isolation\": \"auto\",\n \"contain\": \"none\",\n \"opacity\": \"1\"\n }\n },\n \"innerTooltipClassName\": \"shadow-4 text-sm text-white bg-graphite-800 m-1 rounded-sm py-[2px] px-2 max-w-[300px] break-words\"\n}" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems this file shouldn't be added to the PR
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Same root cause — see #discussion_r3265951267. |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||
| // import type { ComponentProps } from 'react' | ||||||||||||||||||||
| import React, { forwardRef, useRef } from 'react' | ||||||||||||||||||||
| import { Slider as MUIBaseSlider } from '@mui/base/Slider' | ||||||||||||||||||||
| import { useCombinedRefs, useOnScreen } from '@toptal/picasso-utils' | ||||||||||||||||||||
| import { Slider as BaseUISlider } from '@base-ui/react/slider' | ||||||||||||||||||||
| import type { SliderRootChangeEventDetails } from '@base-ui/react/slider' | ||||||||||||||||||||
| import { useOnScreen } from '@toptal/picasso-utils' | ||||||||||||||||||||
| import { twJoin, twMerge } from '@toptal/picasso-tailwind-merge' | ||||||||||||||||||||
| import type { BaseProps } from '@toptal/picasso-shared' | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -56,6 +56,61 @@ export interface Props extends BaseProps { | |||||||||||||||||||
| id?: string | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const formatValueLabel = ( | ||||||||||||||||||||
| value: number, | ||||||||||||||||||||
| index: number, | ||||||||||||||||||||
| format?: string | ((value: number, index: number) => React.ReactNode) | ||||||||||||||||||||
| ): React.ReactNode => { | ||||||||||||||||||||
| if (typeof format === 'function') { | ||||||||||||||||||||
| return format(value, index) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (typeof format === 'string') { | ||||||||||||||||||||
| return format | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return value | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const generateMarkPositions = ( | ||||||||||||||||||||
| min: number, | ||||||||||||||||||||
| max: number, | ||||||||||||||||||||
| step: number | ||||||||||||||||||||
| ): number[] => { | ||||||||||||||||||||
| const positions: number[] = [] | ||||||||||||||||||||
| const range = max - min | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (range <= 0 || step <= 0) { | ||||||||||||||||||||
| return positions | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| for (let mark = min; mark <= max; mark += step) { | ||||||||||||||||||||
| positions.push(mark) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return positions | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const resolveThumbValues = ( | ||||||||||||||||||||
| value: number | number[] | undefined, | ||||||||||||||||||||
| defaultValue: number | number[], | ||||||||||||||||||||
| min: number | ||||||||||||||||||||
| ): number[] => { | ||||||||||||||||||||
| if (Array.isArray(value)) { | ||||||||||||||||||||
| return value | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (typeof value === 'number') { | ||||||||||||||||||||
| return [value] | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (Array.isArray(defaultValue)) { | ||||||||||||||||||||
| return defaultValue | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return [typeof defaultValue === 'number' ? defaultValue : min] | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export const Slider = forwardRef<HTMLElement, Props>(function Slider( | ||||||||||||||||||||
| { defaultValue = 0, min = 0, max = 100, tooltip = 'off', ...props }, | ||||||||||||||||||||
| ref | ||||||||||||||||||||
|
|
@@ -64,7 +119,7 @@ export const Slider = forwardRef<HTMLElement, Props>(function Slider( | |||||||||||||||||||
| marks, | ||||||||||||||||||||
| value, | ||||||||||||||||||||
| tooltipFormat, | ||||||||||||||||||||
| step, | ||||||||||||||||||||
| step = 1, | ||||||||||||||||||||
| disabled, | ||||||||||||||||||||
| onChange, | ||||||||||||||||||||
| onBlur, | ||||||||||||||||||||
|
|
@@ -79,7 +134,6 @@ export const Slider = forwardRef<HTMLElement, Props>(function Slider( | |||||||||||||||||||
| 'data-testid': dataTestid, | ||||||||||||||||||||
| } = props | ||||||||||||||||||||
| const containerRef = useRef<HTMLDivElement>(null) | ||||||||||||||||||||
| const sliderRef = useCombinedRefs<HTMLElement>(ref, useRef<HTMLElement>(null)) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // The rootMargin is not working correctly in the storybooks iframe | ||||||||||||||||||||
| // To test properly we can open the iframe in new window | ||||||||||||||||||||
|
|
@@ -98,69 +152,119 @@ export const Slider = forwardRef<HTMLElement, Props>(function Slider( | |||||||||||||||||||
| const isThumbHidden = | ||||||||||||||||||||
| hideThumbOnEmpty && (typeof value === 'undefined' || value === null) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const isRange = Array.isArray(value) || Array.isArray(defaultValue) | ||||||||||||||||||||
| const thumbValues = resolveThumbValues(value, defaultValue, min) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const handleValueChange = ( | ||||||||||||||||||||
| newValue: number | readonly number[], | ||||||||||||||||||||
| eventDetails: SliderRootChangeEventDetails | ||||||||||||||||||||
| ) => { | ||||||||||||||||||||
| if (!onChange) { | ||||||||||||||||||||
| return | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const normalizedValue: number | number[] = Array.isArray(newValue) | ||||||||||||||||||||
| ? [...newValue] | ||||||||||||||||||||
| : (newValue as number) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| onChange(eventDetails.event, normalizedValue, eventDetails.activeThumbIndex) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Override base-ui's inline `translate: -50% -50%` + `top: 50%` (creates a | ||||||||||||||||||||
| // transform stacking context that affects descendant tooltip box-shadow | ||||||||||||||||||||
| // rasterization) and mirror baseline's margin-based positioning so the | ||||||||||||||||||||
| // thumb renders without a transform — keeps tooltip shadow pixel-identical | ||||||||||||||||||||
| // to pre-migration baseline. | ||||||||||||||||||||
| 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', | ||||||||||||||||||||
| '![translate:none] ![top:-7px] -ml-[6px]', | ||||||||||||||||||||
| '[&_input]:!top-auto [&_input]:!left-auto [&_input]:![clip-path:none] [&_input]:[clip:rect(0,0,0,0)]', | ||||||||||||||||||||
| isThumbHidden && 'hidden' | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const indicatorClassName = twJoin( | ||||||||||||||||||||
| 'block !absolute h-[1px]', | ||||||||||||||||||||
| disableTrackHighlight ? 'bg-gray-200' : 'bg-blue-500' | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const markPositions = marks ? generateMarkPositions(min, max, step) : [] | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <div | ||||||||||||||||||||
| ref={containerRef} | ||||||||||||||||||||
| className={twMerge('my-[6px] mx-0', className)} | ||||||||||||||||||||
| style={style} | ||||||||||||||||||||
| > | ||||||||||||||||||||
| <MUIBaseSlider | ||||||||||||||||||||
| ref={sliderRef} | ||||||||||||||||||||
| <BaseUISlider.Root | ||||||||||||||||||||
| ref={ref as React.Ref<HTMLDivElement>} | ||||||||||||||||||||
| defaultValue={defaultValue} | ||||||||||||||||||||
| value={value} | ||||||||||||||||||||
| min={min} | ||||||||||||||||||||
| max={max} | ||||||||||||||||||||
| step={step} | ||||||||||||||||||||
| marks={marks} | ||||||||||||||||||||
| disabled={disabled} | ||||||||||||||||||||
| name={name} | ||||||||||||||||||||
| data-testid={dataTestid} | ||||||||||||||||||||
| data-private={dataPrivate} | ||||||||||||||||||||
| onFocus={onFocus} | ||||||||||||||||||||
| onBlur={onBlur} | ||||||||||||||||||||
| name={name} | ||||||||||||||||||||
| id={id} | ||||||||||||||||||||
| slots={{ | ||||||||||||||||||||
| mark: SliderMark, | ||||||||||||||||||||
| valueLabel: SliderValueLabel, | ||||||||||||||||||||
| }} | ||||||||||||||||||||
| slotProps={{ | ||||||||||||||||||||
| mark: { | ||||||||||||||||||||
| // @ts-expect-error we have custom Mark component, where we extend props and MUI does not understand it | ||||||||||||||||||||
| forceInactive: disableTrackHighlight, | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| root: { | ||||||||||||||||||||
| className: | ||||||||||||||||||||
| 'block cursor-pointer width-full relative py-[6px] -my-[6px]', | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| rail: { | ||||||||||||||||||||
| className: | ||||||||||||||||||||
| 'block absolute w-full h-[1px] opacity-[0.24] rounded-none bg-gray-500', | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| thumb: { | ||||||||||||||||||||
| className: twJoin( | ||||||||||||||||||||
| 'group/thumb flex justify-center items-center w-[15px] h-[15px]', | ||||||||||||||||||||
| 'rounded-[50%] bg-blue-500 border-[2px] border-solid border-white', | ||||||||||||||||||||
| '-mt-[7px] -ml-[6px] outline-0 absolute transition-shadow cursor-pointer', | ||||||||||||||||||||
| isThumbHidden && 'hidden' | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| role: 'slider', | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| track: { | ||||||||||||||||||||
| className: twJoin( | ||||||||||||||||||||
| 'block absolute h-[1px]', | ||||||||||||||||||||
| disableTrackHighlight ? 'bg-gray-200' : 'bg-blue-500' | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| valueLabel: { | ||||||||||||||||||||
| tooltip: isObserved ? tooltip : 'off', | ||||||||||||||||||||
| onRender: handleValueLabelOnRender, | ||||||||||||||||||||
| yPlacement: isOnScreen ? 'top' : 'bottom', | ||||||||||||||||||||
| isOverlaped: isPartiallyOverlapped, | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| }} | ||||||||||||||||||||
| valueLabelFormat={tooltipFormat} | ||||||||||||||||||||
| onChange={onChange} | ||||||||||||||||||||
| /> | ||||||||||||||||||||
| onValueChange={handleValueChange} | ||||||||||||||||||||
| className='block cursor-pointer w-full relative py-[6px] -my-[6px]' | ||||||||||||||||||||
| > | ||||||||||||||||||||
| <BaseUISlider.Control className='block w-full relative h-[1px] -mb-[1px]'> | ||||||||||||||||||||
| <BaseUISlider.Track className='block !absolute w-full h-[1px] opacity-[0.24] rounded-none bg-gray-500' /> | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think absolute positioning is wrong here, as it won't place the track correctly if the component will be used on the page in various layout locations. Also, we shouldn't use important decorations as it's a bad practice to force override the style
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both points are fair. Let me address them separately: On On That said, if you want the cleaner shape, there's a deeper refactor: Option A (minimal, no visual change) — drop - <BaseUISlider.Track className="block !absolute w-full h-[1px] opacity-[0.24] rounded-none bg-gray-500" />
+ <BaseUISlider.Track style={{ position: 'absolute' }} className="block w-full h-[1px] opacity-[0.24] rounded-none bg-gray-500" />Same pattern for Indicator + Thumb (Thumb gets Option B (structural) — nest Indicator + Thumbs inside Track to follow base-ui's recommended composition (Track > Indicator + Thumb). Track stays Leaning toward Option A for this PR (small, no visual delta, addresses both your concerns for the public-facing element) and tracking Option B as a follow-up after the migration lands. Apply Option A? 👍 to confirm, or |
||||||||||||||||||||
| <BaseUISlider.Indicator className={indicatorClassName} /> | ||||||||||||||||||||
| {marks && | ||||||||||||||||||||
| markPositions.map((markValue, index) => ( | ||||||||||||||||||||
| <SliderMark | ||||||||||||||||||||
| key={`mark-${markValue}`} | ||||||||||||||||||||
| index={index} | ||||||||||||||||||||
| markValue={markValue} | ||||||||||||||||||||
| sliderValue={value} | ||||||||||||||||||||
| positionPercent={ | ||||||||||||||||||||
| max === min ? 0 : ((markValue - min) / (max - min)) * 100 | ||||||||||||||||||||
| } | ||||||||||||||||||||
| forceInactive={Boolean(disableTrackHighlight)} | ||||||||||||||||||||
| /> | ||||||||||||||||||||
| ))} | ||||||||||||||||||||
| {thumbValues.map((thumbValue, index) => { | ||||||||||||||||||||
| const thumbId = index === 0 ? id : undefined | ||||||||||||||||||||
| const label = formatValueLabel(thumbValue, index, tooltipFormat) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <BaseUISlider.Thumb | ||||||||||||||||||||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
The visual rect coincides, but the path taken to get there is different — baseline uses margin-based centering, base-ui uses a CSS Tooltip inner-span box-shadow is identical in both ( Applied fix (latest commit): override base-ui's inline Result: thumb visual position unchanged (still 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. |
||||||||||||||||||||
| // eslint-disable-next-line react/no-array-index-key | ||||||||||||||||||||
| key={`thumb-${index}`} | ||||||||||||||||||||
| index={index} | ||||||||||||||||||||
| id={thumbId} | ||||||||||||||||||||
| onFocus={onFocus} | ||||||||||||||||||||
| onBlur={onBlur} | ||||||||||||||||||||
| className={thumbClassName} | ||||||||||||||||||||
| > | ||||||||||||||||||||
| <SliderValueLabel | ||||||||||||||||||||
| index={index} | ||||||||||||||||||||
| value={thumbValue} | ||||||||||||||||||||
| tooltip={isObserved ? tooltip : 'off'} | ||||||||||||||||||||
| onRender={handleValueLabelOnRender} | ||||||||||||||||||||
| yPlacement={isOnScreen ? 'top' : 'bottom'} | ||||||||||||||||||||
| isOverlaped={isPartiallyOverlapped} | ||||||||||||||||||||
| > | ||||||||||||||||||||
| {label} | ||||||||||||||||||||
| </SliderValueLabel> | ||||||||||||||||||||
| </BaseUISlider.Thumb> | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| })} | ||||||||||||||||||||
| {!isRange && thumbValues.length === 0 && ( | ||||||||||||||||||||
| <BaseUISlider.Thumb | ||||||||||||||||||||
| index={0} | ||||||||||||||||||||
| id={id} | ||||||||||||||||||||
| onFocus={onFocus} | ||||||||||||||||||||
| onBlur={onBlur} | ||||||||||||||||||||
| className={thumbClassName} | ||||||||||||||||||||
| /> | ||||||||||||||||||||
| )} | ||||||||||||||||||||
| </BaseUISlider.Control> | ||||||||||||||||||||
| </BaseUISlider.Root> | ||||||||||||||||||||
| </div> | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| }) | ||||||||||||||||||||
|
|
||||||||||||||||||||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems this file shouldn't be added to the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right — these 5 JSON files are diagnostic snapshots (computed-style dumps + thumb-position rects) the agent saved while investigating earlier Happo regressions. They should have been written under
migration-runs/<date>/<Component>/(gitignored), not at the repo root. They were committed in error.The orchestrator's review-response permission set blocks
rm/git rm/ direct filesystem deletion without operator approval (sandbox policy), so I can't stage the deletion from this sweep tick. Flagging for operator:git rm after-fix-thumbs.json baseline-tooltip-computed.json \ local-thumb-computed.json local-thumbs-all.json \ local-tooltip-computed.jsonWill follow up to your other 4 file comments with a pointer here so they're not abandoned.