From 095346730207bbe8a0626cdbc30e0a503c4d4efa Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Tue, 1 Sep 2026 13:29:03 +0000 Subject: [PATCH 1/3] feat(dash): group the 17 dashboard tabs into 5 question-based groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seventeen co-equal top-level tabs was a flat list pretending to be a hierarchy: every viewer scanned all seventeen labels to find one, and the manager got no signal about which three mattered. The nav is now two levels and five groups, chunked by the question the viewer arrived with — Overview, Savings, Behaviour, Traffic, Admin — so a manager can stop after Savings and an operator can carry on, on one nav rather than two products. The URL carries both levels. The canonical hash is `#//`, and the rule is that the LAST path segment is the view: anything before it is a group hint, ignored whenever the view is known. That makes the one-level `#usage` this dashboard has always written and the new `#/savings/usage` the same link, and it is decidable without a lookahead because no view name contains a slash. Two of those one-level shapes are written by the server — dash/kvcache.go builds `#requests?req=` and `#sessions?diff=` for every row of the KV-cache table, and the UI is forbidden from building them — so the one-segment branch is not a shim to be tidied away later. The whole contract is a table in dash/navhash.test.mjs, run against the real resolver in app.js rather than against a Go re-implementation of it: seventeen bare view names, all fourteen filter dimensions, from/to, sort/dir, the three drawer keys, legacy `range=`, the two server-authored shapes, and every rewrite case. `go test ./dash/ -run NavHash` runs it under node when node is present and says loudly what went unverified when it is not; TestTheNavHashContractIsPinned holds either way. Three things fixed while rebuilding this markup: - ONE TAB STOP PER LEVEL. Seventeen role=tab buttons were seventeen Tab stops, so reaching the filter bar by keyboard took seventeen presses. Now it is a roving tabindex with the arrows moving inside each tablist, and manual activation — automatic would fire seven queries scrubbing across Admin. Measured over 34 tab x theme combinations: 39 stops per page down to 23. - REAL TABPANELS. There were none; all seventeen view sections now carry role=tabpanel and aria-labelledby, and each tab its aria-controls. - 44px TOUCH TARGETS. Nav tabs were 28px tall at every width and nothing about them reflowed. Nav elements under 44x44 at 390px: 34 to 0. Contrast failures and missing focus rings stay at 0 across all 68 audited combinations, and no view gains horizontal body scroll. The three self-mounting views now go through one mountTab({group, after, view, label}) in app.js instead of each reaching into `.tabs` and insertBefore-ing a named sibling. The tab exists before the view body is built, so lazy-loading those bodies later is a change to the caller and not to the nav. On a single-tenant local proxy the manager-gated tabs without data-local-ok are now disabled with a one-line reason rather than hidden: only nine of the seventeen tabs are otherwise reachable there, so hiding the rest made the product look half its size and read as a broken build. data-account tabs stay silently hidden — a signed-out viewer has no use for a tab they cannot enable from here. The chrome is one sticky box now. It was three separately-stuck elements each needing the previous one's height as a magic `top` offset (.filters was `top: 41px`), which a second nav level breaks and which nothing kept honest. Signed-off-by: Osher-Elhadad --- dash/api_test.go | 5 + dash/navhash.test.mjs | 255 +++++++++++++++++++++++++++++++++ dash/navhash_test.go | 104 ++++++++++++++ dash/ui/app.js | 245 +++++++++++++++++++++++++++++-- dash/ui/campaigns.js | 25 ++-- dash/ui/index.html | 111 ++++++++++---- dash/ui/kvcache.js | 21 +-- dash/ui/style.css | 41 +++++- dash/ui/tools.js | 28 ++-- dash/uikvcache_test.go | 14 +- docs/dashboard-kvcache-page.md | 2 +- docs/dashboard.md | 75 +++++++++- 12 files changed, 831 insertions(+), 95 deletions(-) create mode 100644 dash/navhash.test.mjs create mode 100644 dash/navhash_test.go diff --git a/dash/api_test.go b/dash/api_test.go index 13582f8b..6c3439ea 100644 --- a/dash/api_test.go +++ b/dash/api_test.go @@ -336,6 +336,11 @@ func TestUIHasTestIDsForEveryStatTile(t *testing.T) { "detail-summary", "detail-components", "live-table", "live-indicator", "theme-toggle", "tab-overview", "tab-components", "tab-sessions", "tab-requests", "tab-benchmarks", "tab-config", "filter-q", "filter-range", "filter-model", + // The nav's first level. tab-overview above is the Overview group's button: that + // group is also a view, so it is the one that carries data-view as well, and its + // testid stays the way to reach the Overview view. tab-note is the one line that + // says why a tab in the open group is locked. + "group-savings", "group-behaviour", "group-traffic", "group-admin", "tab-note", "filter-provider", "filter-agent", "filter-preset", "filter-mode", "filter-component", "filter-reason", "filter-accounting", "filter-clear", "request-row", "diff-mode-git", "diff-mode-side", "diff-mode-orig", "diff-mode-raw", diff --git a/dash/navhash.test.mjs b/dash/navhash.test.mjs new file mode 100644 index 00000000..0be74500 --- /dev/null +++ b/dash/navhash.test.mjs @@ -0,0 +1,255 @@ +// The dashboard's URL contract, tested against the REAL resolver in app.js. +// +// Every filtered view of this dashboard is a link, and those links are pasted into issues, +// runbooks and commit messages — and two of the shapes are written by the SERVER +// (dash/kvcache.go:510-511), so they cannot be found and updated. Adding a second nav level +// changed the canonical hash from `#` to `#//`, which is precisely the +// kind of change that silently breaks every link ever written. Hence a table rather than a +// hand check. +// +// There is no bundler and no test framework in this project, on purpose: `node --test` and +// `node:assert` ship with node. app.js is a classic script, so it is loaded the way the +// browser loads it — as source, into a function scope, over a stub DOM small enough to read. +// Nothing here is a second implementation of the resolver; a change to app.js changes what +// this file tests. +// +// node --test dash/navhash.test.mjs (or: go test ./dash/ -run NavHash) +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; + +// Beside dash/, not inside dash/ui/, because `//go:embed ui` (dash/ui.go:29) would +// otherwise ship this file inside the proxy binary and serve it at /dashboard/. +const read = (f) => readFileSync(new URL('./ui/' + f, import.meta.url), 'utf8'); + +// ── the stub DOM ─────────────────────────────────────────────────────────── +// The nav is the only DOM the resolver touches, and it reads it through exactly one +// selector shape. So the tab list is parsed out of index.html and the three self-mounting +// views' own mountTab() calls, which makes this a test of the real nav rather than of a +// hand-written copy of it: a tab moved between groups in the markup moves here too. +function navFromSource() { + const groups = new Map(); + const mounted = []; + const html = read('index.html'); + for (const [, group, body] of html.matchAll( + /
]*data-group="([a-z]+)"[^>]*>([\s\S]*?)<\/div>/g)) { + groups.set(group, [...body.matchAll(/data-view="([a-z]+)"/g)].map((m) => m[1])); + } + // The self-mounters declare their own group and position; splice them in the same way + // mountTab does, so `after:` is exercised rather than assumed. + for (const f of ['tools.js', 'kvcache.js', 'campaigns.js']) { + const call = read(f).match(/mountTab\(\{([\s\S]*?)\}\)/); + assert.ok(call, `${f} no longer calls mountTab()`); + const field = (k) => (call[1].match(new RegExp(k + ":\\s*'([a-z]+)'")) || [])[1]; + const [group, view, after] = [field('group'), field('view'), field('after')]; + const list = groups.get(group); + assert.ok(list, `${f} mounts into group ${group}, which index.html does not define`); + const at = list.indexOf(after); + assert.ok(at >= 0, `${f} mounts after ${after}, which is not in group ${group}`); + list.splice(at + 1, 0, view); + mounted.push([view, group]); + } + return { groups, mounted }; +} +const { groups: NAV, mounted: MOUNTED } = navFromSource(); + +/** loadApp evaluates app.js over the stub and hands back the internals under test. */ +function loadApp() { + const loc = { pathname: '/dashboard/', hash: '' }; + // Only the one selector shape the nav reads, and a tab object with only the three + // properties reachable() and firstView() look at. Anything else returns empty, which is + // what a null-DOM would do anyway. + const querySelectorAll = (sel) => { + const m = /^\.viewtabs\[data-group="([a-z]+)"\] \.tab$/.exec(sel); + return (m ? NAV.get(m[1]) || [] : []).map((v) => ({ + hidden: false, dataset: { view: v }, getAttribute: () => null, + })); + }; + const noop = () => {}; + const stub = { + document: { + addEventListener: noop, querySelector: () => null, querySelectorAll, + documentElement: { getAttribute: noop, setAttribute: noop }, head: { appendChild: noop }, + createElement: () => ({ setAttribute: noop, appendChild: noop, classList: {}, style: {} }), + }, + window: { addEventListener: noop }, + localStorage: { getItem: () => null, setItem: noop, removeItem: noop }, + location: loc, + EventSource: function EventSource() {}, setInterval: noop, setTimeout: noop, fetch: noop, + }; + const names = Object.keys(stub); + const api = new Function(...names, + read('app.js') + '\n;return { parseURL, urlFor, resolveNav, navPath, state, DIMS, GROUP_OF };')( + ...names.map((n) => stub[n])); + api.at = (hash) => { loc.hash = hash; return api.parseURL(); }; + // urlFor reads `state`, so a round-trip is: parse a hash, adopt it, write it back. + api.roundTrip = (hash) => { + const w = api.at(hash); + Object.assign(api.state, w); + return api.urlFor().replace('/dashboard/', ''); + }; + return api; +} +const app = loadApp(); +// mountTab() does exactly this to GROUP_OF when a self-mounting view loads. Doing it here is +// what makes the three appended views' hashes testable without a browser; the groups and the +// positions come from their own mountTab() calls, not from a list in this file. +for (const [view, group] of MOUNTED) app.GROUP_OF.set(view, group); + +const VIEWS = [...NAV.values()].flat().concat('overview'); + +// ── 1. every view, by its bare legacy name and in canonical form ─────────── +test('all 17 views resolve from a bare # and canonicalise to #//', () => { + assert.equal(VIEWS.length, 17, 'the nav is no longer 17 views; update this table'); + for (const v of VIEWS) { + assert.equal(app.at('#' + v).view, v, `#${v} must still resolve to ${v}`); + const canon = app.roundTrip('#' + v); + assert.equal(app.at(canon).view, v, `${canon} must resolve back to ${v}`); + assert.match(canon, /^#\//, `${canon} is not in canonical #/... form`); + // Overview is a group AND a view, so its canonical path is the one that is not doubled. + const want = v === 'overview' ? '#/overview' : '#/' + app.navPath(v); + assert.equal(canon, want); + assert.equal(app.at(want).view, v); + } +}); + +// ── 2. all 14 filter dimensions survive a round trip ────────────────────── +test('all 14 filter dimensions are read and written', () => { + const dims = app.DIMS.map(([k]) => k); + assert.deepEqual(dims, ['q', 'model', 'provider', 'agent', 'preset', 'mode', 'component', + 'reason', 'accounting', 'effort', 'thinking', 'stop_reason', 'session', 'tenant']); + for (const k of dims) { + const got = app.at(`#requests?${k}=v1`); + assert.equal(got.filter[k], 'v1', `${k} is not parsed out of the hash`); + assert.match(app.roundTrip(`#requests?${k}=v1`), new RegExp(`[?&]${k}=v1`), + `${k} is parsed but not written back`); + } + // And together, so no dimension is dropped when another is set. + const all = dims.map((k) => `${k}=${k}v`).join('&'); + const got = app.at('#requests?' + all); + for (const k of dims) assert.equal(got.filter[k], k + 'v'); +}); + +// ── 3. the time window ──────────────────────────────────────────────────── +test('from/to take a relative token or absolute ms, and `to` is omitted while now', () => { + assert.equal(app.at('#usage?from=now-24h').from, 'now-24h'); + assert.equal(app.at('#usage?from=now-24h').to, 'now'); + assert.equal(app.at('#usage?from=1700000000000').from, 1700000000000); + assert.equal(app.at('#usage?from=1700000000000&to=1700000900000').to, 1700000900000); + assert.equal(app.at('#usage?from=now-2d&to=now-1d').to, 'now-1d'); + assert.ok(!app.roundTrip('#usage?from=now-6h').includes('to='), + 'a live window must stay live for whoever the link is sent to'); + assert.ok(app.roundTrip('#usage?from=now-2d&to=now-1d').includes('to=now-1d')); + // Junk is all-time rather than NaN, which would send `since=NaN` to the API. + assert.equal(app.at('#usage?from=tomorrow').from, 0); + assert.equal(app.at('#usage?from=-5').from, 0); +}); + +// ── 4. LEGACY range=. Read, never written. docs/dashboard.md documents it. ── +test('legacy range= still maps onto the equivalent relative window', () => { + for (const [ms, want] of [[86400000, 'now-1d'], [3600000, 'now-1h'], [604800000, 'now-1w'], + [300000, 'now-5m'], [1000, 'now-1s'], [1500, 'now-2s']]) { + assert.equal(app.at(`#usage?range=${ms}`).from, want, `range=${ms}`); + } + // The documented example, end to end: docs/dashboard.md. + assert.equal(app.roundTrip('#usage?range=86400000'), '#/savings/usage?from=now-1d'); + assert.ok(!app.roundTrip('#usage?range=86400000').includes('range='), + 'range= is read for compatibility and never written'); +}); + +// ── 5. sort/dir — written only on components, parsed anywhere ────────────── +test('sort and dir parse on any view and are written only on components', () => { + assert.equal(app.at('#components?sort=saved&dir=asc').sort, 'saved'); + assert.equal(app.at('#components?sort=saved&dir=asc').dir, 'asc'); + assert.equal(app.at('#components?sort=saved').dir, 'desc', 'dir defaults to desc'); + assert.equal(app.at('#requests?sort=saved').sort, 'saved', 'sort parses on every view'); + assert.ok(app.roundTrip('#components?sort=saved&dir=asc').includes('sort=saved&dir=asc')); + assert.ok(!app.roundTrip('#requests?sort=saved').includes('sort='), + 'a sort of another view’s table is not this view’s state'); +}); + +// ── 6. the drawer: req | diff | acct, mutually exclusive ────────────────── +test('the drawer keys are mutually exclusive and round-trip', () => { + assert.deepEqual(app.at('#requests?req=9').drawer, { req: 9 }); + assert.deepEqual(app.at('#sessions?diff=s-1').drawer, { diff: 's-1' }); + assert.deepEqual(app.at('#tenants?acct=a-1').drawer, { acct: 'a-1' }); + assert.deepEqual(app.at('#requests?req=9&diff=s-1').drawer, { req: 9 }, 'req wins'); + assert.equal(app.at('#requests').drawer, null); + assert.equal(app.at('#requests?req=0').drawer, null, 'req=0 is not a request'); + assert.equal(app.roundTrip('#requests?req=9'), '#/traffic/requests?req=9'); + assert.equal(app.roundTrip('#sessions?diff=s-1'), '#/traffic/sessions?diff=s-1'); +}); + +// ── 7. THE TWO SHAPES THE SERVER WRITES ─────────────────────────────────── +// dash/kvcache.go:510-511 builds these, asserted in dash/kvcache_test.go:380,383, and +// dash/uikvcache_test.go:267 asserts the UI must NOT build them — so the server stays their +// sole author and these strings cannot be found by grepping the front end. +test('the hashes dash/kvcache.go writes still resolve', () => { + const req = app.at('#requests?req=1234'); + assert.equal(req.view, 'requests'); + assert.deepEqual(req.drawer, { req: 1234 }); + // A session id is client-supplied, so the server percent-escapes it; decoding is + // URLSearchParams' job and a `/` in the id must not read as a path separator. + const diff = app.at('#sessions?diff=sess%2Fwith%20space'); + assert.equal(diff.view, 'sessions'); + assert.deepEqual(diff.drawer, { diff: 'sess/with space' }); +}); + +// ── 8. two-level paths ──────────────────────────────────────────────────── +test('#/group/view, #/group alone, and a stale group segment', () => { + assert.equal(app.at('#/behaviour/kvcache').view, 'kvcache'); + assert.equal(app.at('#/savings/campaigns').view, 'campaigns'); + // The leading slash is written but not required, which is what settles the ambiguity the + // one-level form left: the LAST segment is the view, so `savings/campaigns` cannot be + // read as a view literally named "savings/campaigns". + assert.equal(app.at('#savings/campaigns').view, 'campaigns'); + assert.equal(app.roundTrip('#savings/campaigns'), '#/savings/campaigns'); + // A group on its own opens its first tab. + assert.equal(app.at('#/savings').view, 'usage'); + assert.equal(app.at('#/behaviour').view, 'components'); + assert.equal(app.at('#/traffic').view, 'sessions'); + assert.equal(app.at('#/admin').view, 'config'); + assert.equal(app.at('#/overview').view, 'overview'); + // A view that moved group is still found; the segment before it is only a hint. + assert.equal(app.at('#/admin/campaigns').view, 'campaigns'); + assert.equal(app.roundTrip('#/admin/campaigns'), '#/savings/campaigns'); + // A group that survived and a view name that did not. + assert.equal(app.at('#/savings/typo').view, 'usage'); + // Filters ride along on both forms. + assert.equal(app.at('#/savings/usage?model=m1').filter.model, 'm1'); +}); + +// ── 9. junk resolves to Overview rather than to a blank page ─────────────── +test('an unknown, empty or mis-cased hash resolves to overview', () => { + for (const h of ['', '#', '#/', '#nonsense', '#OVERVIEW', '#Usage', '#/no/such/thing', + '#view-usage', '#/nonsense/nonsense']) { + assert.equal(app.at(h).view, 'overview', `${JSON.stringify(h)} should fall back`); + } + // Case-sensitive on purpose: #OVERVIEW is not a near-miss to be corrected, it is a name + // that does not exist, and go() rewrites the address bar so it does not stay wrong. + assert.equal(app.roundTrip('#OVERVIEW'), '#/overview'); + assert.equal(app.roundTrip('#nonsense'), '#/overview'); + // Empty parameters are dropped rather than becoming empty filters. + const got = app.at('#requests?req=9&&&'); + assert.equal(got.view, 'requests'); + assert.deepEqual(got.drawer, { req: 9 }); + assert.equal(app.roundTrip('#requests?req=9&&&'), '#/traffic/requests?req=9'); + assert.deepEqual(app.at('#requests?model=').filter, {}, 'an empty value is not a filter'); +}); + +// ── 10. every view in the nav is a real view, and vice versa ────────────── +test('the nav, the loaders and the view sections agree', () => { + const html = read('index.html'); + for (const v of VIEWS) { + assert.ok(app.GROUP_OF.has(v) || ['tools', 'kvcache', 'campaigns'].includes(v), + `${v} has no group`); + } + // A tab whose section is missing is a tab that switches to a blank page. The three + // self-mounted sections are built in JS, so they are checked in their own files. + for (const v of VIEWS) { + if (['tools', 'kvcache', 'campaigns'].includes(v)) continue; + assert.ok(html.includes(`id="view-${v}"`), `no section for ${v}`); + assert.ok(html.includes(`aria-labelledby="tab-${v}"`), `${v}'s panel names no tab`); + assert.ok(html.includes(`aria-controls="view-${v}"`), `${v}'s tab controls no panel`); + } +}); diff --git a/dash/navhash_test.go b/dash/navhash_test.go new file mode 100644 index 00000000..0d0352de --- /dev/null +++ b/dash/navhash_test.go @@ -0,0 +1,104 @@ +package dash + +import ( + "os" + "os/exec" + "strings" + "testing" +) + +// The URL contract is tested in JS, against the real resolver, because a second +// implementation of it in Go would prove that the two agreed and nothing about what a +// pasted link does. See navhash.test.mjs for the table and the reasoning; this is the +// wrapper that makes `go test ./dash/` run it. +// +// node is not a build dependency of this project and never will be — the dashboard ships as +// files in a Go binary with no bundler. So this skips when node is absent, LOUDLY, naming +// what then goes unverified. TestTheNavHashContractIsPinned below is the part that holds +// without node. +func TestNavHashCompatibility(t *testing.T) { + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is not on PATH, so navhash.test.mjs did not run: the URL " + + "compatibility contract (17 bare views, 14 filter dimensions, legacy range=, " + + "the two hashes dash/kvcache.go writes, and #/group/view) is UNVERIFIED in this " + + "run. Run `node --test dash/navhash.test.mjs`.") + } + out, err := exec.Command(node, "--test", "navhash.test.mjs").CombinedOutput() + if err != nil { + t.Fatalf("node --test navhash.test.mjs failed: %v\n%s", err, out) + } + // A pass with zero tests run is not a pass. + if !strings.Contains(string(out), "# fail 0") || strings.Contains(string(out), "# pass 0") { + t.Fatalf("unexpected node --test summary:\n%s", out) + } +} + +// What the JS table covers, asserted statically so it holds in a run with no node: the +// pieces of the URL contract that are single strings in the source, and would each break a +// documented or server-authored link if they went missing. +func TestTheNavHashContractIsPinned(t *testing.T) { + app := readUI(t, "ui/app.js") + for _, want := range []struct{ needle, why string }{ + {"function legacyFrom(", "legacy range= bookmarks (docs/dashboard.md) map onto a relative window here"}, + {"p.get('range')", "range= is no longer read, so every pre-from/to link widens to all time"}, + {"function resolveNav(", "the one place a hash path becomes a view"}, + {"function navPath(", "the one place a view becomes a hash path"}, + {`replace(/^#\/?/, '')`, "the leading slash of the canonical #/group/view form is no longer optional"}, + {"'#/' + navPath(", "urlFor no longer writes the canonical two-level hash"}, + } { + if !strings.Contains(app, want.needle) { + t.Errorf("app.js no longer contains %q: %s", want.needle, want.why) + } + } + // The 14 filter dimensions, by name. Dropping one silently narrows nothing and widens + // every link that set it. + for _, dim := range []string{"q", "model", "provider", "agent", "preset", "mode", + "component", "reason", "accounting", "effort", "thinking", "stop_reason", "session", + "tenant"} { + if !strings.Contains(app, "['"+dim+"',") { + t.Errorf("filter dimension %q is not in DIMS; links carrying it break", dim) + } + } + // The nav's five groups, and the fact that mountTab is the only thing that knows the + // nav's DOM shape. + for _, g := range []string{"overview", "savings", "behaviour", "traffic", "admin"} { + if !strings.Contains(app, "['"+g+"', [") { + t.Errorf("nav group %q is gone from GROUPS", g) + } + } + for _, f := range []string{"ui/tools.js", "ui/kvcache.js", "ui/campaigns.js"} { + src := readUI(t, f) + if !strings.Contains(src, "mountTab({") { + t.Errorf("%s does not mount its tab through mountTab()", f) + } + if strings.Contains(src, "$('.tabs')") { + t.Errorf("%s reaches into the nav itself; mountTab() is the one place that knows "+ + "its DOM shape", f) + } + } +} + +// The two hashes the SERVER writes (dash/kvcache.go:510-511) are one-level, legacy-shaped, +// and unreachable by grepping the front end — the UI is forbidden from building them +// (TestTheDetailTableLinksAreServerBuilt). So the resolver's one-segment branch is not a shim +// to be tidied away later; it is what makes every row of the KV-cache table clickable. +func TestTheServerAuthoredHashesNameViewsTheNavStillHas(t *testing.T) { + html := readUI(t, "ui/index.html") + for _, view := range []string{"requests", "sessions"} { + if !strings.Contains(html, `data-view="`+view+`"`) { + t.Errorf("dash/kvcache.go writes #%s?..., and there is no longer a %q tab for the "+ + "resolver to find", view, view) + } + } + src, err := os.ReadFile("kvcache.go") + if err != nil { + t.Fatal(err) + } + for _, shape := range []string{`"#requests?req=%d"`, `"#sessions?diff=" + url.PathEscape`} { + if !strings.Contains(string(src), shape) { + t.Errorf("kvcache.go no longer writes %s; if it moved, navhash.test.mjs "+ + "section 7 must move with it", shape) + } + } +} diff --git a/dash/ui/app.js b/dash/ui/app.js index 2adee55d..579a1959 100644 --- a/dash/ui/app.js +++ b/dash/ui/app.js @@ -159,6 +159,9 @@ function modeLabel(m) { // ── state ────────────────────────────────────────────────────────────────── const state = { view: 'overview', + // The open nav group, derived from `view` by go(). Held rather than recomputed because + // both nav levels read it on every switch. + group: 'overview', filter: {}, // loadedAt is when the rollups on screen were fetched, and dirty is whether the server // has captured a request since. Together they are the whole freshness contract of a page @@ -4209,6 +4212,168 @@ const loaders = { keepalive: loadKeepAlive, }; +// ── nav: two levels, five groups ─────────────────────────────────────────── +// +// MODULE SEAM (app.js split, sequenced after this change): everything from here to the end +// of `applyURL` is the SHELL + ROUTER — the nav, the hash contract and the view switch. It +// depends on `loaders`, `DIMS`, `state` and the DOM helpers, and nothing below it depends on +// its internals except through `go`, `mountTab` and `syncNav`. The other three seams are: +// overview+usage+components (the rollup views), sessions+requests (the traffic views and the +// request/diff drawer), and admin (setup/settings/tenants/strategies/archive/feedback/config). +// +/** + * GROUPS is the nav, and the ONLY place the two levels' shape is written down: the group + * buttons in index.html mirror it, the per-group tablists mirror it, and the hash's first + * path segment is a group name from it. + * + * It deliberately lists only the views index.html authors. Inventory, KV-cache and + * Campaigns are absent because they mount themselves (mountTab) — hardcoding them here + * would put back exactly the coupling that self-mounting exists to avoid. + */ +const GROUPS = [ + ['overview', ['overview']], + ['savings', ['usage', 'benchmarks']], + ['behaviour', ['components', 'keepalive']], + ['traffic', ['sessions', 'requests']], + ['admin', ['config', 'strategies', 'tenants', 'setup', 'settings', 'archive', 'feedback']], +]; +/** GROUP_OF maps a view onto its group. mountTab adds to it, which is how a self-mounted + * view gets a group without this file naming it. */ +const GROUP_OF = new Map(GROUPS.flatMap(([g, views]) => views.map((v) => [v, g]))); + +/** navTab is a view's tab button, in whichever level owns it: Overview's tab is a group + * button (it is a group AND a view), every other view's tab is in a group's tablist. */ +function navTab(view) { return $(`[data-view="${view}"]`); } +/** reachable is "this account can open it now" — the permission gate hides, the local-mode + * lock only disables, and neither is navigable. */ +function reachable(tab) { return !tab.hidden && tab.getAttribute('aria-disabled') !== 'true'; } +/** firstView is what a group opens on: its first REACHABLE tab, so Admin on a single-tenant + * proxy lands on Config rather than on a locked Tenants. null = nothing in it is open to + * this viewer, and the group button hides. */ +function firstView(group) { + if (group === 'overview') return 'overview'; + const t = $$(`.viewtabs[data-group="${group}"] .tab`).find(reachable); + return t ? t.dataset.view : null; +} + +/** + * mountTab adds one tab and its panel, and is the single place outside this section that + * knows the nav's DOM shape — tools.js, kvcache.js and campaigns.js each call it once. + * + * The tab button exists as soon as the caller runs, BEFORE the view's body is built, so + * lazy-loading those bodies later is a change to the caller and not to the nav. + * + * It returns the empty `
`, already appended to #main and already + * wired as the tab's tabpanel, which is what each caller then fills. + */ +function mountTab({ group, after, view, label, manager }) { + const list = $(`.viewtabs[data-group="${group}"]`); + if (!list) throw new Error('mountTab: no such group: ' + group); + const tab = el('button', { + role: 'tab', class: 'tab', id: 'tab-' + view, 'data-view': view, + 'data-testid': 'tab-' + view, 'aria-controls': 'view-' + view, + 'aria-selected': 'false', tabindex: '-1', + // `manager` is the only gate any self-mounting view has needed. data-local-ok and + // data-account are markup-only for that reason; add a flag the day a caller wants one. + 'data-manager': manager ? '' : null, + // A gated tab mounts hidden and applyAccount reveals it, exactly like the ones in the + // markup: a manager tab that is visible for the moment before /api/whoami answers is a + // manager tab a non-manager can click. + hidden: manager ? 'hidden' : null, + }, label); + const sib = after && $(`.tab[data-view="${after}"]`, list); + list.insertBefore(tab, sib ? sib.nextSibling : null); + GROUP_OF.set(view, group); + const panel = el('section', { + class: 'view', id: 'view-' + view, role: 'tabpanel', + 'aria-labelledby': 'tab-' + view, hidden: 'hidden', + }); + $('#main').appendChild(panel); + return panel; +} + +/** + * syncNav makes both nav levels match state: which group is open, which of its tabs is + * selected, which groups this viewer has anything in, and ONE tab stop per level. + * + * That last part is the fix for a measured defect: seventeen `role="tab"` buttons were + * seventeen Tab stops, so reaching the filter bar by keyboard meant seventeen presses. The + * WAI-ARIA tabs pattern is one stop per tablist with the arrows moving inside it — navKeys. + */ +function syncNav() { + const group = state.group || 'overview'; + for (const b of $$('.groups .tab')) { + b.hidden = !firstView(b.dataset.group); + const on = b.dataset.group === group; + b.setAttribute('aria-selected', String(on)); + b.tabIndex = on ? 0 : -1; + } + const panel = $(`#subnav .grouppanel[data-group="${group}"]`); + for (const p of $$('#subnav .grouppanel')) p.hidden = p !== panel; + // Overview is a group with one view, so it has no second level at all rather than a + // sub-nav row holding a single tab that repeats the label above it. And nothing in the + // nav means anything while the gate is up — a stray sub-nav bar over a login form is + // worse than no nav, which is why showGate calls back into here. + $('#subnav').hidden = !panel || gated(); + for (const t of $$('.viewtabs .tab')) { + const on = t.dataset.view === state.view; + t.setAttribute('aria-selected', String(on)); + t.tabIndex = on ? 0 : -1; + } + // A tablist whose selected tab is not in it (mid-switch) would have no tab stop at all. + if (panel && !$('.tab[tabindex="0"]', panel)) { + const t = $$('.tab', panel).find(reachable) || $('.tab', panel); + if (t) t.tabIndex = 0; + } + renderTabNote(panel); +} + +/** + * lockTab marks a tab present-but-not-now instead of hiding it. NN/g `empty-nav-state`: an + * unavailable destination should say why, and on a default single-tenant proxy only nine of + * the seventeen tabs are open, so silently hiding the rest makes the product look half its + * size and reads as a broken build. A locked tab keeps its place and stays focusable. + */ +function lockTab(tab, why) { + if (why) { tab.setAttribute('aria-disabled', 'true'); tab.dataset.why = why; } else { tab.removeAttribute('aria-disabled'); delete tab.dataset.why; } +} + +/** renderTabNote is the one line under a group's tabs naming what is locked and why. All + * current locks share one reason, so the note states it once; give it a per-reason split + * the day a second reason exists. */ +function renderTabNote(panel) { + const note = $('#tab-note'); + const locked = panel ? $$('.tab[aria-disabled="true"]', panel) : []; + note.hidden = !locked.length; + if (!locked.length) return; + const names = locked.map((t) => t.textContent); + const list = names.length > 1 + ? names.slice(0, -1).join(', ') + ' and ' + names[names.length - 1] + ' need' + : names[0] + ' needs'; + note.textContent = list + ' ' + locked[0].dataset.why + '.'; +} + +/** + * navKeys is arrow-key movement inside a tablist, with MANUAL activation: the arrows move + * focus and Enter/Space (the button's own behaviour) opens it. Automatic activation is the + * pattern's default, but every tab here fires a data fetch, so scrubbing across Admin's + * seven tabs would issue seven queries nobody asked for. + */ +function navKeys(ev) { + const from = ev.target.closest('[role="tab"]'); + if (!from) return; + const tabs = $$('[role="tab"]', ev.currentTarget).filter((t) => !t.hidden); + const i = tabs.indexOf(from); + const step = { ArrowRight: 1, ArrowDown: 1, ArrowLeft: -1, ArrowUp: -1 }[ev.key]; + let j = -1; + if (step) j = (i + step + tabs.length) % tabs.length; + else if (ev.key === 'Home') j = 0; + else if (ev.key === 'End') j = tabs.length - 1; + if (j < 0 || j === i) return; + ev.preventDefault(); + tabs[j].focus(); +} + /** * DIMS is every filter dimension, and it is the single list the whole filter layer * reads: the URL, the chips, the facet dropdowns and the "why is this empty" copy. @@ -4278,11 +4443,14 @@ function go(view, push = true) { if (!$('#gate').hidden) return; if (!Object.prototype.hasOwnProperty.call(loaders, view)) view = 'overview'; // A view whose tab this account is not entitled to is not reachable by typing its - // hash either: its loader would 401/403 and paint an error nobody can act on. - const tab = $(`.tab[data-view="${view}"]`); - if (tab && tab.hidden) view = 'overview'; + // hash either: its loader would 401/403 and paint an error nobody can act on. That + // covers a LOCKED tab as well as a hidden one — the lock says "not with this sign-in", + // and a 403 is not a better way to say it. + const tab = navTab(view); + if (tab && !reachable(tab)) view = 'overview'; state.view = view; - for (const t of $$('.tab')) t.setAttribute('aria-selected', String(t.dataset.view === view)); + state.group = GROUP_OF.get(view) || 'overview'; + syncNav(); for (const s of $$('.view')) s.hidden = s.id !== 'view-' + view; // A filter bar over a view with nothing to filter is thirteen controls inviting clicks // that change nothing — and on Settings it sat directly above a form, so the two read as @@ -4493,7 +4661,34 @@ function urlFor() { // send each other, and Back must close the panel rather than undo a filter change. if (state.drawer && state.drawer.acct) p.set('acct', state.drawer.acct); const q = p.toString(); - return location.pathname + '#' + state.view + (q ? '?' + q : ''); + return location.pathname + '#/' + navPath(state.view) + (q ? '?' + q : ''); +} +/** + * navPath is the canonical hash path for a view: `/`, collapsed to just the + * name when a group is also a view (Overview). So `#/savings/usage`, and `#/overview`. + * + * THE RULE, stated once: the LAST path segment is the view; anything before it is a group + * hint and is ignored whenever the view is known. That is what makes the old one-level + * `#usage` and the new `#/savings/usage` the same link, and it is decidable without a + * lookahead because no view name contains a slash. The leading slash is written, not + * required — `#savings/usage` resolves identically and is rewritten. + */ +function navPath(view) { + const g = GROUP_OF.get(view) || 'overview'; + return g === view ? view : g + '/' + view; +} +/** + * resolveNav turns a hash path into a view name. Every link this dashboard has ever + * written is one segment, including the two the SERVER writes (dash/kvcache.go:510-511), + * so that branch is not a compatibility shim to be removed later — it is half the traffic. + */ +function resolveNav(path) { + const seg = String(path || '').split('/').filter(Boolean); + const view = seg[seg.length - 1] || ''; + if (GROUP_OF.has(view)) return view; + // Only a group name survived (`#/admin`, or `#/savings/typo`): open its first tab. + const g = GROUPS.find(([n]) => n === seg[0]); + return (g && firstView(g[0])) || 'overview'; } function syncURL(replace) { const url = urlFor(); @@ -4502,7 +4697,7 @@ function syncURL(replace) { else history.pushState(null, '', url); } function parseURL() { - const [view, query] = (location.hash || '').replace(/^#/, '').split('?'); + const [path, query] = (location.hash || '').replace(/^#\/?/, '').split('?'); const p = new URLSearchParams(query || ''); const filter = {}; for (const [k] of DIMS) if (p.get(k)) filter[k] = p.get(k); @@ -4515,7 +4710,7 @@ function parseURL() { let from = p.get('from') || (legacy ? 'now-' + legacy + 'ms' : 0); if (legacy) from = legacyFrom(legacy); return { - view: view || 'overview', filter, + view: resolveNav(path), filter, from: numish(from), to: numish(p.get('to') || 'now'), sort: p.get('sort') || '', dir: p.get('dir') === 'asc' ? 'asc' : 'desc', drawer: req ? { req } : diff ? { diff } : acct ? { acct } : null, @@ -4795,7 +4990,21 @@ function initTheme() { function init() { initTheme(); - for (const t of $$('.tab')) t.addEventListener('click', () => go(t.dataset.view)); + // Delegated, not one listener per tab: the three self-mounting views add their tabs + // while this file is still being parsed, so a per-tab loop here would either miss them + // or have to be re-run. A group button opens that group's first usable tab. + $('.groups').addEventListener('click', (ev) => { + const b = ev.target.closest('[data-group]'); + if (b) go(firstView(b.dataset.group) || 'overview'); + }); + $('#subnav').addEventListener('click', (ev) => { + const t = ev.target.closest('.tab'); + // A locked tab is inert; #tab-note beside it already says why, so a click that + // bounced to Overview would be a worse answer than no click at all. + if (t && reachable(t)) go(t.dataset.view); + }); + $('.groups').addEventListener('keydown', navKeys); + $('#subnav').addEventListener('keydown', navKeys); // One control changes one filter. Nothing else in state.filter is touched, which is // what stops a filter with no control (session, tenant) from being wiped — or kept // invisibly — by a change to an unrelated dropdown. @@ -5267,10 +5476,13 @@ function showGate(show) { // form invites clicking things that will 401 — and the TABS did exactly that, each // click firing a data fetch that answered 401 and logged a console error. $('#main').hidden = show; - for (const sel of ['.filters', '.tabs', '.live']) { + for (const sel of ['.filters', '.groups', '.live']) { const n = $(sel); if (n) n.hidden = show; } + // The second level is syncNav's, because whether it is shown at all depends on the open + // group as well as on the gate. + syncNav(); } /** Reflect who is signed in, and which tabs that entitles them to. */ @@ -5287,9 +5499,18 @@ function applyAccount() { // fine on a single-tenant proxy, where there is no principal and nothing to scope: // /api/benchmarks is manager-gated in hosted mode but open locally, and hiding the tab // there would break the local dev path. - for (const el of $$('[data-manager]')) { - el.hidden = account.hosted ? !(t && t.role === 'manager') : !el.hasAttribute('data-local-ok'); - } + // + // In HOSTED mode a non-manager still sees nothing: that is a permissions boundary over + // somebody else's data. In single-tenant local mode there is no other tenant to protect, + // so the manager-only tabs are LOCKED rather than hidden — see lockTab. + for (const b of $$('[data-manager]')) { + const ok = account.hosted ? !!(t && t.role === 'manager') : b.hasAttribute('data-local-ok'); + b.hidden = account.hosted && !ok; + lockTab(b, ok || account.hosted ? '' : 'a manager sign-in'); + } + // data-account tabs stay silently hidden. A signed-out viewer has no use for a tab they + // cannot enable from here, which is the whole difference from the manager case above. + syncNav(); loadTenantOptions(); } function isManager() { return !!(account.tenant && account.tenant.role === 'manager'); } diff --git a/dash/ui/campaigns.js b/dash/ui/campaigns.js index 07539b9d..735c363a 100644 --- a/dash/ui/campaigns.js +++ b/dash/ui/campaigns.js @@ -15,20 +15,17 @@ 'use strict'; // ── mount ────────────────────────────────────────────────────────────────── -// Right after Strategies: a campaign is a bulk way to create the same rows that tab -// edits by hand. -(function mountCampaignsTab() { - const tabs = $('.tabs'); - const tab = el('button', { - role: 'tab', class: 'tab', 'data-view': 'campaigns', 'data-testid': 'tab-campaigns', - 'data-manager': '', hidden: 'hidden', 'aria-selected': 'false', - }, 'Campaigns'); - const after = $('.tab[data-view="strategies"]', tabs); - tabs.insertBefore(tab, after ? after.nextSibling : null); -})(); - -const campView = el('section', { class: 'view', id: 'view-campaigns', hidden: 'hidden' }); -$('#main').appendChild(campView); +// Savings, right after Usage. A campaign is a bulk way to create the rows the Strategies +// tab edits by hand, which is why it used to sit next to it — but what a reader comes here +// to see is money it did or did not save, so it belongs beside the evidence for the +// headline number rather than beside the editor. Still manager-only. +// +// mountTab (app.js) is the single place that knows the nav's DOM shape; it returns the +// section, already appended to #main and wired as this tab's tabpanel. +const campView = mountTab({ + group: 'savings', after: 'usage', view: 'campaigns', label: 'Campaigns', + manager: true, +}); // ── local state ──────────────────────────────────────────────────────────── // Its own object, not app.js's shared filter state: this view is in UNFILTERED_VIEWS, diff --git a/dash/ui/index.html b/dash/ui/index.html index e28f9897..ce177878 100644 --- a/dash/ui/index.html +++ b/dash/ui/index.html @@ -10,26 +10,34 @@ + +

context-guru

-
+ + +
+
-
+
-