refactor(app): carve the DOM/modal primitives out of app.js (R3a) - #882
Conversation
static/js/dom.js (203 lines) — esc, _escAttr, _isElementVisible, _trapFocusInModal,
_confirmDialog, uiPrompt. Bodies VERBATIM. app.js 10,593 → 10,414.
A GATHER, not a slice — the six lived in six different places (108, 635, 659,
2617, 2623, 8892). They belong together because they are the BOTTOM of the UI
stack: `esc` alone has 25 call sites and `_escAttr` 23, and every later carve that
renders HTML will need them.
That is the actual point of doing this one now. Give them a home and the next
carve imports them; leave them in app.js and the next carve that renders HTML has
to invent a host seam to reach back into app.js — exactly the trap the
plugin-loader carve had to work around until the viz layer became a module. This
is the cheapest possible way to stop that recurring.
app.js -> { plugin-loader, viz, diagnostics-export, dom }
plugin-loader -> viz
viz, diagnostics-export, dom -> (nothing)
Zero imports. Six exports (every one is used outside the cluster).
VERIFIED BY DRIVING THE MODALS, not just booting — they are interactive, so a
green suite says little. A/B against origin/main in two browsers:
* window.uiPrompt / _confirmDialog / _trapFocusInModal all resolve
* uiPrompt() mounts its modal, accepts typed input, and resolves with the typed
value ('typed') — IDENTICAL on both
* _confirmDialog() mounts and resolves true on confirm — IDENTICAL
* zero console/page errors either side
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…883) static/js/highway-colors.js (601 lines) — bodies VERBATIM. app.js 10,415 → 9,837. Under 10k. DECOMPOSED, not sliced. The "settings" blob measured 47 fns / 19 inbound and was not carvable as-is. Seeding the closure from a FUNCTION (initHighwayColors) found only 18 fns and left 4 HWC_* constants used outside it — i.e. the seed was wrong, not the cluster. Re-seeding from the STATE (every function touching HWC_*/`_hwc*`) found the true cluster: 45 top-level nodes, lines 2751-3331, CONTIGUOUS, with ZERO foreign nodes inside the span. INBOUND: 0. EXPORTS: 2 (initHighwayColors, hwcInitSettingsUI). The other 43 symbols — the HWC_* tables, the 12 presets, the theme store, the share codec, the picker handlers, the window.feedBack.highwayColors facade — are used nowhere else in core and stay private. No inline on*= handlers here (the Settings buttons are wired by addEventListener inside hwcInitSettingsUI), so nothing needed re-exposing on window. The three bus listeners register inside initHighwayColors, which app.js calls — not at module top level — so no ordering change. THE no-undef GATE EARNED ITS KEEP. My closure said INBOUND=0; the module actually uses `uiPrompt` (the "name this theme" prompt). It was missed because uiPrompt is no longer an app.js DECLARATION — it's an IMPORT BINDING (from #882's dom.js), and I was collecting declarations only. `no-undef` with typeof:true caught it. => Lesson for the next carve: seed `tops` from ImportDeclaration bindings too. => And it VALIDATES carving dom.js early: this module just imports uiPrompt from it. Had dom.js still been stranded in app.js, this carve would have needed a host seam. app.js -> { plugin-loader, viz, diagnostics-export, dom, highway-colors } plugin-loader -> viz highway-colors -> dom viz, diagnostics-export, dom -> (leaves) VERIFIED BY DRIVING THE FACADE. A/B against origin/main in two browsers: window.feedBack.highwayColors installed, identical method surface, 12 presets, identical default slot colours, and a share-code encode→decode round-trip returning #112233 — IDENTICAL on both, zero console/page errors either side. Harnesses: highway_colors_facade + highway_string_colors retargeted (both brace-extract blocks out of the source by signature). pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fourth carve.
static/js/dom.js(203 lines) —esc,_escAttr,_isElementVisible,_trapFocusInModal,_confirmDialog,uiPrompt. Bodies verbatim.app.js10,593 → 10,414.A gather, not a slice — and that's the point
The six lived in six different places in app.js (lines 108, 635, 659, 2617, 2623, 8892). They belong together because they're the bottom of the UI stack:
escalone has 25 call sites,_escAttr23, and every later carve that renders HTML will need them.That's the real reason to do this one now, ahead of bigger clusters:
imports them.That's exactly the trap the plugin-loader carve (#878) had to work around with
configurePluginLoader(), until the viz layer became a module (#880) and the seam could be deleted. This is the cheapest way to stop that recurring.Zero imports. Six exports (every one is used outside the cluster).
Verified by driving the modals
These are interactive, so a green suite says very little. A/B against
origin/mainin two browsers:window.uiPrompt/_confirmDialog/_trapFocusInModalfunctionuiPrompt()mounts its modaltrue'typed'_confirmDialog()mounts and resolves on confirmtruepytest 2396 · node 1038/1038 · ESLint 0 · tailwind-fresh clean · Codex 0.
🤖 Generated with Claude Code