Skip to content

feat(core): support oklch() colors in the color parser - #3

Open
SiTaggart wants to merge 1 commit into
TheSisb:mainfrom
SiTaggart:feat/oklch-color-support
Open

feat(core): support oklch() colors in the color parser#3
SiTaggart wants to merge 1 commit into
TheSisb:mainfrom
SiTaggart:feat/oklch-color-support

Conversation

@SiTaggart

Copy link
Copy Markdown

Summary

Theme colors, themeOverrides, and cell colors can now be written as oklch() strings. Before, any oklch color that passed through a blend, alpha, luminance, or hover-interpolation path came out as rgba(NaN, …), which the canvas silently ignores — cells kept stale fills, selection tints and row stripes vanished, and text-contrast checks broke. Colors handed straight to ctx.fillStyle already worked; only the parsed paths were broken.

Root cause: parseToRgba asks the browser to compute the color, then comma-splits the result assuming legacy rgb(r, g, b) serialization. CSS Color 4 colors keep their functional form (oklch(0.7 0.15 150), space-separated), so the split produced [0.7, 1].

What changed

  • parseToRgba is now a short pipeline: literal oklch() is parsed directly (oklch → oklab → linear sRGB, the Ottosson / CSS Color 4 matrices); anything else goes through the browser, dispatching on the computed serialization. Browser-resolved oklch (e.g. from a CSS variable) uses the same conversion. Other unsupported modern forms (lab(), color()) return the standard invalid-color black with a dev warning instead of garbage.
  • rgb / hex / hsl / named behavior is unchanged; the original split lives on verbatim as parseLegacyRgb.
  • Per-color cache is unchanged, so there is no render-path cost.
  • Nothing downstream needed touching: blend, withAlpha, interpolateColors, getLuminance, theme merging, and the cells package all work with oklch inputs.

Design notes

  • The literal fast path exists because jsdom cannot resolve oklch through getComputedStyle, and it skips DOM work for the common case.
  • Blending stays in sRGB space, same as today for rgb inputs.
  • Out-of-gamut channels are clamped; browsers gamut-map by reducing chroma, so far-out-of-gamut colors could differ slightly. None of the tested cases did, including a saturated blue well outside sRGB.
  • Considered replacing the whole parser with a 1×1 canvas readback (handles every CSS color in ~12 lines). Rejected: vitest-canvas-mock returns zeroed pixels, and premultiplied-alpha readback changes existing low-alpha blends such as borderColor.

Verification

  • Unit tests assert byte-exact equality against Chromium 146 canvas pixel readback for 7 oklch colors, plus alpha / percentage / deg forms, invalid input, a hex DOM-path regression, and each drawing helper with oklch inputs.
  • New story Tests/TestCases → OklchColors renders two grids from the same data, one entirely oklch and one in the sRGB equivalents. Canvas byte-diff between them: max channel delta 1 (0.4% of bytes) with this change; 37% of bytes differing, deltas up to 241, with the old parser.
  • Story checked manually in each browser.
  • Full core suite matches main: the same 4 tests fail there (data-editor-input additive blending ×2, data-editor row-marker drag, use-column-sizer last row), and the Build workflow is already red on main at the npm run test -- --coverage step. Nothing color-related.

No dist/ in this PR; build on main at release as usual.

🤖 Generated with Claude Code

parseToRgba assumed browsers serialize computed colors as legacy
"rgb(r, g, b)". CSS Color 4 colors keep their functional form, so
oklch() strings were mangled into NaN and every blend, alpha, luminance
and interpolation path silently failed. Parse oklch() literals directly
(oklch -> oklab -> linear sRGB), route browser-resolved oklch through the
same conversion, and warn on other unsupported modern forms instead of
producing garbage.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant