From a651c58dc3ab51091bf75c0db0ddb94a93c8edf0 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Wed, 23 Sep 2026 01:15:35 -0400 Subject: [PATCH 1/2] Show when the pivot is rebuilding, and stop layouts overriding the theme Two unrelated changes, in one commit because both touch Forecast.jsx in ways that cannot be staged apart without an interactive add. Calculating indicator. Perspective shows a beaker while the view rebuilds, but it lives in the viewer's status bar, which is only rendered while the settings panel is open -- so the signal disappears exactly when the panel is closed and you have nothing else to go on. Forecast.jsx now raises a counter around the calls that *start* a rebuild and lowers it after viewer.flush(), broadcasting `pf-pivot-busy` on the window; StatusBar listens and shows a spinner, following the `pf-data-changed` pattern already used between the two. Deliberately not driven from `perspective-config-update`, which was the obvious hook and is wrong: measured on a depth change over a large grain, the event arrives *after* the rebuild -- seconds late -- and the flush that follows resolves in 6ms. It reports the end of the work, not the start. This therefore covers what the toolbar drives (depth on both axes, applying a layout) and not sorting or filtering from the viewer's own chrome, which the app is never told has begun. Theme. viewer.save() includes `theme`, so every persisted layout had one baked in, and cleanLayout passed it through -- each restore reasserted the stored theme and silently undid the light/dark toggle. It is the reader's choice, not a property of the layout, so it is dropped on the way in; a published layout can no longer impose its author's theme either. sameConfig ignores it too, or toggling would light the dirty dot and offer to save a change nobody made. Co-Authored-By: Claude Opus 5 --- ui/src/components/StatusBar.jsx | 27 ++++++++++++++++ ui/src/views/Forecast.jsx | 57 +++++++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/ui/src/components/StatusBar.jsx b/ui/src/components/StatusBar.jsx index b7440dd..284f2b7 100644 --- a/ui/src/components/StatusBar.jsx +++ b/ui/src/components/StatusBar.jsx @@ -11,6 +11,10 @@ export default function StatusBar({ view, sources = [], sourceId, setSourceId, v const [info, setInfo] = useState(null) const [showInfo, setShow] = useState(false) const [copied, setCopied] = useState(false) + // Forecast.jsx mirrors the pivot's own updating state onto this event. The + // viewer shows the same thing as a beaker in its settings panel, which is + // exactly where you cannot see it once the panel is closed. + const [busy, setBusy] = useState(false) const refreshInfo = useCallback(async () => { if (!versionId || !showVersion) { setInfo(null); return } @@ -29,6 +33,12 @@ export default function StatusBar({ view, sources = [], sourceId, setSourceId, v return () => window.removeEventListener('pf-data-changed', onChange) }, [refreshInfo]) + useEffect(() => { + const onBusy = (e) => setBusy(!!e.detail?.busy) + window.addEventListener('pf-pivot-busy', onBusy) + return () => window.removeEventListener('pf-pivot-busy', onBusy) + }, []) + async function copyTable() { if (!info?.fc_table) return try { @@ -127,6 +137,23 @@ export default function StatusBar({ view, sources = [], sourceId, setSourceId, v )} + {/* Left of the ml-auto block, so it sits beside the pivot's own figures + rather than out among the account controls. Occupies no space at all + when idle: a permanently reserved slot reads as a thing that is broken + when empty. */} + {busy && view === 'forecast' && ( + + + + + Calculating… + + )} +
{user && ( <> diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 91e2318..0d81545 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -29,6 +29,13 @@ const LAYOUTS_KEY = (vid) => `pf_layouts_v${vid}` // legacy named list, migrate function cleanLayout(cfg, validCols) { if (!cfg) return cfg const c = { ...cfg } + // The theme is the reader's own light/dark choice, not a property of the + // layout. viewer.save() includes it, so it ends up baked into every stored + // config -- and restoring one then snapped the pivot back to whatever the + // author had, silently undoing the toggle. Dropped on the way in, which makes + // the app's `dark` state the only thing that sets it, and means a published + // layout cannot impose its author's theme on everyone who applies it. + delete c.theme // Dead expressions go before the axis filter, not after: dropping them from // `expressions` is what makes `ok()` reject them everywhere else. if (DEAD_ORDER_EXPRS.some(n => c.expressions?.[n] !== undefined)) { @@ -263,6 +270,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const [undoingId, setUndoingId] = useState(null) const viewerRef = useRef(null) + // Depth, not a boolean: config updates overlap, and the first one to finish + // would otherwise clear the flag while the others are still rebuilding. + const busyRef = useRef(0) const workerRef = useRef(null) const tableRef = useRef(null) const colMetaRef = useRef([]) @@ -784,7 +794,11 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // differs on every refresh, so it is never a real difference. function sameConfig(a, b) { if (!a || !b) return false - const strip = ({ table, ...rest }) => JSON.stringify(rest) + // table is the per-load table name, different on every refresh. theme is + // the reader's light/dark choice and is not part of the layout (see + // cleanLayout) -- without it here, toggling the theme would light the dirty + // dot and offer to save a layout nothing had edited. + const strip = ({ table, theme, ...rest }) => JSON.stringify(rest) return strip(a) === strip(b) } @@ -1099,16 +1113,18 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const levels = (cfg.split_by || []).length if (!levels) return const depth = Math.max(0, Math.min(n, levels)) + pivotBusy(true) try { // A count of levels to show, exactly as group_by_depth is -- the engine // treats both axes identically. See applyDepth for why 0 is floored to 1. await viewer.restore({ ...cfg, split_by_depth: Math.max(1, depth) }) + await viewer.flush() setSplitDepth(depth) } catch (err) { console.error('[applySplitDepth]', err) flash(err.message || String(err), 'error') return - } + } finally { pivotBusy(false) } try { const cfg2 = await captureConfig() if (cfg2) await persistLayout(versionId, cfg2) @@ -1441,6 +1457,35 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio } ` + // Tell the app's status bar whether the pivot is rebuilding, so the wait is + // visible with the settings panel closed. + // + // Perspective shows this as the beaker in its own status bar, which is the + // honest signal -- it is driven by the session's update_count. It cannot be + // read from here, though: that bar lives in the viewer's shadow root, and the + // component carrying it is only *rendered* while settings are open once the + // table has loaded, so with the panel closed there is no element to watch. An + // earlier attempt did exactly that and only ever lit up when the panel was + // open, which is precisely when you do not need it. + // + // So it is raised by the callers that *start* a rebuild, and lowered once + // viewer.flush() says everything pending has rendered. + // + // Not from `perspective-config-update`, which was the obvious hook and is the + // wrong one: it is dispatched *after* the rebuild it describes. Measured on a + // depth change over a large grain, the event arrived seconds late and the + // flush that followed resolved in 6ms -- so the flag went up and down well + // after the wait it was meant to cover. + // + // Consequence: this covers what the toolbar drives -- depth on both axes, and + // applying a layout. Sorting or filtering from the viewer's own chrome still + // shows nothing, because the app never learns those have begun. + function pivotBusy(on) { + busyRef.current = Math.max(0, busyRef.current + (on ? 1 : -1)) + window.dispatchEvent( + new CustomEvent('pf-pivot-busy', { detail: { busy: busyRef.current > 0 } })) + } + // Find the grid wherever it is. Guessing the nesting was wrong once already -- // the plugin element's own shadow root is not where the cells live -- so locate // regular-table by walking through shadow roots, then inject into whatever root @@ -1506,6 +1551,8 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // effect on its own. table is dropped: it is loaded by reference, and a stale // name in a restored config fails the lookup. const target = Math.max(1, d) + pivotBusy(true) + try { const { table: _t, ...cfg } = await viewer.save() await viewer.restore({ ...cfg, group_by_depth: target }) @@ -1530,7 +1577,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const plugin = await viewer.getPlugin() await plugin.draw(view) } + await viewer.flush() setExpandDepth(d) + } finally { pivotBusy(false) } } async function captureConfig() { @@ -1615,7 +1664,10 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const validCols = new Set(tableRef.current ? Object.keys(await tableRef.current.schema()) : []) const cfg = cleanLayout(layout.config, validCols) cfg.plugin_config = { ...(cfg.plugin_config || {}), edit_mode: 'SELECT_REGION' } + pivotBusy(true) + try { await viewer.restore(cfg) + await viewer.flush() adoptSplit(cfg.split_full?.length ? cfg.split_full : cfg.split_by, cfg.split_by_depth != null ? cfg.split_by_depth : null) setGroupFull(cfg.group_by || []) @@ -1631,6 +1683,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const merged = await captureConfig() markClean(merged || cfg, layout.id) await persistLayout(versionId, merged || cfg) + } finally { pivotBusy(false) } } async function deleteLayout(layout) { From 5b11222e094ec106cb861e2068187dfd0399d9f5 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Wed, 23 Sep 2026 01:18:26 -0400 Subject: [PATCH 2/2] Set the pivot theme through restore(), not setAttribute The previous commit stopped stored layouts overriding the theme, which was necessary and not sufficient: nothing was setting it either. setAttribute('theme', ...) does nothing in 5.4.0 -- the theme is a ViewerConfig field (ViewerConfig.theme, a theme name) and the element observes no such attribute -- so the pivot stayed light whatever the toggle said. Presumably it worked on an older Perspective; CLAUDE.md documented it and is corrected here too. A partial restore, so it does not rebuild the view the way applyDepth's whole-config restore does. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 6 +++++- ui/src/views/Forecast.jsx | 13 +++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c6ccf00..ae7f2bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -803,7 +803,11 @@ Theme state lives in `ui/src/theme.jsx` — a React context (`ThemeContext`) wit - **Toggle:** `setDark(d => !d)` in `StatusBar.jsx`; effect writes `localStorage` and toggles the `.dark` class on `` - **CSS:** `ui/src/index.css` defines CSS custom properties under `:root` (light) and `.dark`. All Tailwind color overrides are written as `.dark .bg-white { ... }` etc. — no Tailwind dark-mode config needed - **Palette:** dark mode uses Perspective's "Pro Dark" colours (`--bg-primary: #242526`, panels `#2a2c2f`, gridlines `#3b3f46`, text `#c5c9d0`) -- **Perspective viewer:** `Forecast.jsx` calls `viewer.setAttribute('theme', dark ? 'Pro Dark' : 'Pro Light')` both on initial load and in a `useEffect([dark, versionId])` so the viewer stays in sync when the toggle fires +- **Perspective viewer:** `Forecast.jsx` calls `viewer.restore({ theme: dark ? 'Pro Dark' : 'Pro Light' })` both on initial load and in a `useEffect([dark, versionId])`. A **partial** restore, so it does not rebuild the view the way `applyDepth`'s whole-config one does. + + It used to be `setAttribute('theme', …)`, which in 5.4.0 does **nothing** — the theme is a `ViewerConfig` field (`ViewerConfig.theme`, a theme *name*), and the element observes no such attribute. The pivot simply stayed light whatever the toggle said. + + Half the fix is in `cleanLayout`, which **drops `theme`**. `viewer.save()` includes it, so every stored layout had one baked in, and restoring one reasserted the author's theme and undid the toggle. It is the reader's choice, not a property of the layout — which also means a published layout cannot impose its theme on whoever applies it. `sameConfig()` ignores `theme` for the same reason: toggling is not a layout edit and must not light the dirty dot. - **Consuming the theme:** `import useTheme from '../theme.jsx'` then `const { dark, setDark } = useTheme()` ## After a change: restart, and Generate SQL diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 0d81545..45b77f2 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -449,7 +449,16 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio useEffect(() => { if (viewerRef.current) { - viewerRef.current.setAttribute('theme', dark ? 'Pro Dark' : 'Pro Light') + // restore(), not setAttribute('theme'): the theme is a ViewerConfig field + // in 5.4.0 (ViewerConfig.theme, a theme *name* like "Pro Dark"), and the + // element observes no such attribute -- the attribute call this replaced + // did nothing at all, so the pivot simply stayed light. + // + // A partial update: everything else is left as it is, so this does not + // rebuild the view the way applyDepth's whole-config restore does. + // cleanLayout drops `theme` on the way in, so no layout restore puts it + // back afterwards. + viewerRef.current.restore({ theme: dark ? 'Pro Dark' : 'Pro Light' }) } }, [dark, versionId]) @@ -923,7 +932,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio console.error('[setAutoPause]', err) } - viewer.setAttribute('theme', dark ? 'Pro Dark' : 'Pro Light') + await viewer.restore({ theme: dark ? 'Pro Dark' : 'Pro Light' }) if (!hideSplitTotal()) setTimeout(hideSplitTotal, 400) // restore last-used layout or build default