Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR migrates color picker slider components from Radix UI to Base UI, updating prop types from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@packages/raystack/components/color-picker/color-picker-alpha.tsx`:
- Around line 19-31: The Slider.onValueChange currently casts value to number
which will yield NaN when Base UI emits a range (number[]) — update the
onValueChange handler in the ColorPickerAlpha component to normalize the input:
detect Array.isArray(value) and extract a single numeric value (e.g., value[0])
or otherwise coerce to a number, then compute alpha as that number / 100 and
call setColor({ alpha }). Ensure this change is applied where onValueChange is
defined and reference the ColorPickerAlphaProps/Slider.Root.Props types so the
handler safely supports both number and number[] inputs.
| onValueChange={value => setColor({ alpha: (value as number) / 100 })} | ||
| step={1} | ||
| value={[alpha * 100]} | ||
| value={alpha * 100} | ||
| thumbAlignment='edge' | ||
| {...props} | ||
| > | ||
| <Slider.Track className={cx(styles.sliderTrack, styles.alphaTrack)}> | ||
| <div className={styles.alphaTrackGradient} /> | ||
| <Slider.Range className={styles.sliderRange} /> | ||
| </Slider.Track> | ||
| <Slider.Thumb className={styles.sliderThumb} /> | ||
| <Slider.Control className={styles.sliderControl}> | ||
| <Slider.Track className={cx(styles.sliderTrack, styles.alphaTrack)}> | ||
| <div className={styles.alphaTrackGradient} /> | ||
| <Slider.Indicator className={styles.sliderRange} /> | ||
| <Slider.Thumb className={styles.sliderThumb} aria-label='Alpha' /> | ||
| </Slider.Track> | ||
| </Slider.Control> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Base UI Slider onValueChange callback value type documentation
💡 Result:
In Base UI’s Slider (Slider.Root), the onValueChange callback receives the new value as:
number | number[](an array is used for range sliders)
Signature (from the API docs):
onValueChange?: (value: number | number[], eventDetails: Slider.Root.ChangeEventDetails) => void[1]
Base UI change handlers also always receive an eventDetails object as the second argument (with fields like reason, event, cancel(), etc.) [2].
Sources: [1], [2]
🏁 Script executed:
# Find and examine the color-picker-alpha.tsx file
fd -t f "color-picker-alpha.tsx" --exec cat -n {} \;Repository: raystack/apsara
Length of output: 1373
🏁 Script executed:
# Check the package.json to see Base UI version
cat package.json | grep -A 5 -B 5 "@base-ui" || cat package.json | grep -i sliderRepository: raystack/apsara
Length of output: 41
🏁 Script executed:
# Search for any range prop configuration in color-picker components
fd -t f "color-picker.*\\.tsx" --exec grep -l "range" {} \;Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
# Check for any constraints or configuration preventing range in Slider.Root
rg "range" packages/raystack/components/color-picker/ -A 2 -B 2Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
# Look for Base UI version in lock files or any config
fd -t f "package-lock.json|yarn.lock|pnpm-lock.yaml" --exec grep -A 1 "@base-ui/react" {} \; | head -20Repository: raystack/apsara
Length of output: 376
Guard against range values in onValueChange.
Base UI's Slider can emit number | number[] in the onValueChange callback. Since ColorPickerAlphaProps exposes the full Slider.Root.Props, a consumer could pass range={true}, causing the value to be an array. The current cast to number would produce NaN if an array is passed. Add a guard to handle both cases:
♻️ Suggested guard for single-value normalization
- onValueChange={value => setColor({ alpha: (value as number) / 100 })}
+ onValueChange={value => {
+ const next = Array.isArray(value) ? value[0] : value;
+ if (typeof next === 'number') {
+ setColor({ alpha: next / 100 });
+ }
+ }}🤖 Prompt for AI Agents
In `@packages/raystack/components/color-picker/color-picker-alpha.tsx` around
lines 19 - 31, The Slider.onValueChange currently casts value to number which
will yield NaN when Base UI emits a range (number[]) — update the onValueChange
handler in the ColorPickerAlpha component to normalize the input: detect
Array.isArray(value) and extract a single numeric value (e.g., value[0]) or
otherwise coerce to a number, then compute alpha as that number / 100 and call
setColor({ alpha }). Ensure this change is applied where onValueChange is
defined and reference the ColorPickerAlphaProps/Slider.Root.Props types so the
handler safely supports both number and number[] inputs.
1 participant
Description
This PR migrates the ColorPicker component to use Base UI primitives.
Changes
ColorPicker.Hue&ColorPicker.Alphasliders to use Base UI primitivesSummary by CodeRabbit
Refactor
Style
Accessibility