feat(color-picker): migrate to culori and add add OKLCH support#816
Open
paanSinghCoder wants to merge 7 commits into
Open
feat(color-picker): migrate to culori and add add OKLCH support#816paanSinghCoder wants to merge 7 commits into
paanSinghCoder wants to merge 7 commits into
Conversation
Replaces the `color` dependency with `culori` so the picker can accept and emit `oklch()` values alongside hex/rgb/hsl. The picker's HSL state shape is unchanged, so sliders and the context contract are untouched. OKLCH input is parsed via culori; OKLCH output is formatted with 4-decimal L/C, 2-decimal H, hue pinned to 0 for achromatic colors. Hex output remains uppercase with alpha-aware width to match the previous behavior. Note: sliders still operate in HSL — OKLCH is an I/O format, not a perceptual editing mode, so OKLCH round-trips are not bit-identical. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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:
📝 WalkthroughWalkthroughThis PR adds OKLCH color mode support to the ColorPicker component. The core change replaces the Sequence Diagram(s)sequenceDiagram
participant Client
participant ColorPickerRoot
participant Utils as utils.ts
participant Culori as culori
Client->>ColorPickerRoot: mount with value/defaultValue/mode
ColorPickerRoot->>Utils: parseColor(value/defaultValue)
Utils->>Culori: parse & convert to OKLCH
Culori-->>Utils: ColorObject { l,c,h,alpha }
ColorPickerRoot->>Utils: getColorString(ColorObject, mode)
Utils->>Culori: format to hex/hsl/rgb/oklch (and clamp when needed)
Culori-->>Utils: formatted string
ColorPickerRoot-->>Client: emit onValueChange(formatted string)
Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🤖 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/raystack/components/color-picker/utils.ts`:
- Around line 10-12: SUPPORTED_MODES is currently typed as string[] so ModeType
widens to string; change SUPPORTED_MODES to a readonly tuple by appending "as
const" to preserve literal types and make ModeType a union of
'hex'|'hsl'|'rgb'|'oklch'. Update the declaration of SUPPORTED_MODES (and any
related imports if necessary) so ModeType (derived from (typeof
SUPPORTED_MODES)[number]) correctly narrows, which will ensure getColorString
and its switch/default behavior can be type-checked against only valid modes.
- Around line 30-53: formatOklch currently uses round (which returns a Number
via Number.parseFloat(n.toFixed(p))) causing trailing zeros to be dropped and
violating the "4-decimal L/C, 2-decimal H" contract; change the implementation
so the values are formatted as fixed-width decimal strings (use toFixed(p)
directly) and ensure formatOklch concatenates those string results (update the
round helper to return a string or replace calls to round(L/C/H/alpha) in
formatOklch with num.toFixed(p)) so L and C are 4-decimal, H is 2-decimal, and
alpha is 4-decimal when included (refer to formatOklch and the round helper).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c786a0e2-b310-412e-81da-d1764df00178
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/www/src/app/examples/color-picker/page.tsxapps/www/src/content/docs/components/color-picker/demo.tsapps/www/src/content/docs/components/color-picker/index.mdxapps/www/src/content/docs/components/color-picker/props.tspackages/raystack/components/color-picker/__tests__/color-picker.test.tsxpackages/raystack/components/color-picker/color-picker-area.tsxpackages/raystack/components/color-picker/color-picker-input.tsxpackages/raystack/components/color-picker/color-picker-root.tsxpackages/raystack/components/color-picker/utils.tspackages/raystack/package.json
Comment on lines
+30
to
+53
| const round = (n: number, p: number) => Number.parseFloat(n.toFixed(p)); | ||
|
|
||
| /** | ||
| * Serializes a culori-shaped HSL color as oklch(L C H[ / A]). | ||
| * Matches the design system's token format: 4-decimal L/C, 2-decimal H, | ||
| * H pinned to 0 for achromatic colors (culori would emit `none` per CSS | ||
| * Color 4, which is correct but inconsistent with how tokens are written). | ||
| */ | ||
| const formatOklch = (hsl: { | ||
| mode: 'hsl'; | ||
| h: number; | ||
| s: number; | ||
| l: number; | ||
| alpha: number; | ||
| }): string => { | ||
| const oklch = toOklch(hsl); | ||
| if (!oklch) return ''; | ||
| const L = round(oklch.l ?? 0, 4); | ||
| const C = round(oklch.c ?? 0, 4); | ||
| const H = C === 0 || !Number.isFinite(oklch.h) ? 0 : round(oklch.h ?? 0, 2); | ||
| const body = `${L} ${C} ${H}`; | ||
| return hsl.alpha === 1 | ||
| ? `oklch(${body})` | ||
| : `oklch(${body} / ${round(hsl.alpha, 4)})`; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript, does Number.parseFloat((0.5).toFixed(4)) preserve trailing zeros in the resulting string representation?
💡 Result:
No, Number.parseFloat((0.5).toFixed(4)) does not preserve trailing zeros in the resulting value because it returns a number type, which does not store formatting information like trailing zeros [1][2]. Here is the breakdown of the process: 1. The.toFixed(4) method is called on the number 0.5. This method returns a string representation of the number, padded with zeros to the specified number of decimal places [3]. In this case, it returns the string "0.5000" [3]. 2. The Number.parseFloat function then takes that string ("0.5000") and parses it into a floating-point number [1][2]. 3. Because the result is a JavaScript Number (a 64-bit binary floating-point value), it cannot maintain trailing zeros [1][4]. When the number 0.5 is represented as a numeric type, it is simply 0.5, and any trailing zeros from the string representation are discarded [1]. If you need to preserve trailing zeros, you must keep the value as a string [3][4].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global%5FObjects/parseFloat
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/parseFloat
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toFixed
- 4: https://stackoverflow.com/questions/2283566/how-can-i-round-a-number-in-javascript-tofixed-returns-a-string
🏁 Script executed:
# First, locate and examine the actual file
fd -t f "utils.ts" -path "*color-picker*"Repository: raystack/apsara
Length of output: 228
🏁 Script executed:
# Read the specific file to see the code at lines 30-53
cat -n packages/raystack/components/color-picker/utils.ts | sed -n '25,60p'Repository: raystack/apsara
Length of output: 1501
🏁 Script executed:
# Search for tests related to formatOklch
fd -t f "*.test\|*.spec" -path "*color-picker*" | head -20Repository: raystack/apsara
Length of output: 228
🏁 Script executed:
# Check if there are any tests that validate the format
rg "formatOklch" --type typescript -A 5 -B 5Repository: raystack/apsara
Length of output: 94
🏁 Script executed:
# Find test files in color-picker directory
find packages/raystack/components/color-picker -type f \( -name "*.test.*" -o -name "*.spec.*" \)Repository: raystack/apsara
Length of output: 133
🏁 Script executed:
# Search for formatOklch usage in tests or elsewhere
rg "formatOklch" -B 3 -A 3Repository: raystack/apsara
Length of output: 1127
🏁 Script executed:
# Look for any validation of oklch format
rg "oklch\(" -B 2 -A 2 | head -100Repository: raystack/apsara
Length of output: 8924
🏁 Script executed:
# Check the test file for formatOklch tests
cat -n packages/raystack/components/color-picker/__tests__/color-picker.test.tsx | head -150Repository: raystack/apsara
Length of output: 5362
🏁 Script executed:
# Search for any format validation or assertions on oklch output
rg "oklch\(" packages/raystack/components/color-picker/__tests__/ -B 2 -A 2Repository: raystack/apsara
Length of output: 626
🏁 Script executed:
# Search for more tests related to format, especially comparing output
rg "formatOklch\|oklch\(" packages/raystack/components/color-picker/ -B 3 -A 5Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
# Look for utils.test or any unit tests specifically for utils.ts
find packages/raystack/components/color-picker -name "*utils*" -type fRepository: raystack/apsara
Length of output: 110
🏁 Script executed:
# Check if there's any validation of the output format or round function
rg "round\|toFixed" packages/raystack/components/color-picker/ -B 2 -A 2Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
# Create a test to demonstrate the actual behavior of formatOklch
cat > /tmp/test_format.js << 'EOF'
// Simulate the current implementation
const round = (n, p) => Number.parseFloat(n.toFixed(p));
// Test with values that would lose trailing zeros
const testValues = [
{ name: "0.5 with 4 decimals", value: 0.5, decimals: 4 },
{ name: "0.1 with 4 decimals", value: 0.1, decimals: 4 },
{ name: "1.0 with 4 decimals", value: 1.0, decimals: 4 },
{ name: "50.12 with 2 decimals", value: 50.12, decimals: 2 },
];
console.log("Current implementation (parseFloat(toFixed())):");
testValues.forEach(({name, value, decimals}) => {
const rounded = round(value, decimals);
const inTemplate = `Value: ${rounded}`;
console.log(` ${name}: ${value.toFixed(decimals)} -> ${rounded} -> "${inTemplate}"`);
});
console.log("\nProposed implementation (toFixed() directly):");
testValues.forEach(({name, value, decimals}) => {
const fixed = value.toFixed(decimals);
const inTemplate = `Value: ${fixed}`;
console.log(` ${name}: ${value.toFixed(decimals)} -> "${inTemplate}"`);
});
// Test a realistic oklch output
console.log("\nRealistic oklch output example:");
const hsl = { mode: 'hsl', h: 45, s: 100, l: 50, alpha: 1 };
const fakeOklch = { l: 0.5, c: 0.15, h: 45.67 }; // Simplified example
// Current implementation
const L_current = round(0.5, 4);
const C_current = round(0.15, 4);
const H_current = round(45.67, 2);
const oklch_current = `oklch(${L_current} ${C_current} ${H_current})`;
console.log(` Current: ${oklch_current}`);
// Proper implementation
const L_proper = (0.5).toFixed(4);
const C_proper = (0.15).toFixed(4);
const H_proper = (45.67).toFixed(2);
const oklch_proper = `oklch(${L_proper} ${C_proper} ${H_proper})`;
console.log(` Proper: ${oklch_proper}`);
EOF
node /tmp/test_format.jsRepository: raystack/apsara
Length of output: 667
formatOklch rounds values but does not preserve the stated decimal format.
The current implementation using Number.parseFloat(n.toFixed(p)) converts to a number, which loses trailing zeros when interpolated into the template literal. This violates the documented contract: "4-decimal L/C, 2-decimal H." For example, with L=0.5 and C=0.15, the output is oklch(0.5 0.15 ...) instead of the intended oklch(0.5000 0.1500 ...).
To preserve the fixed decimal width, use toFixed(p) directly, which returns a properly formatted string:
Proposed fix
-const round = (n: number, p: number) => Number.parseFloat(n.toFixed(p));
+const fixed = (n: number, p: number) => (Number.isFinite(n) ? n : 0).toFixed(p);
@@
- const L = round(oklch.l ?? 0, 4);
- const C = round(oklch.c ?? 0, 4);
- const H = C === 0 || !Number.isFinite(oklch.h) ? 0 : round(oklch.h ?? 0, 2);
+ const cRaw = oklch.c ?? 0;
+ const L = fixed(oklch.l ?? 0, 4);
+ const C = fixed(cRaw, 4);
+ const H = cRaw === 0 || !Number.isFinite(oklch.h) ? '0.00' : fixed(oklch.h ?? 0, 2);
@@
- : `oklch(${body} / ${round(hsl.alpha, 4)})`;
+ : `oklch(${body} / ${fixed(hsl.alpha, 4)})`;📝 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.
Suggested change
| const round = (n: number, p: number) => Number.parseFloat(n.toFixed(p)); | |
| /** | |
| * Serializes a culori-shaped HSL color as oklch(L C H[ / A]). | |
| * Matches the design system's token format: 4-decimal L/C, 2-decimal H, | |
| * H pinned to 0 for achromatic colors (culori would emit `none` per CSS | |
| * Color 4, which is correct but inconsistent with how tokens are written). | |
| */ | |
| const formatOklch = (hsl: { | |
| mode: 'hsl'; | |
| h: number; | |
| s: number; | |
| l: number; | |
| alpha: number; | |
| }): string => { | |
| const oklch = toOklch(hsl); | |
| if (!oklch) return ''; | |
| const L = round(oklch.l ?? 0, 4); | |
| const C = round(oklch.c ?? 0, 4); | |
| const H = C === 0 || !Number.isFinite(oklch.h) ? 0 : round(oklch.h ?? 0, 2); | |
| const body = `${L} ${C} ${H}`; | |
| return hsl.alpha === 1 | |
| ? `oklch(${body})` | |
| : `oklch(${body} / ${round(hsl.alpha, 4)})`; | |
| const fixed = (n: number, p: number) => (Number.isFinite(n) ? n : 0).toFixed(p); | |
| /** | |
| * Serializes a culori-shaped HSL color as oklch(L C H[ / A]). | |
| * Matches the design system's token format: 4-decimal L/C, 2-decimal H, | |
| * H pinned to 0 for achromatic colors (culori would emit `none` per CSS | |
| * Color 4, which is correct but inconsistent with how tokens are written). | |
| */ | |
| const formatOklch = (hsl: { | |
| mode: 'hsl'; | |
| h: number; | |
| s: number; | |
| l: number; | |
| alpha: number; | |
| }): string => { | |
| const oklch = toOklch(hsl); | |
| if (!oklch) return ''; | |
| const cRaw = oklch.c ?? 0; | |
| const L = fixed(oklch.l ?? 0, 4); | |
| const C = fixed(cRaw, 4); | |
| const H = cRaw === 0 || !Number.isFinite(oklch.h) ? '0.00' : fixed(oklch.h ?? 0, 2); | |
| const body = `${L} ${C} ${H}`; | |
| return hsl.alpha === 1 | |
| ? `oklch(${body})` | |
| : `oklch(${body} / ${fixed(hsl.alpha, 4)})`; |
🤖 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/raystack/components/color-picker/utils.ts` around lines 30 - 53,
formatOklch currently uses round (which returns a Number via
Number.parseFloat(n.toFixed(p))) causing trailing zeros to be dropped and
violating the "4-decimal L/C, 2-decimal H" contract; change the implementation
so the values are formatted as fixed-width decimal strings (use toFixed(p)
directly) and ensure formatOklch concatenates those string results (update the
round helper to return a string or replace calls to round(L/C/H/alpha) in
formatOklch with num.toFixed(p)) so L and C are 4-decimal, H is 2-decimal, and
alpha is 4-decimal when included (refer to formatOklch and the round helper).
paanSinghCoder
changed the title
Shreyag02
reviewed
May 20, 2026
Shreyag02
approved these changes
May 20, 2026
Without `as const`, `SUPPORTED_MODES` is `string[]` so `ModeType` widens to `string` — invalid modes pass type-checking and silently fall back to RGB in `getColorString`. `options` on `ColorPickerMode` is widened to `readonly ModeType[]` to accept the const tuple as its default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Shreyag02
approved these changes
May 20, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/raystack/components/color-picker/color-picker-area.tsx (1)
197-212:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame unmount cleanup issue as OklchArea.
The
HslAreacomponent has the same potential for leaked event listeners if unmounted during drag. Apply the same cleanup pattern here.🤖 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/raystack/components/color-picker/color-picker-area.tsx` around lines 197 - 212, The HslArea's drag handlers (handlePointerDown/handlePointerUp) can leak window event listeners if the component unmounts during a drag; ensure you add an unmount cleanup that removes the listeners and resets isDragging. Add a useEffect that references handlePointerMove and handlePointerUp (and isDragging via ref) and returns a cleanup function which calls window.removeEventListener('pointermove', handlePointerMove) and window.removeEventListener('pointerup', handlePointerUp) and sets isDragging.current = false so listeners added in handlePointerDown are always removed on unmount.
🧹 Nitpick comments (1)
packages/raystack/components/color-picker/color-picker-root.tsx (1)
84-93: 💤 Low valueSide effect inside setState updater.
Calling
onValueChangeinside the state updater function is a side effect that ideally should be avoided. React expects updater functions to be pure. While this works in practice, the callback timing could become unpredictable under concurrent rendering or batching scenarios.Consider calling
onValueChangeaftersetInternalColor:♻️ Suggested refactor
const setColor = useCallback<ColorPickerContextValue['setColor']>( value => { - setInternalColor(prev => { - const next = { ...prev, ...value }; - onValueChange?.(getColorString(next, mode), mode); - return next; - }); + setInternalColor(prev => ({ ...prev, ...value })); + // Compute eagerly for the callback; internalColor will update on next render + const next = { ...internalColor, ...value }; + onValueChange?.(getColorString(next, mode), mode); }, - [mode, onValueChange] + [internalColor, mode, onValueChange] );🤖 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/raystack/components/color-picker/color-picker-root.tsx` around lines 84 - 93, The state updater passed to setInternalColor currently invokes onValueChange inside the updater (in setColor), which is a side effect; instead compute the next color object outside the updater by merging the incoming value with the current internalColor state, call setInternalColor(next), then call onValueChange?.(getColorString(next, mode), mode) after the state setter; update the setColor callback to use the external internalColor variable and include internalColor and mode/onValueChange in its dependency array so you no longer perform side effects inside the setInternalColor updater.
🤖 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/raystack/components/color-picker/__tests__/utils.test.ts`:
- Around line 69-73: The test currently only asserts the output starts with
"hsl(", so change it to parse the returned string from getColorString({
...oklch, alpha: 1 }, 'hsl') and assert the numeric H, S, L components match the
original input 45, 80, 60 within a small tolerance (e.g., ±0.5 or ±1), and also
verify alpha is 1; use the existing helper functions hslToOklch and
getColorString to locate the conversion and update the expectation to compare
values not just format.
In `@packages/raystack/components/color-picker/color-picker-area.tsx`:
- Around line 104-119: The pointer listeners added in handlePointerDown
(window.addEventListener('pointermove', handlePointerMove) and
window.addEventListener('pointerup', handlePointerUp)) are not removed if the
component unmounts during a drag; add a useEffect that returns a cleanup which
removes those window listeners and resets isDragging.current = false to avoid
leaks or stale refs. Implement a useEffect that depends on handlePointerMove and
handlePointerUp (or an empty deps array if those callbacks are stable) and in
its cleanup call window.removeEventListener('pointermove', handlePointerMove)
and window.removeEventListener('pointerup', handlePointerUp) and clear
isDragging.current so handlers won't run after unmount. Ensure you reference the
existing functions handlePointerDown, handlePointerMove, handlePointerUp and the
isDragging ref when adding the cleanup.
In `@packages/raystack/components/color-picker/color-picker.module.css`:
- Around line 84-85: The background-size declaration uses inconsistent fallbacks
causing a non-square checkerboard; update the background-size line that
references --track-checker-size to provide identical fallback values for both
dimensions (e.g., var(--track-checker-size, 8px) var(--track-checker-size, 8px))
so the checker pattern stays square when --track-checker-size is undefined.
---
Outside diff comments:
In `@packages/raystack/components/color-picker/color-picker-area.tsx`:
- Around line 197-212: The HslArea's drag handlers
(handlePointerDown/handlePointerUp) can leak window event listeners if the
component unmounts during a drag; ensure you add an unmount cleanup that removes
the listeners and resets isDragging. Add a useEffect that references
handlePointerMove and handlePointerUp (and isDragging via ref) and returns a
cleanup function which calls window.removeEventListener('pointermove',
handlePointerMove) and window.removeEventListener('pointerup', handlePointerUp)
and sets isDragging.current = false so listeners added in handlePointerDown are
always removed on unmount.
---
Nitpick comments:
In `@packages/raystack/components/color-picker/color-picker-root.tsx`:
- Around line 84-93: The state updater passed to setInternalColor currently
invokes onValueChange inside the updater (in setColor), which is a side effect;
instead compute the next color object outside the updater by merging the
incoming value with the current internalColor state, call
setInternalColor(next), then call onValueChange?.(getColorString(next, mode),
mode) after the state setter; update the setColor callback to use the external
internalColor variable and include internalColor and mode/onValueChange in its
dependency array so you no longer perform side effects inside the
setInternalColor updater.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b770453a-de1b-458e-bad9-d2220be75f83
📒 Files selected for processing (10)
apps/www/src/app/examples/color-picker/page.tsxapps/www/src/content/docs/components/color-picker/index.mdxpackages/raystack/components/color-picker/__tests__/color-picker.test.tsxpackages/raystack/components/color-picker/__tests__/utils.test.tspackages/raystack/components/color-picker/color-picker-area.tsxpackages/raystack/components/color-picker/color-picker-hue.tsxpackages/raystack/components/color-picker/color-picker-input.tsxpackages/raystack/components/color-picker/color-picker-root.tsxpackages/raystack/components/color-picker/color-picker.module.csspackages/raystack/components/color-picker/utils.ts
✅ Files skipped from review due to trivial changes (1)
- apps/www/src/content/docs/components/color-picker/index.mdx
Comment on lines
+69
to
+73
| it('produces a color whose HSL serialization matches the input', () => { | ||
| const oklch = hslToOklch(45, 80, 60); | ||
| const out = getColorString({ ...oklch, alpha: 1 }, 'hsl'); | ||
| expect(out).toMatch(/^hsl\(/); | ||
| }); |
Contributor
There was a problem hiding this comment.
Strengthen this assertion to validate values, not just format.
At Line 72, the test name says serialization “matches the input,” but it only checks the hsl( prefix. This can pass even when H/S/L drift significantly.
Suggested test tightening
it('produces a color whose HSL serialization matches the input', () => {
const oklch = hslToOklch(45, 80, 60);
const out = getColorString({ ...oklch, alpha: 1 }, 'hsl');
- expect(out).toMatch(/^hsl\(/);
+ const match = out.match(/^hsl\(([\d.]+)\s+([\d.]+)%\s+([\d.]+)%\)$/);
+ expect(match).not.toBeNull();
+ const [, h, s, l] = match as RegExpMatchArray;
+ expect(Number(h)).toBeCloseTo(45, 1);
+ expect(Number(s)).toBeCloseTo(80, 1);
+ expect(Number(l)).toBeCloseTo(60, 1);
});📝 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.
Suggested change
| it('produces a color whose HSL serialization matches the input', () => { | |
| const oklch = hslToOklch(45, 80, 60); | |
| const out = getColorString({ ...oklch, alpha: 1 }, 'hsl'); | |
| expect(out).toMatch(/^hsl\(/); | |
| }); | |
| it('produces a color whose HSL serialization matches the input', () => { | |
| const oklch = hslToOklch(45, 80, 60); | |
| const out = getColorString({ ...oklch, alpha: 1 }, 'hsl'); | |
| const match = out.match(/^hsl\(([\d.]+)\s+([\d.]+)%\s+([\d.]+)%\)$/); | |
| expect(match).not.toBeNull(); | |
| const [, h, s, l] = match as RegExpMatchArray; | |
| expect(Number(h)).toBeCloseTo(45, 1); | |
| expect(Number(s)).toBeCloseTo(80, 1); | |
| expect(Number(l)).toBeCloseTo(60, 1); | |
| }); |
🤖 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/raystack/components/color-picker/__tests__/utils.test.ts` around
lines 69 - 73, The test currently only asserts the output starts with "hsl(", so
change it to parse the returned string from getColorString({ ...oklch, alpha: 1
}, 'hsl') and assert the numeric H, S, L components match the original input 45,
80, 60 within a small tolerance (e.g., ±0.5 or ±1), and also verify alpha is 1;
use the existing helper functions hslToOklch and getColorString to locate the
conversion and update the expectation to compare values not just format.
Comment on lines
+84
to
+85
| background-size: var(--track-checker-size, 8px) | ||
| var(--track-checker-size, 12px); |
Contributor
There was a problem hiding this comment.
Inconsistent fallback values in background-size.
The fallback values differ (8px vs 12px), creating a non-square checkerboard if --track-checker-size is undefined. This is likely unintentional.
♻️ Suggested fix
- background-size: var(--track-checker-size, 8px)
- var(--track-checker-size, 12px);
+ background-size: var(--track-checker-size, 12px)
+ var(--track-checker-size, 12px);📝 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.
Suggested change
| background-size: var(--track-checker-size, 8px) | |
| var(--track-checker-size, 12px); | |
| background-size: var(--track-checker-size, 12px) | |
| var(--track-checker-size, 12px); |
🤖 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/raystack/components/color-picker/color-picker.module.css` around
lines 84 - 85, The background-size declaration uses inconsistent fallbacks
causing a non-square checkerboard; update the background-size line that
references --track-checker-size to provide identical fallback values for both
dimensions (e.g., var(--track-checker-size, 8px) var(--track-checker-size, 8px))
so the checker pattern stays square when --track-checker-size is undefined.
- Updated color input to display the current color value as a read-only string, reflecting real-time changes. - Enhanced the alpha slider to show a gradient based on the current color, improving user experience. - Optimized rendering of color values using `useMemo` for better performance across components. - Adjusted hue slider to provide sub-degree precision in OKLCH mode, enhancing color selection accuracy. - Refactored color calculation logic to utilize native CSS for better compatibility with wide-gamut displays.
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/raystack/components/color-picker/color-picker-alpha.tsx`:
- Around line 19-24: The trackStyle useMemo currently builds the OKLCH string
manually and skips the rounding and achromatic hue pinning used elsewhere;
import and call the shared formatOklchString (from utils.ts) inside the
trackStyle computation (replacing the manual `oklch(${lightness} ${chroma}
${hue})` construction) so the value uses the same L/C rounding to 4 decimals, H
rounding to 2 decimals and forces H to 0 when C === 0; update the dependency
list ([lightness, chroma, hue]) remains the same and remove any local rounding
logic to keep formatting consistent across the codebase.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3a6d39d-195d-4c4f-a668-cbfebf9e8a3a
📒 Files selected for processing (7)
apps/www/src/content/docs/components/color-picker/index.mdxpackages/raystack/components/color-picker/color-picker-alpha.tsxpackages/raystack/components/color-picker/color-picker-area.tsxpackages/raystack/components/color-picker/color-picker-hue.tsxpackages/raystack/components/color-picker/color-picker-input.tsxpackages/raystack/components/color-picker/color-picker-root.tsxpackages/raystack/components/color-picker/color-picker.module.css
✅ Files skipped from review due to trivial changes (1)
- apps/www/src/content/docs/components/color-picker/index.mdx
Comment on lines
+19
to
+24
| const trackStyle = useMemo( | ||
| () => | ||
| ({ | ||
| ['--rs-color-picker-alpha-end' as string]: `oklch(${lightness} ${chroma} ${hue})` | ||
| }) as CSSProperties, | ||
| [lightness, chroma, hue] |
Contributor
There was a problem hiding this comment.
Use consistent OKLCH formatting logic.
The manual OKLCH string construction here differs from formatOklchString in utils.ts:
- Missing rounding (L/C to 4 decimals, H to 2 decimals per
formatOklchString) - Missing achromatic hue pinning (H forced to 0 when C = 0)
This creates inconsistency across the codebase. Consider extracting the rounding logic or reusing the formatting utility.
♻️ Proposed fix to align with formatOklchString rounding
First, import or define a rounding helper (assuming round is available in utils.ts):
+'use client';
+
import { Slider } from '`@base-ui/react/slider`';
import { cx } from 'class-variance-authority';
import { CSSProperties, useMemo } from 'react';
import styles from './color-picker.module.css';
import { useColorPicker } from './color-picker-root';
+import { round } from './utils';Then update the trackStyle computation:
const trackStyle = useMemo(
() =>
+ {
+ const L = round(lightness, 4);
+ const C = round(chroma, 4);
+ const H = C === 0 ? 0 : round(hue, 2);
+ return {
- ({
- ['--rs-color-picker-alpha-end' as string]: `oklch(${lightness} ${chroma} ${hue})`
+ ['--rs-color-picker-alpha-end' as string]: `oklch(${L} ${C} ${H})`
- }) as CSSProperties,
+ } as CSSProperties;
+ },
[lightness, chroma, hue]
);🤖 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/raystack/components/color-picker/color-picker-alpha.tsx` around
lines 19 - 24, The trackStyle useMemo currently builds the OKLCH string manually
and skips the rounding and achromatic hue pinning used elsewhere; import and
call the shared formatOklchString (from utils.ts) inside the trackStyle
computation (replacing the manual `oklch(${lightness} ${chroma} ${hue})`
construction) so the value uses the same L/C rounding to 4 decimals, H rounding
to 2 decimals and forces H to 0 when C === 0; update the dependency list
([lightness, chroma, hue]) remains the same and remove any local rounding logic
to keep formatting consistent across the codebase.
- Updated ColorPicker.Input to include a `copyable` prop, rendering a copy-to-clipboard button for the current color value. - Enhanced documentation to explain the new `copyable` feature and its usage in examples. - Added tests to verify the presence and functionality of the copy button in various scenarios.
- Added event listener for `pointercancel` to ensure proper cleanup and prevent stranded listeners during drag operations. - Updated cleanup logic in both OklchArea and HslArea components to remove `pointercancel` listeners, maintaining consistent state management during user interactions.
Shreyag02
reviewed
May 26, 2026
| defaultValue='oklch(0.5438 0.191 267.01)' | ||
| defaultMode='oklch' | ||
| style={{ | ||
| width: '240px', |
Contributor
There was a problem hiding this comment.
Moving only the slider values leaves the first component of the OKLCH value unchanged, making it look like the value isn't updating. Since the cursor also resets upon updating the slider, consider increasing the card's width to resolve this.
Shreyag02
approved these changes
May 26, 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.
Summary
colorforculoriin theColorPickercomponent so it can accept and emitoklch()alongside hex/rgb/hsloklchtoSUPPORTED_MODESand aformatOklch()helper (4-decimal L/C, 2-decimal H, hue pinned to 0 for grays)/examples/color-pickerWhy culori (not color-convert)
The picker's
valueprop accepts any CSS color string from consumers — hex, rgb/rgba, hsl/hsla, oklch, named colors.color-convertis route-based; it has no CSS parser and no stringifiers (except a hex helper), so we'd hand-roll ~170 lines of parsers and formatters.culoriships exactly the surface this component needs.parseconverter('hsl')c.rgb.hsl,c.hex.rgb, …)formatHex#rrggbbc.rgb.hexreturns'FF0000'(no#, no alpha)formatHex8#rrggbbaaformatHslhsl(...)/hsla(...)formatRgbrgb(...)/rgba(...)Caveat
Sliders still operate in HSL.
oklchis an I/O format, not a perceptual editing mode — anoklch()value passed in will not be bit-identical when read back, because the round-trip goes through HSL → sRGB → OKLCH.Follow-up
The VS Code plugin (#759) currently uses
color-convertbecause its input is a closed set ofoklch()tokens. We could move it toculorilater to consolidate on one color library across the monorepo — the trade-off is ~9 KB gzipped in the plugin bundle vs. removing the parser/scaling/type-shim glue we hand-rolled there.Test plan
pnpm test components/color-picker— 24/24 passing (3 new tests for oklch input, oklch mode output, and oklch alpha tail)pnpm tsc --noEmitclean on touched filespnpm biome checkclean on touched files/examples/color-pickerin the docs site and verify each card🤖 Generated with Claude Code