From a651c58dc3ab51091bf75c0db0ddb94a93c8edf0 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Wed, 23 Sep 2026 01:15:35 -0400 Subject: [PATCH] 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) {