From e29b59c7eb227faf70fe2722eced075e4b484e5d Mon Sep 17 00:00:00 2001 From: Paco Cartones <253313177+pacocartones@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:52:51 +0200 Subject: [PATCH] fix(web): normalize CSS Color 4 token colors before defining the Monaco theme (#3453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): normalize CSS Color 4 token colors before defining Monaco theme Carapace themes write --oc-* tokens as oklch(), and some browsers serialize the computed values as lab(). applyMonacoTheme() forwarded those raw values into monaco.editor.defineTheme(), whose strict token color parser threw "Illegal value for token color" and crashed the skill Diff tab (#3440). Resolve every theme token through monacoColor() before defineTheme(): hex and rgb(a) keep working as before, lab()/lch()/oklab()/oklch() are converted to #rrggbb(aa) with the CSS Color 4 matrices, and unknown syntax falls back to theme-safe colors. Adds regression coverage that drives the component with oklch()/lab() tokens and asserts the colors handed to defineTheme(), plus unit tests for the converter checked against independently computed reference values. * fix(theme): correct SkillDiffCard editor.background expectation and gate the monaco union - The scout's test expected #262626 for oklch(0.205 0 0), whose correct oklch→hex conversion is #171717 (proven by the cssColor4ToHex unit test). - Add oxlint disable for the Monaco union (resolves to any via @monaco-editor/react types) and apply oxfmt formatting. --------- Co-authored-by: pacocartones --- src/components/SkillDiffCard.test.tsx | 117 ++++++++++++++++- src/components/SkillDiffCard.tsx | 174 +++++++++++++++++++++++--- 2 files changed, 271 insertions(+), 20 deletions(-) diff --git a/src/components/SkillDiffCard.test.tsx b/src/components/SkillDiffCard.test.tsx index 7451d2e0..4d6502d9 100644 --- a/src/components/SkillDiffCard.test.tsx +++ b/src/components/SkillDiffCard.test.tsx @@ -1,13 +1,16 @@ /* @vitest-environment jsdom */ +import type { Monaco } from "@monaco-editor/react"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { useEffect } from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Doc, Id } from "../../convex/_generated/dataModel"; -import { SkillDiffCard } from "./SkillDiffCard"; +import { SkillDiffCard, cssColor4ToHex } from "./SkillDiffCard"; const getFilePreviewMock = vi.fn(); let diffEditorMounts = 0; let diffEditorUnmounts = 0; +// oxlint-disable-next-line typescript/no-redundant-type-constituents -- Monaco resolves to any via @monaco-editor/react types +let monacoInstanceMock: Monaco | null = null; vi.mock("convex/react", () => ({ useAction: () => getFilePreviewMock, @@ -25,7 +28,7 @@ vi.mock("@monaco-editor/react", () => ({ className?: string; options?: { renderSideBySide?: boolean; useInlineViewWhenSpaceIsLimited?: boolean }; }) => , - useMonaco: () => null, + useMonaco: () => monacoInstanceMock, })); function MockDiffEditor({ @@ -91,6 +94,11 @@ describe("SkillDiffCard", () => { getFilePreviewMock.mockResolvedValue({ text: "content" }); diffEditorMounts = 0; diffEditorUnmounts = 0; + monacoInstanceMock = null; + document.documentElement.removeAttribute("data-theme-resolved"); + for (const property of ["--oc-bg-elevated", "--oc-text-primary", "--oc-text-secondary"]) { + document.documentElement.style.removeProperty(property); + } }); it("defaults to inline mode on narrow screens", async () => { @@ -185,4 +193,109 @@ describe("SkillDiffCard", () => { ).toBeTruthy(); expect(screen.queryByTestId("diff-editor")).toBeNull(); }); + + it("passes only Monaco-safe colors to defineTheme when tokens use CSS Color 4", async () => { + installMatchMedia(false); + // Sanity guard: if jsdom stops resolving custom properties through + // getComputedStyle, this test must fail loudly instead of asserting fallbacks. + document.documentElement.style.setProperty("--oc-bg-elevated", "oklch(0.205 0 0)"); + document.documentElement.style.setProperty("--oc-text-primary", "oklch(0.985 0 0)"); + document.documentElement.style.setProperty("--oc-text-secondary", "lab(98.26% 0 0)"); + expect(getComputedStyle(document.documentElement).getPropertyValue("--oc-text-primary")).toBe( + "oklch(0.985 0 0)", + ); + + const defineTheme = vi.fn(); + monacoInstanceMock = { + editor: { defineTheme, setTheme: vi.fn() }, + } as unknown as Monaco; + + render( + , + ); + + await waitFor(() => { + expect(defineTheme).toHaveBeenCalled(); + }); + const themeData = defineTheme.mock.calls[0][1] as { + rules: { foreground: string }[]; + colors: Record; + }; + for (const rule of themeData.rules) { + expect(rule.foreground).toMatch(/^#[0-9a-f]{6,8}$/i); + } + expect(themeData.rules[0].foreground).toBe("#fafafa"); + expect(themeData.rules[1].foreground).toBe("#fafafa"); + expect(themeData.colors["editor.background"]).toBe("#171717"); + expect(themeData.colors["editor.foreground"]).toBe("#fafafa"); + }); + + it("falls back to theme-safe colors when a token uses syntax Monaco cannot parse", async () => { + installMatchMedia(false); + document.documentElement.dataset.themeResolved = "dark"; + document.documentElement.style.setProperty("--oc-text-primary", "color(display-p3 1 1 1)"); + + const defineTheme = vi.fn(); + monacoInstanceMock = { + editor: { defineTheme, setTheme: vi.fn() }, + } as unknown as Monaco; + + render( + , + ); + + await waitFor(() => { + expect(defineTheme).toHaveBeenCalled(); + }); + const themeData = defineTheme.mock.calls[0][1] as { + rules: { foreground: string }[]; + }; + expect(themeData.rules[0].foreground).toBe("#fafafa"); + }); +}); + +describe("cssColor4ToHex", () => { + // Expected values computed independently from the CSS Color 4 conversion + // matrices (Ottosson's OKLab; CIE Lab with D50 white and Bradford adaptation). + it.each([ + ["oklch(0.985 0 0)", "#fafafa"], + ["oklab(0.985 0 0)", "#fafafa"], + ["lab(98.26% 0 0)", "#fafafa"], + ["lch(98.26% 0 0)", "#fafafa"], + ["lab(0% 0 0)", "#000000"], + ["oklch(1 0 0)", "#ffffff"], + ["oklch(0.7 0.15 180)", "#00bca2"], + ["oklch(0.6 0.2 30)", "#de3e2d"], + ["lab(60% 40 -30)", "#c175c7"], + ["lch(60% 50 250)", "#009ce3"], + ["oklch(0.87 0 0)", "#d4d4d4"], + ["oklch(0.205 0 0)", "#171717"], + ["lab(50% 20% -20%)", "#9168a2"], + ])("converts %s to %s", (input, expected) => { + expect(cssColor4ToHex(input)).toBe(expected); + }); + + it("keeps an alpha channel as a fourth hex pair", () => { + expect(cssColor4ToHex("oklch(0 0 0 / 0.3)")).toBe("#0000004d"); + expect(cssColor4ToHex("oklch(0 0 0 / 30%)")).toBe("#0000004d"); + }); + + it("returns null for syntaxes it does not convert", () => { + expect(cssColor4ToHex("color(display-p3 1 0 0)")).toBeNull(); + expect(cssColor4ToHex("hsl(120 50% 50%)")).toBeNull(); + expect(cssColor4ToHex("oklch(0.5 0.1 0.5turn)")).toBeNull(); + expect(cssColor4ToHex("rebeccapurple")).toBeNull(); + }); }); diff --git a/src/components/SkillDiffCard.tsx b/src/components/SkillDiffCard.tsx index 52416ca4..7aea18d3 100644 --- a/src/components/SkillDiffCard.tsx +++ b/src/components/SkillDiffCard.tsx @@ -744,28 +744,37 @@ function buildDiffOptions( function applyMonacoTheme(monaco: NonNullable>) { const styles = getComputedStyle(document.documentElement); - const surface = normalizeHex(styles.getPropertyValue("--oc-bg-elevated").trim() || "#ffffff"); - const surfaceMuted = styles.getPropertyValue("--oc-bg-surface").trim() || "#f6f1ec"; - const ink = styles.getPropertyValue("--oc-text-primary").trim() || "#1d1a17"; - const inkSoft = styles.getPropertyValue("--oc-text-secondary").trim() || "#4c463f"; - const line = styles.getPropertyValue("--oc-border-subtle").trim() || "rgba(29, 26, 23, 0.12)"; - const accent = styles.getPropertyValue("--oc-accent-primary").trim() || "#e65c46"; - const seafoam = styles.getPropertyValue("--oc-accent-secondary").trim() || "#2bc6a4"; - const diffAdded = styles.getPropertyValue("--oc-diff-added").trim() || "#9bb955"; - const diffAddedStrong = styles.getPropertyValue("--oc-diff-added-strong").trim() || seafoam; - const diffRemoved = styles.getPropertyValue("--oc-diff-removed").trim() || "#e47866"; - const diffRemovedStrong = styles.getPropertyValue("--oc-diff-removed-strong").trim() || accent; + const isDark = isDarkThemeResolved(); + const surface = monacoColor(styles.getPropertyValue("--oc-bg-elevated"), "#ffffff"); + const surfaceMuted = monacoColor(styles.getPropertyValue("--oc-bg-surface"), "#f6f1ec"); + const ink = monacoColor( + styles.getPropertyValue("--oc-text-primary"), + isDark ? "#fafafa" : "#1d1a17", + ); + const inkSoft = monacoColor( + styles.getPropertyValue("--oc-text-secondary"), + isDark ? "#d4d4d4" : "#4c463f", + ); + const line = monacoColor(styles.getPropertyValue("--oc-border-subtle"), "rgba(29, 26, 23, 0.12)"); + const accent = monacoColor(styles.getPropertyValue("--oc-accent-primary"), "#e65c46"); + const seafoam = monacoColor(styles.getPropertyValue("--oc-accent-secondary"), "#2bc6a4"); + const diffAdded = monacoColor(styles.getPropertyValue("--oc-diff-added"), "#9bb955"); + const diffAddedStrong = monacoColor(styles.getPropertyValue("--oc-diff-added-strong"), seafoam); + const diffRemoved = monacoColor(styles.getPropertyValue("--oc-diff-removed"), "#e47866"); + const diffRemovedStrong = monacoColor( + styles.getPropertyValue("--oc-diff-removed-strong"), + accent, + ); const diffDiagonal = toMonacoColor( - styles.getPropertyValue("--diff-diagonal").trim() || "#22222233", + monacoColor(styles.getPropertyValue("--diff-diagonal"), "#22222233"), + ); + const diffBorder = toMonacoColor(monacoColor(styles.getPropertyValue("--diff-border"), line)); + const lineNumber = monacoColor( + styles.getPropertyValue("--diff-line-number").trim() || styles.getPropertyValue("--ink-soft"), + "#4c463f", ); - const diffBorder = toMonacoColor(styles.getPropertyValue("--diff-border").trim() || line); - const lineNumber = - styles.getPropertyValue("--diff-line-number").trim() || - styles.getPropertyValue("--ink-soft").trim() || - "#4c463f"; const background = surface; const gutter = surfaceMuted; - const isDark = isDarkThemeResolved(); const base = isDark ? "vs-dark" : "vs"; const diffInserted = withAlpha(diffAdded, isDark ? 0.26 : 0.14); @@ -861,3 +870,132 @@ function toMonacoColor(color: string) { .map((value) => Math.round(value).toString(16).padStart(2, "0")) .join("")}${channel}`; } + +// Monaco's theme parsers reject CSS Color 4 functions such as oklch() and lab(), +// which the Carapace themes write into the --oc-* tokens this component reads +// (#3440). monacoColor resolves a token to a Monaco-safe #hex color and swaps in +// the fallback when the syntax is unknown, so defineTheme() never sees a value +// it cannot parse. +function monacoColor(rawValue: string, fallback: string): string { + const value = rawValue.trim(); + if (!value) return fallback; + const hex = normalizeHex(value); + if (hex.startsWith("#")) return hex; + const rgbHex = toMonacoColor(value); + if (rgbHex.startsWith("#")) return rgbHex; + return cssColor4ToHex(value) ?? fallback; +} + +type LinearSrgb = { r: number; g: number; b: number }; + +// Converts a lab(), lch(), oklab() or oklch() color to #rrggbb (or #rrggbbaa +// when it carries an alpha). Returns null for anything else so callers can +// fall back instead of forwarding an unparseable value to Monaco. +export function cssColor4ToHex(value: string): string | null { + const match = /^(ok)?(lab|lch)\(([^)]+)\)$/i.exec(value.trim()); + if (!match) return null; + const isOk = Boolean(match[1]); + const isPolar = match[2].toLowerCase() === "lch"; + const [componentsRaw, alphaRaw] = match[3].split("/"); + const parts = componentsRaw.trim().split(/\s+/); + if (parts.length !== 3) return null; + + // Percentages resolve per CSS Color 4: lightness against 100 (lab) or 1 + // (oklab), a/b against 125 (lab) or 0.4 (oklab), chroma against 150 (lch) + // or 0.4 (oklch). + const lightness = parseColor4Component(parts[0], isOk ? 1 : 100); + const second = parseColor4Component(parts[1], isPolar ? (isOk ? 0.4 : 150) : isOk ? 0.4 : 125); + if (lightness === null || second === null) return null; + + let a: number; + let b: number; + if (isPolar) { + const hue = parseHue(parts[2]); + if (hue === null) return null; + a = second * Math.cos((hue * Math.PI) / 180); + b = second * Math.sin((hue * Math.PI) / 180); + } else { + const third = parseColor4Component(parts[2], isOk ? 0.4 : 125); + if (third === null) return null; + a = second; + b = third; + } + + let alpha = 1; + if (alphaRaw !== undefined) { + const parsedAlpha = parseColor4Component(alphaRaw, 1); + if (parsedAlpha === null) return null; + alpha = Math.min(1, Math.max(0, parsedAlpha)); + } + + const rgb = isOk ? oklabToLinearSrgb(lightness, a, b) : labToLinearSrgb(lightness, a, b); + const hex = `#${linearToSrgbHex(rgb.r)}${linearToSrgbHex(rgb.g)}${linearToSrgbHex(rgb.b)}`; + if (alpha >= 1) return hex; + return `${hex}${Math.round(alpha * 255) + .toString(16) + .padStart(2, "0")}`; +} + +function parseColor4Component(raw: string, percentScale: number): number | null { + const value = raw.trim(); + if (value === "none") return 0; + if (value.endsWith("%")) { + const parsed = Number.parseFloat(value.slice(0, -1)); + return Number.isNaN(parsed) ? null : (parsed / 100) * percentScale; + } + const parsed = Number.parseFloat(value); + return Number.isNaN(parsed) ? null : parsed; +} + +function parseHue(raw: string): number | null { + const value = raw.trim(); + if (value === "none") return 0; + // Only plain degrees are resolved; turn/rad/grad fall back rather than + // converting with the wrong unit. + if (/[a-z%]/i.test(value) && !/deg$/i.test(value)) return null; + const parsed = Number.parseFloat(value); + return Number.isNaN(parsed) ? null : parsed; +} + +function oklabToLinearSrgb(lightness: number, a: number, b: number): LinearSrgb { + const lPrime = lightness + 0.3963377774 * a + 0.2158037573 * b; + const mPrime = lightness - 0.1055613458 * a - 0.0638541729 * b; + const sPrime = lightness - 0.0894841775 * a - 1.291485548 * b; + const l = lPrime ** 3; + const m = mPrime ** 3; + const s = sPrime ** 3; + return { + r: 4.0767416621 * l - 3.3077115913 * m + 0.2309699292 * s, + g: -1.2684380046 * l + 2.6097574011 * m - 0.3413193965 * s, + b: -0.0041960863 * l - 0.7034186149 * m + 1.707614701 * s, + }; +} + +function labToLinearSrgb(lightness: number, a: number, b: number): LinearSrgb { + const fy = (lightness + 16) / 116; + const fx = fy + a / 500; + const fz = fy - b / 200; + const delta = 6 / 29; + const toXyz = (f: number) => (f > delta ? f ** 3 : 3 * delta * delta * (f - 4 / 29)); + // Lab is defined against the D50 white point. + const x = toXyz(fx) * 0.96422; + const y = toXyz(fy); + const z = toXyz(fz) * 0.82521; + // Bradford chromatic adaptation from D50 to sRGB's D65. + const xd = 0.9555766 * x - 0.0230393 * y + 0.0631636 * z; + const yd = -0.0282895 * x + 1.0099416 * y + 0.0210077 * z; + const zd = 0.0122982 * x - 0.020483 * y + 1.3299098 * z; + return { + r: 3.2404542 * xd - 1.5371385 * yd - 0.4985314 * zd, + g: -0.969266 * xd + 1.8760108 * yd + 0.041556 * zd, + b: 0.0556434 * xd - 0.2040259 * yd + 1.0572252 * zd, + }; +} + +function linearToSrgbHex(channel: number): string { + const clamped = Math.min(1, Math.max(0, channel)); + const srgb = clamped <= 0.0031308 ? 12.92 * clamped : 1.055 * clamped ** (1 / 2.4) - 0.055; + return Math.round(srgb * 255) + .toString(16) + .padStart(2, "0"); +}