pt/pf_app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'pivot-busy-indicator'

Browse Source 
Show a Calculating indicator in the app's status bar while the pivot
rebuilds, since Perspective's own beaker is only rendered while the
settings panel is open. Also make the light/dark toggle actually reach
the pivot: the theme is a ViewerConfig field, not an attribute, and
stored layouts were reasserting their author's theme on every restore.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-23 01:18:32 -04:00
commit b378ab7639
3 changed files with 98 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
CLAUDE.md
View File

@ -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 `<html>`
- **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

27
ui/src/components/StatusBar.jsx
View File

@ -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' && (
<span
className="flex items-center gap-1.5 text-blue-600"
title="The pivot is rebuilding its view — sorting, filtering, or changing depth on a large grain takes a moment"
>
<svg width="12" height="12" viewBox="0 0 24 24" fill="none" stroke="currentColor"
strokeWidth="3" strokeLinecap="round" className="animate-spin">
<path d="M12 3a9 9 0 1 0 9 9" />
</svg>
Calculating…
</span>
)}
<div className="ml-auto flex items-center gap-2">
{user && (
<>

70
ui/src/views/Forecast.jsx
View File

@ -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([])
@ -439,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])
@ -784,7 +803,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)
}
@ -909,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
@ -1099,16 +1122,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 +1466,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 +1560,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 +1586,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 +1673,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 +1692,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) {