UI: Fix Expand/Collapse All on XComs and Audit Log JSON cells#67316
Conversation
The Expand/Collapse All toggle on the XComs and Audit Log pages was no-op: clicking the buttons updated the toggle state but the JSON content in each row stayed in its initial fold state. `RenderedJsonField` runs Monaco's `editor.foldAll` action in `handleMount` based on the initial `collapsed` prop, but never reacts to subsequent changes to that prop. Save a ref to the editor instance and add a `useEffect` that runs `editor.foldAll`/`editor.unfoldAll` whenever `collapsed` changes after mount.
10c36b1 to
e902c5d
Compare
… exist The previous useEffect in RenderedJsonField calls `editor.foldAll` / `editor.unfoldAll` to react to `collapsed` prop changes, but apache#66647 switched Monaco from the CDN bundle to a local `editor.api` import that does not register any editor contributions. As a result those actions do not exist, `editor.getAction(...)` returns null, and the toggle does nothing. Side-effect-import the folding contribution alongside `editor.api` to register the FoldingController and its `editor.foldAll` / `editor.unfoldAll` actions — the minimum needed for the expand/collapse toggle to work. Other contributions are intentionally left out to keep this change focused on the bug.
`editor.foldAll` awaits the FoldingModel, which in turn waits for fold ranges from the JSON syntax provider. When the JSON worker is unavailable (common in proxied dev setups where Vite's worker URLs do not resolve) that promise can stay pending forever, leaving `isReady` false and the parent Flex wrapper collapsed to `height: 0px; overflow: hidden` — which makes the editor completely invisible. Fire the foldAll action and set `isReady` immediately rather than gating it on the promise. The folded layout still settles via `onDidContentSizeChange` once Monaco computes it; this just removes the hard dependency on a promise that may never resolve.
In dev mode the SPA shell is served by the airflow api-server on a different origin than Vite. Without `server.origin`, Vite emits asset URLs as absolute paths (e.g. `/node_modules/.../json.worker.js?worker_file`) which the browser resolves against the page origin — the api-server, which serves the SPA fallback HTML for unknown paths. The Monaco workers therefore fail to load (text/html MIME), and the JSON fold-range provider's promise stays pending, which is what made the editor stay unfolded on collapse in dev mode. Pinning `server.origin` to localhost:5173 (matching the `--strictPort` on the dev script) makes Vite emit fully-qualified URLs so workers load from Vite directly, regardless of the page origin.
…mise" This reverts commit 8c2439f.
The Monaco folding contribution renders the per-line expand/collapse indicator (the `>` arrow next to a foldable region) by placing a `codicon codicon-folding-collapsed` / `codicon-folding-expanded` span in the gutter. Without the codicon CSS (which defines the @font-face and maps the class names to glyph characters) the span renders as an empty square. The CDN bundle pulled in `codiconStyles` transitively via `editor.all`; the local ESM `editor.api` entry point does not. Import it explicitly alongside the folding contribution.
Screen.Recording.2026-05-22.at.18.24.42.mov |
Importing `codiconStyles` (which transitively imports `codicon.css`)
broke the production build through `vite-plugin-css-injected-by-js`:
- the inlined `@font-face { src: url(./codicon.ttf) }` rule resolved
the relative URL against the page origin (the airflow api-server)
instead of `/static/assets/`, so the font fetch fell back to the SPA
HTML and Chrome reported 'OTS parsing error';
- the plugin removes the emitted CSS chunk but the JS bundle still
contained a phantom `modulepreload` for it, which 404'd and rejected
the `configureMonaco` promise — `isReady` never flipped and the
editor never mounted.
Inject the @font-face + base `.codicon` class ourselves, using Vite's
`?url` import to resolve the font asset path. Monaco still registers
the per-icon `::before` content rules at runtime via its theme service,
so that is the only piece of the static codicon CSS we need.
This reverts commit 9b6fc3f.
Rather than hand-injecting an @font-face with a manually resolved URL, exclude the codicon CSS from vite-plugin-css-injected-by-js. The plugin inlining the stylesheet was what broke the relative font URL in the first place (the CSS ended up inside a <style> tag, where url(./codicon.ttf) gets resolved against the page origin instead of the asset directory). With cssAssetsFilterFunction skipping codicon CSS, Monaco's stylesheet stays as a normal file at /static/assets/, the font URL resolves naturally next to it, and the standard codiconStyles import is enough to get the gutter glyph rendered.
choo121600
left a comment
There was a problem hiding this comment.
I’ve also taken a look at this, and overall it looks good to me.
That said, there seems to be a minor issue with noisy error logs in the console.
network.js:225 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'toUrl')
at FileAccessImpl.toUri (network.js:225:40)
at FileAccessImpl.asBrowserUri (network.js:174:26)
at editorSimpleWorker.js:322:56
at new Promise (<anonymous>)
at EditorSimpleWorker.$loadForeignModule (editorSimpleWorker.js:313:16)
at standaloneWebWorker.js:44:30
|
Yep I created an issue here: That's only happening in 'dev mode' and was already here before this PR. It's just that this fix makes it visible, we want to fix in in a follow up work. |
|
Cool :) |
|
Unrelated CI failures, merging since it's blocking for 3.2.2rc1. playright install keeps failing. |
Backport successfully created: v3-2-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
This looks similar to the issue reported here: We may need to consider bumping the Playwright version to 1.60.0. |
…ells (#67316) (#67361) * UI: Fix Expand/Collapse All on XComs and Audit Log JSON cells The Expand/Collapse All toggle on the XComs and Audit Log pages was no-op: clicking the buttons updated the toggle state but the JSON content in each row stayed in its initial fold state. `RenderedJsonField` runs Monaco's `editor.foldAll` action in `handleMount` based on the initial `collapsed` prop, but never reacts to subsequent changes to that prop. Save a ref to the editor instance and add a `useEffect` that runs `editor.foldAll`/`editor.unfoldAll` whenever `collapsed` changes after mount. * UI: Register Monaco folding contribution so foldAll/unfoldAll actions exist The previous useEffect in RenderedJsonField calls `editor.foldAll` / `editor.unfoldAll` to react to `collapsed` prop changes, but #66647 switched Monaco from the CDN bundle to a local `editor.api` import that does not register any editor contributions. As a result those actions do not exist, `editor.getAction(...)` returns null, and the toggle does nothing. Side-effect-import the folding contribution alongside `editor.api` to register the FoldingController and its `editor.foldAll` / `editor.unfoldAll` actions — the minimum needed for the expand/collapse toggle to work. Other contributions are intentionally left out to keep this change focused on the bug. * UI: Unblock RenderedJsonField visibility from the foldAll promise `editor.foldAll` awaits the FoldingModel, which in turn waits for fold ranges from the JSON syntax provider. When the JSON worker is unavailable (common in proxied dev setups where Vite's worker URLs do not resolve) that promise can stay pending forever, leaving `isReady` false and the parent Flex wrapper collapsed to `height: 0px; overflow: hidden` — which makes the editor completely invisible. Fire the foldAll action and set `isReady` immediately rather than gating it on the promise. The folded layout still settles via `onDidContentSizeChange` once Monaco computes it; this just removes the hard dependency on a promise that may never resolve. * UI: Set Vite server.origin so dev-mode worker URLs are absolute In dev mode the SPA shell is served by the airflow api-server on a different origin than Vite. Without `server.origin`, Vite emits asset URLs as absolute paths (e.g. `/node_modules/.../json.worker.js?worker_file`) which the browser resolves against the page origin — the api-server, which serves the SPA fallback HTML for unknown paths. The Monaco workers therefore fail to load (text/html MIME), and the JSON fold-range provider's promise stays pending, which is what made the editor stay unfolded on collapse in dev mode. Pinning `server.origin` to localhost:5173 (matching the `--strictPort` on the dev script) makes Vite emit fully-qualified URLs so workers load from Vite directly, regardless of the page origin. * Revert "UI: Unblock RenderedJsonField visibility from the foldAll promise" This reverts commit 8c2439f. * UI: Load codicon styles so the fold gutter glyph renders The Monaco folding contribution renders the per-line expand/collapse indicator (the `>` arrow next to a foldable region) by placing a `codicon codicon-folding-collapsed` / `codicon-folding-expanded` span in the gutter. Without the codicon CSS (which defines the @font-face and maps the class names to glyph characters) the span renders as an empty square. The CDN bundle pulled in `codiconStyles` transitively via `editor.all`; the local ESM `editor.api` entry point does not. Import it explicitly alongside the folding contribution. * UI: Inject codicon font face with a Vite-resolved URL Importing `codiconStyles` (which transitively imports `codicon.css`) broke the production build through `vite-plugin-css-injected-by-js`: - the inlined `@font-face { src: url(./codicon.ttf) }` rule resolved the relative URL against the page origin (the airflow api-server) instead of `/static/assets/`, so the font fetch fell back to the SPA HTML and Chrome reported 'OTS parsing error'; - the plugin removes the emitted CSS chunk but the JS bundle still contained a phantom `modulepreload` for it, which 404'd and rejected the `configureMonaco` promise — `isReady` never flipped and the editor never mounted. Inject the @font-face + base `.codicon` class ourselves, using Vite's `?url` import to resolve the font asset path. Monaco still registers the per-icon `::before` content rules at runtime via its theme service, so that is the only piece of the static codicon CSS we need. * Revert "UI: Inject codicon font face with a Vite-resolved URL" This reverts commit 9b6fc3f. * UI: Keep Monaco codicon CSS as an emitted file Rather than hand-injecting an @font-face with a manually resolved URL, exclude the codicon CSS from vite-plugin-css-injected-by-js. The plugin inlining the stylesheet was what broke the relative font URL in the first place (the CSS ended up inside a <style> tag, where url(./codicon.ttf) gets resolved against the page origin instead of the asset directory). With cssAssetsFilterFunction skipping codicon CSS, Monaco's stylesheet stays as a normal file at /static/assets/, the font URL resolves naturally next to it, and the standard codiconStyles import is enough to get the gutter glyph rendered. (cherry picked from commit 83d8f40) Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com> Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
closes: #67310
The Expand/Collapse All toggle on the XComs and Audit Log pages was a
no-op: clicking the buttons updated the toggle state in the header but
the JSON content in each row stayed in its initial fold state.
Four layered issues had to be fixed for both prod and dev mode:
RenderedJsonFieldonly folded once at mount. The originalhandleMountcallededitor.foldAllbased on the initialcollapsedprop, but never reacted to prop changes. Save the editor instance in
a ref and add a
useEffectthat runseditor.foldAll/editor.unfoldAllwhenevercollapsedchanges after mount.The
editor.foldAllaction wasn't registered. UI: Use local Monaco editor module instead of CDN #66647 switchedMonaco from the CDN bundle (which auto-loads all editor contributions)
to a local
editor.apiimport — which is API-only. Side-effect-importthe folding contribution so the FoldingController and its
foldAll/unfoldAllactions are actually available.foldAll's promise can stay pending.editor.foldAllawaitsthe FoldingModel, which waits for the JSON syntax range provider's
worker. Fire the action and set
isReadyimmediately rather thangating visibility on a promise that might never resolve.
Dev-mode worker URLs resolved against the wrong origin. When the
dev SPA shell is served by the airflow api-server (different origin
from Vite), Vite emitted absolute-path worker URLs that the browser
resolved against the api-server and got 404s. Setting
server.originto
localhost:5173(matching the--strictPorton the dev script)makes Vite emit fully-qualified URLs so workers load from Vite
directly.
Tested both in dev mode and non dev mode.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines