Skip to content

Commit 7dba46a

Browse files
committed
UI: Use culori for Monaco theme color conversion
Address review feedback: the canvas-based cssVarToHex was brittle (depended on Canvas2D rendering and browser-specific fillStyle behavior, and required canvas mocking in happy-dom tests). Replace it with culori's parse/formatHex, which handles OKLCH and other modern color spaces directly with no DOM rasterization. Also add afterEach(vi.restoreAllMocks()) to the tests so the getComputedStyle spy does not leak between runs.
1 parent eff86d1 commit 7dba46a

File tree

4 files changed

+39
-47
lines changed

4 files changed

+39
-47
lines changed

airflow-core/src/airflow/ui/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"chart.js": "^4.5.1",
4444
"chartjs-adapter-dayjs-4": "^1.0.4",
4545
"chartjs-plugin-annotation": "^3.1.0",
46+
"culori": "^4.0.2",
4647
"dayjs": "^1.11.19",
4748
"elkjs": "^0.11.1",
4849
"html-to-image": "^1.11.13",
@@ -78,6 +79,7 @@
7879
"@testing-library/jest-dom": "^6.9.1",
7980
"@testing-library/react": "^16.3.2",
8081
"@trivago/prettier-plugin-sort-imports": "^4.3.0",
82+
"@types/culori": "^4.0.1",
8183
"@types/node": "^24.10.1",
8284
"@types/react": "^19.2.14",
8385
"@types/react-dom": "^19.2.3",

airflow-core/src/airflow/ui/pnpm-lock.yaml

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

airflow-core/src/airflow/ui/src/context/colorMode/useMonacoTheme.test.ts

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919
import type { Monaco } from "@monaco-editor/react";
2020
import { renderHook } from "@testing-library/react";
21-
import { beforeEach, describe, expect, it, vi } from "vitest";
21+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2222

2323
// `useColorMode` is the only dependency of the hook we want to test. We mock
2424
// it with a mutable return so individual tests can drive light/dark behaviour.
@@ -42,29 +42,25 @@ const createFakeMonaco = () => {
4242
return { defineTheme, monaco: { editor: { defineTheme } } as unknown as Monaco };
4343
};
4444

45-
// happy-dom does not implement Canvas2D rendering, so the hook's `getContext`
46-
// call would return null and short-circuit before registering themes. We stub
47-
// it with the minimal surface the hook needs: the draw/clear methods and a
48-
// `getImageData` that returns arbitrary RGB bytes (the exact hex values are
49-
// irrelevant to these tests — we only care that theme registration happens).
50-
const stubCanvasContext = () => {
51-
const fakeContext = {
52-
clearRect: vi.fn(),
53-
fillRect: vi.fn(),
54-
fillStyle: "",
55-
getImageData: vi.fn(() => ({ data: new Uint8ClampedArray([0, 0, 0, 255]) })),
56-
};
57-
58-
vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue(
59-
fakeContext as unknown as CanvasRenderingContext2D,
60-
);
45+
// happy-dom does not resolve Chakra's CSS custom properties, so the hook's
46+
// `getPropertyValue` calls would return empty strings. Stub it to return a
47+
// parseable value — culori accepts plain hex, so the exact string doesn't
48+
// matter as long as it's a valid CSS color the parser recognizes.
49+
const stubComputedStyle = () => {
50+
vi.spyOn(globalThis, "getComputedStyle").mockReturnValue({
51+
getPropertyValue: () => "#abcdef",
52+
} as unknown as CSSStyleDeclaration);
6153
};
6254

6355
describe("useMonacoTheme", () => {
6456
beforeEach(() => {
6557
vi.resetModules();
6658
colorModeMock.mockReturnValue({ colorMode: "light" });
67-
stubCanvasContext();
59+
stubComputedStyle();
60+
});
61+
62+
afterEach(() => {
63+
vi.restoreAllMocks();
6864
});
6965

7066
it("returns the airflow-light theme name when color mode is light", async () => {

airflow-core/src/airflow/ui/src/context/colorMode/useMonacoTheme.ts

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* under the License.
1818
*/
1919
import type { Monaco } from "@monaco-editor/react";
20+
import { formatHex, parse } from "culori";
2021

2122
import { useColorMode } from "./useColorMode";
2223

@@ -26,44 +27,20 @@ const DARK_THEME_NAME = "airflow-dark";
2627
let themesRegistered = false;
2728

2829
// Convert any CSS color (including modern color spaces like OKLCH that Chakra
29-
// UI uses) to a #rrggbb string that Monaco's `defineTheme` accepts.
30-
//
31-
// We rasterize a single pixel and read back its sRGB bytes via `getImageData`.
32-
// `ctx.fillStyle` readback does NOT work for this: starting in Chrome 111, it
33-
// preserves the original OKLCH string instead of converting to hex, which
34-
// Monaco would silently ignore.
35-
const cssVarToHex = (ctx: CanvasRenderingContext2D, cssVar: string): string => {
30+
// UI uses) to a #rrggbb string that Monaco's `defineTheme` accepts. culori
31+
// handles parsing and gamut mapping; we fall back to black for unset or
32+
// unparseable values so Monaco never sees an invalid color.
33+
const toHex = (cssVar: string): string => {
3634
const value = getComputedStyle(document.documentElement).getPropertyValue(cssVar).trim();
3735

38-
if (value === "") {
39-
return "#000000";
40-
}
41-
42-
ctx.fillStyle = value;
43-
ctx.clearRect(0, 0, 1, 1);
44-
ctx.fillRect(0, 0, 1, 1);
45-
const [red, green, blue] = ctx.getImageData(0, 0, 1, 1).data;
46-
47-
return `#${[red, green, blue].map((channel) => (channel ?? 0).toString(16).padStart(2, "0")).join("")}`;
36+
return formatHex(parse(value)) ?? "#000000";
4837
};
4938

5039
const defineAirflowMonacoThemes = (monaco: Monaco) => {
5140
if (themesRegistered) {
5241
return;
5342
}
5443

55-
const canvas = document.createElement("canvas");
56-
57-
canvas.width = 1;
58-
canvas.height = 1;
59-
const ctx = canvas.getContext("2d");
60-
61-
if (!ctx) {
62-
return;
63-
}
64-
65-
const toHex = (cssVar: string) => cssVarToHex(ctx, cssVar);
66-
6744
monaco.editor.defineTheme(LIGHT_THEME_NAME, {
6845
base: "vs",
6946
colors: {

0 commit comments

Comments
 (0)