Skip to content

UI: Rework Monaco editor theme to match Chakra UI palette#64748

Open
shivaam wants to merge 4 commits intoapache:mainfrom
shivaam:fix/monaco-theme-64253
Open

UI: Rework Monaco editor theme to match Chakra UI palette#64748
shivaam wants to merge 4 commits intoapache:mainfrom
shivaam:fix/monaco-theme-64253

Conversation

@shivaam
Copy link
Copy Markdown
Contributor

@shivaam shivaam commented Apr 5, 2026

Summary

Register custom airflow-light / airflow-dark Monaco themes derived from Chakra UI color tokens so that every Monaco editor in the UI (DAG Code viewer, code diff viewer, JSON editor, RenderedJsonField) visually integrates with the rest of the app instead of using Monaco's default vs / vs-dark themes.

closes: #64253

Open Questions

  1. Design question: I added a module-level themesRegistered flag so defineTheme
    only runs on the first beforeMount. Without it, pages with many editors (e.g.
    XCom table with ~50 rows) re-register themes on every mount. Measured cost without the guard: ~4ms on a 50-editor page. Negligible. Happy to drop the flag for simpler code + tests if you prefer..
  2. Design question: Chakra v3 uses OKLCH, Monaco's defineTheme only accepts
    #rrggbb. I convert by rasterizing a 1x1 canvas pixel and reading back sRGB
    via getImageData. I checked ctx.fillStyle readback — it preserves the OKLCH string unchanged on Chrome 111+, so that path silently fails (this is what PR Rework Monaco editor style to match Chakra UI color palette  #64268 does).

Implementation notes

  • New hook useMonacoTheme (src/context/colorMode/useMonacoTheme.ts) — registers both custom themes exactly once via a module-level flag, returns a stable beforeMount callback and the correct theme name for the current color mode.
  • inherit: true + empty rules — Monaco's default syntax highlighting (keywords, strings, comments) is preserved. Only editor shell colors (background, foreground, gutter, selection, scrollbar, line numbers) are overridden.
  • OKLCH → hex conversion via getImageData — Chakra v3 uses OKLCH colors. ctx.fillStyle readback cannot be relied on because Chrome 111+ preserves the OKLCH string instead of converting to hex, which Monaco would silently ignore.
  • Single registration — themes are registered once per session via a module-level flag so pages with many editor instances (e.g. XCom table with many RenderedJsonField rows) don't pay the cost on every mount.
  • All four Monaco usages (JsonEditor, RenderedJsonField, Code, CodeDiffViewer) are updated to use the hook. No changes to component structure, dynamic height behavior, or any other behavior beyond theme wiring.

How to test

  1. breeze start-airflow --dev-mode --load-example-dags
  2. Visit the DAG Code tab, XCom page, Connections > Edit > Extra field, Trigger DAG dialog, DAG Details page
  3. Toggle between light and dark mode — editor shell colors should smoothly match the surrounding Chakra UI surfaces instead of the default VS Code palette
white_color_dag_code dag_run dag_code_diff Screenshot 2026-04-05 at 12 16 12 PM audit_log Screenshot 2026-04-05 at 12 21 04 PM xcom
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.6)

Generated-by: Claude Code (Opus 4.6) following the guidelines

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Apr 5, 2026
Copy link
Copy Markdown
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one nit, but directions looks good to me.

Let me know when this is out of draft (fix static checks, CI more generally) and it's ready for a more in depth review + local testing.

@shivaam shivaam marked this pull request as ready for review April 8, 2026 06:41
@shivaam
Copy link
Copy Markdown
Contributor Author

shivaam commented Apr 8, 2026

Just one nit, but directions looks good to me.

Let me know when this is out of draft (fix static checks, CI more generally) and it's ready for a more in depth review + local testing.

@pierrejeambrun handled the nits and the PR is ready for local testing

Copy link
Copy Markdown
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested locally and working as expected.

IT's also nicely inheriting custom theming:
Overriding the brand color palette (see text selection)
Image

Someone overriding the 'grey' color palette will have something like:

Image

vs

Image

@pierrejeambrun pierrejeambrun added this to the Airflow 3.2.1 milestone Apr 10, 2026
@pierrejeambrun pierrejeambrun added the backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch label Apr 10, 2026
@kaxil kaxil requested a review from Copilot April 10, 2026 19:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Registers custom airflow-light / airflow-dark Monaco themes derived from Chakra UI tokens, and wires them into all Monaco editor usages so editor chrome matches the app’s design system.

Changes:

  • Add useMonacoTheme hook that defines and selects the Monaco theme based on current Chakra color mode.
  • Update all Monaco editor call sites (Editor + DiffEditor) to use the hook’s beforeMount + theme.
  • Add unit tests covering theme selection and one-time registration behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
airflow-core/src/airflow/ui/src/context/colorMode/useMonacoTheme.ts Defines custom Monaco themes using Chakra CSS variables and exposes beforeMount + theme.
airflow-core/src/airflow/ui/src/context/colorMode/useMonacoTheme.test.ts Tests theme name selection and single registration behavior.
airflow-core/src/airflow/ui/src/context/colorMode/index.ts Exports useMonacoTheme from the colorMode barrel.
airflow-core/src/airflow/ui/src/pages/Dag/Code/Code.tsx Uses useMonacoTheme for the DAG code viewer Monaco editor.
airflow-core/src/airflow/ui/src/pages/Dag/Code/CodeDiffViewer.tsx Uses useMonacoTheme for the DAG code diff Monaco editor.
airflow-core/src/airflow/ui/src/components/JsonEditor.tsx Uses useMonacoTheme for the JSON editor component.
airflow-core/src/airflow/ui/src/components/RenderedJsonField.tsx Uses useMonacoTheme for read-only rendered JSON Monaco editor instances.

shivaam added 3 commits April 11, 2026 05:23
Register custom airflow-light and airflow-dark Monaco themes derived
from Chakra UI color tokens so that the DAG Code viewer, diff viewer,
and JSON editors visually integrate with the rest of the app instead
of using Monaco's default vs/vs-dark themes.

The new useMonacoTheme hook rasterizes a single pixel through a 2D
canvas and reads it back via getImageData to convert Chakra's OKLCH
color values into the #rrggbb strings that Monaco's defineTheme
accepts — ctx.fillStyle readback cannot be used because modern
Chrome preserves the original OKLCH string. Themes are registered
once via a module-level flag and then passed to every Monaco editor
via the beforeMount callback.

closes: apache#64253
Address review feedback: React Compiler handles memoization,
so the useCallback wrapper around defineAirflowMonacoThemes
is redundant. Pass the function reference directly instead.
Also fix prettier formatting in tests.
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.
@shivaam shivaam force-pushed the fix/monaco-theme-64253 branch from 061ca08 to 7dba46a Compare April 11, 2026 13:21
@shivaam
Copy link
Copy Markdown
Contributor Author

shivaam commented Apr 11, 2026

Pushed 7dba46a addressing review feedback:

Canvas → culori for color conversion (@pierrejeambrun)
afterEach(vi.restoreAllMocks()) to prevent mock leak (@Copilot)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers. backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rework style for the monaco-editor

3 participants