diff --git a/CHANGELOG.md b/CHANGELOG.md index ba2b6854..d394c450 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,18 @@ auto-generated per-PR notes; this file is the curated, human-readable history. on `.sidebar`'s new `container-type: inline-size`) restores the full presentation the instant the sidebar widens past the threshold, with no JS state to reconcile. +- **Dashboard tree rows now use one consistent column layout for counts and + actions, and Add panel moved to the Panels row** (#553). The Dashboard row's + panel count now renders as the same inline `· N` right after the label that + Variables/Panels already used, instead of competing right-aligned text next + to the action cluster; the rightmost column is actions only. The Add panel + (`+`) control moved from the Dashboard row to the Panels group row, since it + creates a member of that group. #552's narrow-sidebar container query + (`@container sidebar (max-width: 220px)`, reused verbatim — no second + breakpoint) now also hides the Dashboard tree's `· N` counts, recovering + space for the Dashboard/Variables/Panels labels; the count stays in each + row's accessible name even while visually hidden, and every row's + expand/collapse and direct actions remain reachable by mouse and keyboard. ## [0.7.2] - 2026-07-29 diff --git a/src/application/dashboard-tree-model.ts b/src/application/dashboard-tree-model.ts index 71f976cd..a74a299a 100644 --- a/src/application/dashboard-tree-model.ts +++ b/src/application/dashboard-tree-model.ts @@ -198,11 +198,18 @@ export interface DashboardTreeRow { parentKey: string | null; label: string; /** An inline count rendered right after the label (`Variables · 3`), matching - * the lower switcher's `.side-count` treatment. */ + * the lower switcher's `.side-count` treatment. #553 gave the Dashboard row + * this same field for its panel count (`On-time flights · 7`) rather than + * the right-aligned `meta` it used before, so all three of Dashboard, + * Variables and Panels use ONE placement — and the narrow-sidebar + * breakpoint (`@container sidebar (max-width: 220px)`, `.dash-tree-count`) + * hides it uniformly across the three, never leaving one behind. */ count: number | null; - /** Right-aligned trailing text — the Dashboard row's panel count, or a - * variable row's type(s) (`String`, `String | UInt64`, or nothing at all for - * an orphan with no `lastKnownType`). */ + /** Right-aligned trailing text — a variable row's type(s) (`String`, + * `String | UInt64`, or nothing at all for an orphan with no + * `lastKnownType`). #553 moved the Dashboard row's own panel count OUT of + * this field and into `count` above, so a Dashboard row's `meta` is now + * always `''`. */ meta: string; expandable: boolean; expanded: boolean; @@ -233,7 +240,9 @@ export interface DashboardTreeRow { * * This replaced three separate expressions of the same idea: `renamable` * (#429 phase 3's Dashboard pencil), `deletable` (#447's orphaned-variable - * trash) and the `menu` list. A group row has none. + * trash) and the `menu` list. A group row has none, EXCEPT the Panels group + * row: #553 moved `add-panel` off the Dashboard row onto it, since Add panel + * creates a member of that group rather than acting on the Dashboard itself. */ actions: readonly DashboardTreeAction[]; dashboardId: string; @@ -478,6 +487,25 @@ const unavailableAction = ( * everywhere else. */ const quoted = (name: string): string => '“' + name + '”'; +/** + * The Add-panel control — #553 moved it off the Dashboard row onto the Panels + * GROUP row, since it creates a member of that group rather than acting on + * the Dashboard document itself. Availability is unchanged from before the + * move: an ambiguous Dashboard id or the 100-tile ceiling still render it, + * disabled, rather than withholding it outright (#494's "vocabulary must not + * silently shrink"). + */ +const addPanelAction = ( + dashboardId: string, dashboardLabel: string, tileCount: number, dashboardIdDuplicated: boolean, +): DashboardTreeAction => { + const label = 'Add panel to ' + dashboardLabel; + if (dashboardIdDuplicated) return unavailableAction('add-panel', label, AMBIGUOUS_DASHBOARD_REASON); + if (tileCount >= PORTABLE_LIMITS.maxTilesPerDashboard) { + return unavailableAction('add-panel', label, DASHBOARD_TILE_LIMIT_REASON); + } + return action('add-panel', label, 'Add panel', { kind: 'dashboard', dashboardId }); +}; + export function deriveDashboardTree( { workspace, surface, ui }: DashboardTreeInput, ): DashboardTree { @@ -645,10 +673,14 @@ export function deriveDashboardTree( level: 1, parentKey: null, label: title || UNTITLED_DASHBOARD, - count: null, - // Variables are Dashboard-level controls and are deliberately NOT counted - // here — this is the PANEL count. - meta: String(tiles.length), + // #553: the Dashboard row's own panel count, in the SAME inline `· N` + // placement as the Variables/Panels group rows below — it used to be the + // right-aligned `meta` text, competing with the action cluster for space + // and surviving the narrow-sidebar breakpoint that hides every other + // count. Variables are Dashboard-level controls and are deliberately NOT + // counted here — this is the PANEL count. + count: tiles.length, + meta: '', expandable: true, expanded: dashboardExpanded, toggleable: !dashboardForced, @@ -657,9 +689,11 @@ export function deriveDashboardTree( invalid: null, severity: null, diagnostic: null, - // #494/#515: the Dashboard row's own three direct controls. Its `⋯` menu is + // #494/#515/#553: the Dashboard row's own two direct controls — Add panel + // moved to the Panels group row below, since it creates a member of that + // group rather than acting on the Dashboard document. Its `⋯` menu is // gone — *Open in Edit* was its last item, and a menu button that opens - // a one-item menu beside two real controls is chrome, not vocabulary. + // a one-item menu beside a real control is chrome, not vocabulary. // Shift-click / Shift+Enter remain the Edit gesture (`shift` below). // // A duplicated Dashboard id leaves both unavailable: `findDashboardStrict` @@ -668,19 +702,12 @@ export function deriveDashboardTree( // commit would only refuse is the exact bug this closes. actions: dashboardIdDuplicated ? [ - unavailableAction('add-panel', 'Add panel to ' + (title || UNTITLED_DASHBOARD), - AMBIGUOUS_DASHBOARD_REASON), unavailableAction('edit-dashboard', 'Edit dashboard ' + (title || UNTITLED_DASHBOARD), AMBIGUOUS_DASHBOARD_REASON), unavailableAction('delete-dashboard', 'Delete dashboard ' + (title || UNTITLED_DASHBOARD), AMBIGUOUS_DASHBOARD_REASON), ] : [ - ...(tiles.length >= PORTABLE_LIMITS.maxTilesPerDashboard - ? [unavailableAction('add-panel', 'Add panel to ' + (title || UNTITLED_DASHBOARD), - DASHBOARD_TILE_LIMIT_REASON)] - : [action('add-panel', 'Add panel to ' + (title || UNTITLED_DASHBOARD), - 'Add panel', { kind: 'dashboard', dashboardId: dashboard.id })]), action('edit-dashboard', 'Edit dashboard ' + (title || UNTITLED_DASHBOARD), 'Edit dashboard title & description', { kind: 'dashboard', dashboardId: dashboard.id }), action('delete-dashboard', 'Delete dashboard ' + (title || UNTITLED_DASHBOARD), @@ -882,7 +909,14 @@ export function deriveDashboardTree( invalid: null, severity: null, diagnostic: null, - actions: [], + // #553: the Panels group row is the only group row with a direct + // action — Add panel, moved here from the Dashboard row because it + // creates a member of THIS group. Variables offers nothing: a + // variable is inferred or configured through its own row, never added + // from the group. + actions: group === 'panels' + ? [addPanelAction(dashboard.id, title || UNTITLED_DASHBOARD, tiles.length, dashboardIdDuplicated)] + : [], dashboardId: dashboard.id, member: null, queryId: null, diff --git a/src/styles.css b/src/styles.css index b5ecc332..7b507d06 100644 --- a/src/styles.css +++ b/src/styles.css @@ -1013,10 +1013,17 @@ h1, h2, h3, h4, h5, h6 { its separator dot share one text node, so hiding the element hides both). Widening the sidebar past 220px restores the icon/count with no JS state — see `.sidebar`'s container-type/-name declaration above, alongside - `.query-host`/`.dashboard-host`. */ + `.query-host`/`.dashboard-host`. + #553 reuses this SAME axis for the Dashboard tree rows below + (`.dash-tree-count`, Dashboard/Variables/Panels' `· N`) rather than adding a + second container or threshold — one narrow-sidebar breakpoint, one place it + is declared. Hiding it is purely visual: `rowAccessibleName` in + `ui/dashboard-tree.ts` sets the row's `aria-label` explicitly, so a hidden + count never drops out of what a screen reader announces. */ @container sidebar (max-width: 220px) { .side-tab svg { display: none; } .side-tab .side-count { display: none; } + .dash-tree-count { display: none; } } .history-row { position: relative; diff --git a/tests/e2e/dashboard-tree.spec.js b/tests/e2e/dashboard-tree.spec.js index 92a078e5..55ee127b 100644 --- a/tests/e2e/dashboard-tree.spec.js +++ b/tests/e2e/dashboard-tree.spec.js @@ -310,9 +310,11 @@ test.describe('Dashboard hierarchy tree', () => { // #472: "focus styling must visibly distinguish the disclosure control from the // navigation target from the trailing action". happy-dom can see none of this, and // `:focus-visible` only applies under real keyboard modality — so it has to be a - // real Tab walk in a real browser. Which doubles as proof that all five targets - // (#515 added the plus before the rename pencil) are keyboard-reachable, in row order, - // within ONE composite tab stop. + // real Tab walk in a real browser. Which doubles as proof that all four targets + // are keyboard-reachable, in row order, within ONE composite tab stop. + // + // #553 moved Add panel off this row onto the Panels group row (see the + // dedicated test below), so the Dashboard row's own cluster is pencil, trash. test('Tab walks the row, its chevron and its trailing actions, each ringed differently', async ({ page }) => { await open(page); await roleTab(page, 'Dashboards').click(); @@ -336,17 +338,14 @@ test.describe('Dashboard hierarchy tree', () => { await page.keyboard.press('Tab'); const chevron = await ring(); await page.keyboard.press('Tab'); - const plus = await ring(); - await page.keyboard.press('Tab'); const pencil = await ring(); await page.keyboard.press('Tab'); const trash = await ring(); - // #515: the cluster is plus, pencil, trash — destructive last — and Tab - // reaches all three inside the one composite tab stop, in paint order. - expect([row.what, chevron.what, plus.what, pencil.what, trash.what]).toEqual([ - 'workspace:sales', 'chevron', 'Add panel to Sales revenue', - 'Edit dashboard Sales revenue', 'Delete dashboard Sales revenue', + // The cluster is pencil, trash — destructive last — and Tab reaches both + // inside the one composite tab stop, in paint order. + expect([row.what, chevron.what, pencil.what, trash.what]).toEqual([ + 'workspace:sales', 'chevron', 'Edit dashboard Sales revenue', 'Delete dashboard Sales revenue', ]); // The row rings with a box-shadow and no outline; the chevron with an outline and // no shadow. Different channels, so neither reads as the other — and neither @@ -355,7 +354,6 @@ test.describe('Dashboard hierarchy tree', () => { expect(row.outline).toBe('none'); expect(chevron.outline).toContain('solid'); expect(chevron.shadow).toBe('none'); - expect(plus.outline).not.toBe(chevron.outline); expect(pencil.outline).not.toBe(chevron.outline); expect(trash.outline).not.toBe(chevron.outline); // Nothing was opened or expanded by walking the row. @@ -363,6 +361,30 @@ test.describe('Dashboard hierarchy tree', () => { await expect(page.locator('.dash-tree-row')).toHaveCount(3); }); + // #553: Add panel's new home. The Panels group row's own composite tab stop + // is chevron then plus — proof the move did not strand it from the keyboard. + test('Tab walks the Panels group row to its own chevron and Add panel action', async ({ page }) => { + await open(page); + await roleTab(page, 'Dashboards').click(); + await treeRow(page, 'workspace:sales').locator('.dash-tree-chev').click(); + const panels = treeRow(page, 'workspace:sales:group:panels'); + await panels.focus(); + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('ArrowUp'); + await expect(panels).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(panels.locator('.dash-tree-chev')).toBeFocused(); + await page.keyboard.press('Tab'); + const plus = panels.locator('.dash-tree-act[aria-label="Add panel to Sales revenue"]'); + await expect(plus).toBeFocused(); + await expect(plus).toHaveAttribute('aria-haspopup', 'dialog'); + // Reachable by mouse too: click opens the dialog, matching the keyboard path. + await page.keyboard.press('Enter'); + await expect(page.getByRole('dialog', { name: 'Add panel' })).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog', { name: 'Add panel' })).toBeHidden(); + }); + test('a variable row opens its variable tab immediately and never a query', async ({ page }) => { await open(page); await roleTab(page, 'Dashboards').click(); @@ -857,13 +879,13 @@ test.describe('direct row actions (#494)', () => { await open(page); await roleTab(page, 'Dashboards').click(); // Tab to the pencil the way a keyboard user reaches it — row, chevron, - // plus, pencil — rather than calling `.click()`, which is what let the #495 - // review defect through: the tree's own Enter handler runs on the LIST and - // would otherwise navigate instead. + // pencil (#553 moved Add panel off this row onto the Panels group row) — + // rather than calling `.click()`, which is what let the #495 review defect + // through: the tree's own Enter handler runs on the LIST and would + // otherwise navigate instead. await treeRow(page, 'workspace:sales').focus(); await page.keyboard.press('Tab'); await page.keyboard.press('Tab'); - await page.keyboard.press('Tab'); await expect(treeRow(page, 'workspace:sales') .getByRole('button', { name: 'Edit dashboard Sales revenue' })).toBeFocused(); await page.keyboard.press('Enter'); @@ -879,7 +901,6 @@ test.describe('direct row actions (#494)', () => { await treeRow(page, 'workspace:sales').focus(); await page.keyboard.press('Tab'); await page.keyboard.press('Tab'); - await page.keyboard.press('Tab'); await page.keyboard.press('Space'); await expect(page.getByRole('dialog', { name: 'Edit dashboard' })).toBeVisible(); expect(await page.evaluate(() => window.__opened)).toEqual([]); @@ -1032,7 +1053,9 @@ test.describe('direct row actions (#494)', () => { rowOverflow: el.scrollWidth - list.clientWidth, }; }); - expect(box.actCount).toBe(3); + // #553: Add panel moved to the Panels group row, so a Dashboard row's own + // cluster is pencil + trash. + expect(box.actCount).toBe(2); expect(box.lines).toBeLessThanOrEqual(24); expect(box.clipped).toBe(true); expect(box.overlap).toBe(false); @@ -1066,3 +1089,97 @@ test.describe('direct row actions (#494)', () => { expect(landed).toMatch(/:tile:t-rev$/); }); }); + +// #553 — Dashboard, Variables and Panels counts share ONE inline `· N` +// placement, and the narrow-sidebar breakpoint (#552's `@container sidebar +// (max-width: 220px)`, reused verbatim — no second container or threshold) +// hides all three uniformly. happy-dom cannot see any of this: the container +// query is real CSS layout, and the drag that reaches it needs a real +// `.col-resize` pointer sequence (`src/ui/splitters.ts`'s `dragValue('col', +// ev)` reads `ev.clientX` directly), the same pattern `sidebar-tabs-narrow.spec.js` +// (#552) uses. +test.describe('Dashboard tree counts at the narrow sidebar (#553)', () => { + /** Drag `.col-resize` to `targetX` the way a real user does. */ + const dragSidebarTo = async (page, targetX) => { + const handle = page.locator('.col-resize'); + const box = await handle.boundingBox(); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); + await page.mouse.down(); + await page.mouse.move(targetX, box.y + box.height / 2, { steps: 5 }); + await page.mouse.up(); + }; + const expandSales = async (page) => { + await roleTab(page, 'Dashboards').click(); + await treeRow(page, 'workspace:sales').locator('.dash-tree-chev').click(); + await treeRow(page, 'workspace:sales:group:variables').click(); + await treeRow(page, 'workspace:sales:group:panels').click(); + }; + + test('the wide (default) sidebar shows Dashboard, Variables and Panels counts inline after the label', async ({ page }) => { + await open(page); + await expandSales(page); + const countText = (key) => treeRow(page, key).locator('.dash-tree-count').textContent(); + await expect.poll(() => countText('workspace:sales')).toBe('· 2'); + await expect.poll(() => countText('workspace:sales:group:variables')).toBe('· 2'); + await expect.poll(() => countText('workspace:sales:group:panels')).toBe('· 2'); + for (const key of ['workspace:sales', 'workspace:sales:group:variables', 'workspace:sales:group:panels']) { + await expect(treeRow(page, key).locator('.dash-tree-count')).toBeVisible(); + } + }); + + test('dragging to <=220px hides every dot/count, but the count stays in the accessible name and actions stay reachable', async ({ page }) => { + await open(page); + await expandSales(page); + await dragSidebarTo(page, 200); + await expect.poll(() => page.locator('.sidebar').evaluate((el) => el.getBoundingClientRect().width)) + .toBeLessThanOrEqual(220); + + for (const key of ['workspace:sales', 'workspace:sales:group:variables', 'workspace:sales:group:panels']) { + await expect(treeRow(page, key).locator('.dash-tree-count')).toBeHidden(); + } + // Hidden from sight only: `rowAccessibleName` sets the row's `aria-label` + // explicitly, so the count a sighted user no longer sees is still what a + // screen reader announces. + const tree = page.getByRole('tree', { name: 'Dashboards' }); + await expect(tree.getByRole('treeitem', { name: 'Sales revenue 2', exact: true })).toHaveCount(1); + await expect(tree.getByRole('treeitem', { name: 'Variables 2', exact: true })).toHaveCount(1); + await expect(tree.getByRole('treeitem', { name: 'Panels 2', exact: true })).toHaveCount(1); + + // The label recovered the space rather than being crushed, and Add panel — + // its new home on the Panels row — is still reachable by both mouse and + // keyboard at this width. + const panelsRow = treeRow(page, 'workspace:sales:group:panels'); + const label = await panelsRow.locator('.label').boundingBox(); + expect(label.width).toBeGreaterThan(0); + const plus = panelsRow.locator('.dash-tree-act[aria-label="Add panel to Sales revenue"]'); + await plus.focus(); + await expect(plus).toBeFocused(); + await page.keyboard.press('Enter'); + await expect(page.getByRole('dialog', { name: 'Add panel' })).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog', { name: 'Add panel' })).toBeHidden(); + + // Expand/collapse by mouse still works too. + const chev = treeRow(page, 'workspace:sales').locator('.dash-tree-chev'); + await expect(chev).toHaveAttribute('aria-expanded', 'true'); + await chev.click(); + await expect(chev).toHaveAttribute('aria-expanded', 'false'); + await expect(treeRow(page, 'workspace:sales:group:panels')).toHaveCount(0); + }); + + test('widening the sidebar back past 220px restores every count', async ({ page }) => { + await open(page); + await expandSales(page); + await dragSidebarTo(page, 200); + await expect.poll(() => page.locator('.sidebar').evaluate((el) => el.getBoundingClientRect().width)) + .toBeLessThanOrEqual(220); + + await dragSidebarTo(page, 300); + await expect.poll(() => page.locator('.sidebar').evaluate((el) => el.getBoundingClientRect().width)) + .toBeGreaterThan(220); + + for (const key of ['workspace:sales', 'workspace:sales:group:variables', 'workspace:sales:group:panels']) { + await expect(treeRow(page, key).locator('.dash-tree-count')).toBeVisible(); + } + }); +}); diff --git a/tests/e2e/tile-open-workbench.spec.js b/tests/e2e/tile-open-workbench.spec.js index e653858a..cf27778c 100644 --- a/tests/e2e/tile-open-workbench.spec.js +++ b/tests/e2e/tile-open-workbench.spec.js @@ -189,15 +189,19 @@ test('the action is keyboard reachable and activates on Enter', async ({ page }) expect((await tabs(page)).at(-1)).toMatchObject({ savedId: 'q-sales', active: true }); }); -test('a Dashboard-row plus creates a blank linked Panel and focuses its SQL editor', async ({ page }) => { +// #553: Add panel moved off the Dashboard row onto the Panels group row — it +// creates a member of that group rather than acting on the Dashboard itself. +test('the Panels-row plus creates a blank linked Panel and focuses its SQL editor', async ({ page }) => { await open(page); // Begin on the actual Dashboard surface. A successful mutation must not // rerender that surface (which force-closes overlays) before the dialog can // close and perform its reveal/open/focus settlement. await openDashboard(page, 'sales'); await expect.poll(() => surface(page)).toBe('dashboard'); - const dashboard = treeRow(page, 'workspace:sales'); - const plus = dashboard.locator('.dash-tree-act[aria-label="Add panel to Sales"]'); + // The Panels group row only paints once the Dashboard is expanded. + await treeRow(page, 'workspace:sales').locator('.dash-tree-chev').click(); + const panelsGroup = treeRow(page, 'workspace:sales:group:panels'); + const plus = panelsGroup.locator('.dash-tree-act[aria-label="Add panel to Sales"]'); // The direct action is a real keyboard target, revealed by focus just like // the adjacent pencil and trash. Focus it explicitly: browser tab order also diff --git a/tests/unit/dashboard-tree-model.test.ts b/tests/unit/dashboard-tree-model.test.ts index 29754ab8..3815e3c9 100644 --- a/tests/unit/dashboard-tree-model.test.ts +++ b/tests/unit/dashboard-tree-model.test.ts @@ -52,7 +52,9 @@ const variableRows = (rows: readonly DashboardTreeRow[]): DashboardTreeRow[] => rows.filter((candidate) => candidate.kind === 'variable'); describe('deriveDashboardTree — collection and ordering', () => { - it('renders Dashboards in array order, collapsed, with the panel count at the right', () => { + // #553: the Dashboard row's panel count is the SAME inline `· N` placement + // (`count`) as Variables/Panels, not the right-aligned `meta` it used before. + it('renders Dashboards in array order, collapsed, with the panel count inline after the label', () => { const tree = derive(ws({ dashboards: [ dashboard({ id: 'b', title: 'Beta', tiles: [{ id: 't1', queryId: 'q1' }, { id: 't2', queryId: 'q1' }] }), @@ -61,7 +63,10 @@ describe('deriveDashboardTree — collection and ordering', () => { queries: [query('q1', 'Q1')], })); expect(labels(tree.rows)).toEqual(['Beta', 'Alpha']); - expect(tree.rows.map((r) => r.meta)).toEqual(['2', '0']); + expect(tree.rows.map((r) => r.count)).toEqual([2, 0]); + // The right-aligned `meta` slot is empty — #553 reserves it for a + // variable's type(s) only, never a Dashboard row's own count. + expect(tree.rows.map((r) => r.meta)).toEqual(['', '']); expect(tree.rows.every((r) => r.level === 1 && r.parentKey === null && r.expandable)).toBe(true); expect(tree.dashboardCount).toBe(2); }); @@ -75,7 +80,7 @@ describe('deriveDashboardTree — collection and ordering', () => { })], queries: [query('q1', 'Q1')], })); - expect(row(tree.rows, 'w1:d').meta).toBe('1'); + expect(row(tree.rows, 'w1:d').count).toBe(1); }); it('keys rows by workspace + Dashboard + member id, never by index or label', () => { @@ -603,8 +608,7 @@ describe('deriveDashboardTree — search', () => { expect(dash.single).toMatchObject({ kind: 'open-dashboard', request: { mode: 'view' } }); expect(dash.shift).toMatchObject({ kind: 'open-dashboard', request: { mode: 'edit' } }); // The search forcing expansion open does not touch the row's OWN actions. - expect(dash.actions.map((a) => a.kind)) - .toEqual(['add-panel', 'edit-dashboard', 'delete-dashboard']); + expect(dash.actions.map((a) => a.kind)).toEqual(['edit-dashboard', 'delete-dashboard']); }); it('every row is toggleable again once the search clears', () => { @@ -705,14 +709,29 @@ describe('deriveDashboardTree — command sets', () => { // Both requests name the Dashboard alone: a Dashboard row focuses no member. expect('focus' in (dash.single as { request: object }).request).toBe(false); // #494 removed the `⋯` menu — *Open in Edit* was its last item, and Shift - // (asserted above via `shift`) is still how Edit is reached. The row's - // vocabulary is now its three direct actions, and nothing else mirrors a menu. + // (asserted above via `shift`) is still how Edit is reached. #553 moved + // Add panel to the Panels group row, so the Dashboard row's own vocabulary + // is now its two direct actions, and nothing else mirrors a menu. expect(dash.actions.map((a) => a.kind)) - .toEqual(['add-panel', 'edit-dashboard', 'delete-dashboard']); + .toEqual(['edit-dashboard', 'delete-dashboard']); }); - it('gives a group row ONLY a toggle — no double, no Shift, no actions', () => { - const group = row(tree().rows, 'w1:d:group:panels'); + // #553: Add panel now lives on the Panels group row — it creates a member of + // THAT group, not an operation on the Dashboard document. + it('gives the Panels group row an add-panel action too, but Variables and a group\'s toggle stay untouched', () => { + const rows = tree().rows; + const panels = row(rows, 'w1:d:group:panels'); + expect(panels.actions.map((a) => a.kind)).toEqual(['add-panel']); + expect(panels.single).toEqual({ kind: 'toggle' }); + expect(panels.double).toBeNull(); + expect(panels.shift).toBeNull(); + expect(row(rows, 'w1:d:group:variables').actions).toEqual([]); + }); + + it('gives a group row ONLY a toggle — no double, no Shift', () => { + // Variables, not Panels: #553 gave Panels its own add-panel action, so + // this is the one group row with NO actions at all. + const group = row(tree().rows, 'w1:d:group:variables'); expect(group.single).toEqual({ kind: 'toggle' }); expect(group.double).toBeNull(); expect(group.shift).toBeNull(); @@ -806,50 +825,55 @@ describe('deriveDashboardTree — direct actions (#494)', () => { ]); }); - it('orders and names the Dashboard row add / edit / delete controls', () => { + it('orders and names the Dashboard row edit / delete controls', () => { const tree = derive(ws({ dashboards: [dashboard({ id: 'd', title: 'D' })] })); const dash = row(tree.rows, 'w1:d'); - expect(dash.actions.map((a) => a.kind)) - .toEqual(['add-panel', 'edit-dashboard', 'delete-dashboard']); - expect(dash.actions.map((a) => a.label)) - .toEqual(['Add panel to D', 'Edit dashboard D', 'Delete dashboard D']); - expect(dash.actions[0]).toMatchObject({ - tooltip: 'Add panel', target: { kind: 'dashboard', dashboardId: 'd' }, - unavailable: null, confirm: null, - }); - expect(dash.actions[2].confirm).toBe('Delete dashboard “D”? This also deletes every query its panels own.'); + expect(dash.actions.map((a) => a.kind)).toEqual(['edit-dashboard', 'delete-dashboard']); + expect(dash.actions.map((a) => a.label)).toEqual(['Edit dashboard D', 'Delete dashboard D']); + expect(dash.actions[1].confirm).toBe('Delete dashboard “D”? This also deletes every query its panels own.'); + }); + + // #553: Add panel moved to the Panels group row. + it('names and resolves the Panels group row\'s add-panel control', () => { + const tree = derive(ws({ dashboards: [dashboard({ id: 'd', title: 'D' })] }), + toggleDashboardExpanded(EMPTY_TREE_UI, 'd')); + const panels = row(tree.rows, 'w1:d:group:panels'); + expect(panels.actions).toEqual([{ + kind: 'add-panel', label: 'Add panel to D', tooltip: 'Add panel', + target: { kind: 'dashboard', dashboardId: 'd' }, unavailable: null, confirm: null, + }]); }); it('keeps Add panel visible but unavailable at the 100-tile limit', () => { const tiles = Array.from({ length: 100 }, (_, i) => ({ id: 't' + i, queryId: 'q' + i })); - const dash = row(derive(ws({ + const panels = row(derive(ws({ dashboards: [dashboard({ id: 'd', title: 'Full', tiles })], - })).rows, 'w1:d'); + }), toggleDashboardExpanded(EMPTY_TREE_UI, 'd')).rows, 'w1:d:group:panels'); - expect(dash.actions.map((action) => action.kind)) - .toEqual(['add-panel', 'edit-dashboard', 'delete-dashboard']); - expect(dash.actions[0]).toMatchObject({ + expect(panels.actions.map((action) => action.kind)).toEqual(['add-panel']); + expect(panels.actions[0]).toMatchObject({ label: 'Add panel to Full', target: null, unavailable: 'This dashboard already has the maximum of 100 panels, so another panel cannot be added.', }); }); - it('falls back to the row\'s own Untitled label in an action name, on both row kinds', () => { + it('falls back to the row\'s own Untitled label in an action name, on every row kind', () => { const tree = derive(ws({ dashboards: [dashboard({ id: 'd', title: ' ', tiles: [{ id: 't1', queryId: 'q1' }] })], queries: [query('q1')], }), allOpen(['d'])); const dash = row(tree.rows, 'w1:d'); + const panels = row(tree.rows, 'w1:d:group:panels'); const panel = row(tree.rows, 'w1:d:tile:t1'); // Same fallback the ROW itself displays — never a second, disagreeing default. expect(dash.label).toBe(UNTITLED_DASHBOARD); expect(panel.label).toBe(UNTITLED_PANEL); expect(dash.actions.map((a) => a.label)).toEqual([ - 'Add panel to ' + UNTITLED_DASHBOARD, 'Edit dashboard ' + UNTITLED_DASHBOARD, 'Delete dashboard ' + UNTITLED_DASHBOARD, ]); + expect(panels.actions.map((a) => a.label)).toEqual(['Add panel to ' + UNTITLED_DASHBOARD]); expect(panel.actions.map((a) => a.label)).toEqual([ 'Edit ' + UNTITLED_PANEL, 'Remove ' + UNTITLED_PANEL + ' from dashboard', ]); @@ -975,8 +999,7 @@ describe('deriveDashboardTree — direct actions (#494)', () => { const dash0 = row(tree.rows, 'w1:d:dup:0'); const dash1 = row(tree.rows, 'w1:d:dup:1'); for (const dash of [dash0, dash1]) { - expect(dash.actions.map((a) => a.kind)) - .toEqual(['add-panel', 'edit-dashboard', 'delete-dashboard']); + expect(dash.actions.map((a) => a.kind)).toEqual(['edit-dashboard', 'delete-dashboard']); for (const a of dash.actions) { expect(a.target).toBeNull(); expect(a.confirm).toBeNull(); @@ -987,6 +1010,14 @@ describe('deriveDashboardTree — direct actions (#494)', () => { // answers here. expect(dash.dropTarget).toBeNull(); } + // #553: the ambiguity reaches the Panels group row's add-panel too — it + // is addressed by `dashboardId` alone, same as the pencil/trash above. + for (const key of ['w1:d:dup:0:group:panels', 'w1:d:dup:1:group:panels']) { + const panelsGroup = row(tree.rows, key); + expect(panelsGroup.actions.map((a) => a.kind)).toEqual(['add-panel']); + expect(panelsGroup.actions[0].target).toBeNull(); + expect(panelsGroup.actions[0].unavailable).toContain('share this id'); + } // The ambiguity cascades to the panels underneath: which Dashboard "d" // even is has no answer, so its tiles cannot be resolved either — and the // panel row's own key inherits dash0's `:dup:0` disambiguation rather @@ -1038,12 +1069,15 @@ describe('deriveDashboardTree — direct actions (#494)', () => { }); }); - it('gives no actions to a group row or an ACTIVE (non-orphaned) variable row', () => { + // #553: the Panels group row now carries its own add-panel action, so this + // is no longer true of every group row — only Variables, and an active + // variable, offer nothing. + it('gives no actions to the Variables group row or an ACTIVE (non-orphaned) variable row', () => { const tree = derive(ws({ dashboards: [dashboard({ id: 'd', title: 'D', tiles: [{ id: 't1', queryId: 'q1' }] })], queries: [query('q1', 'Q1', undefined, 'SELECT 1 WHERE c = {country:String}')], }), allOpen(['d'])); - expect(row(tree.rows, 'w1:d:group:panels').actions).toEqual([]); + expect(row(tree.rows, 'w1:d:group:panels').actions.map((a) => a.kind)).toEqual(['add-panel']); expect(row(tree.rows, 'w1:d:group:variables').actions).toEqual([]); expect(row(tree.rows, 'w1:d:variable:country').actions).toEqual([]); }); diff --git a/tests/unit/dashboard-tree.test.ts b/tests/unit/dashboard-tree.test.ts index fdd60579..5a919544 100644 --- a/tests/unit/dashboard-tree.test.ts +++ b/tests/unit/dashboard-tree.test.ts @@ -175,9 +175,11 @@ describe('renderDashboardTree — structure and ARIA', () => { expect(tabbableChevrons[0].closest('.dash-tree-row')!.getAttribute('data-key')).toBe('w1:sales'); // WebKit needs explicit tabindex values on the hover-concealed actions; // they must rove with their row rather than create stops for every row. + // #553: the Dashboard row's own vocabulary is edit + delete (Add panel + // moved to the Panels group row), so its own tabbable cluster is 2. const tabbableActions = [...list.querySelectorAll('.dash-tree-act')] .filter((action) => action.getAttribute('tabindex') === '0'); - expect(tabbableActions).toHaveLength(3); + expect(tabbableActions).toHaveLength(2); expect(tabbableActions.every((action) => action.closest('.dash-tree-row')?.dataset.key === 'w1:sales')) .toBe(true); }); @@ -709,8 +711,10 @@ describe('renderDashboardTree — the disclosure control (#472)', () => { expect(name('w1:sales')).toBe('Sales 2'); expect(name('w1:sales')).not.toContain('Expand'); expect(name('w1:sales')).not.toContain('Actions for'); - // Group row: the count is announced, the disclosure verb is not. + // Group row: the count is announced, the disclosure verb is not — the same + // placement as the Dashboard row above (#553), on Variables AND Panels. expect(name('w1:sales:group:variables')).toBe('Variables 2'); + expect(name('w1:sales:group:panels')).toBe('Panels 2'); // Everything that was announced before still is: the status WORD, the type meta // and the marker's severity label. expect(name('w1:sales:variable:region')).toBe('region unused String Unused'); @@ -719,12 +723,15 @@ describe('renderDashboardTree — the disclosure control (#472)', () => { // The chevron and every trailing control keep their own, distinct names // (`openAll` expanded this row, so its verb is Collapse). expect(chevron(list, 'w1:sales').getAttribute('aria-label')).toBe('Collapse Sales'); - expect(actionNames(list, 'w1:sales')) - .toEqual(['Add panel to Sales', 'Edit dashboard Sales', 'Delete dashboard Sales']); + // #553: Add panel moved to the Panels group row, so the Dashboard row's own + // vocabulary is now edit + delete only. + expect(actionNames(list, 'w1:sales')).toEqual(['Edit dashboard Sales', 'Delete dashboard Sales']); + expect(actionNames(list, 'w1:sales:group:panels')).toEqual(['Add panel to Sales']); // …and none of those names leaks into the row's own. - for (const label of ['Add panel', 'Edit dashboard', 'Delete dashboard', 'Collapse']) { + for (const label of ['Edit dashboard', 'Delete dashboard', 'Collapse']) { expect(name('w1:sales')).not.toContain(label); } + expect(name('w1:sales:group:panels')).not.toContain('Add panel'); expect(name('w1:sales:tile:t1')).toBe('Revenue'); expect(name('w1:sales:variable:region')).not.toContain('Delete the stored option SQL'); }); @@ -738,9 +745,10 @@ describe('renderDashboardTree — the disclosure control (#472)', () => { vi.advanceTimersByTime(400); expect(readTreeUi(app.state.dashboardTreeUi, 'w1').expandedDashboardIds.size).toBe(0); expect(app.openDashboard).not.toHaveBeenCalled(); - // Chevron, row, plus, pencil, trash are independent targets. + // Chevron, row, pencil, trash are independent targets. #553 moved the plus + // to the (unrendered, since this Dashboard is collapsed) Panels group row. expect(row.querySelectorAll('.dash-tree-chev')).toHaveLength(1); - expect(row.querySelectorAll('.dash-tree-act')).toHaveLength(3); + expect(row.querySelectorAll('.dash-tree-act')).toHaveLength(2); vi.useRealTimers(); }); }); @@ -770,23 +778,24 @@ describe('renderDashboardTree — direct row actions (#494)', () => { expect(list.querySelectorAll('[aria-label^="Actions for"]')).toHaveLength(0); }); - it('gives the Dashboard row plus, pencil and trash, in that order', () => { + it('gives the Dashboard row pencil and trash, in that order', () => { const { list } = open(); // Destructive rightmost — never where the pointer lands by habit. - expect(actionNames(list, 'w1:sales')) - .toEqual(['Add panel to Sales', 'Edit dashboard Sales', 'Delete dashboard Sales']); + expect(actionNames(list, 'w1:sales')).toEqual(['Edit dashboard Sales', 'Delete dashboard Sales']); }); - it('gives a Panel row a pencil and a trash that name the panel', () => { + // #553: Add panel moved off the Dashboard row onto the Panels group row — + // it creates a member of that group rather than acting on the Dashboard. + it('gives the Panels group row the plus, and the Variables group row none', () => { const { list } = open(); - expect(actionNames(list, 'w1:sales:tile:t1')) - .toEqual(['Edit Revenue', 'Remove Revenue from dashboard']); + expect(actionNames(list, 'w1:sales:group:panels')).toEqual(['Add panel to Sales']); + expect(actionNames(list, 'w1:sales:group:variables')).toEqual([]); }); - it('gives group rows none', () => { + it('gives a Panel row a pencil and a trash that name the panel', () => { const { list } = open(); - expect(actionNames(list, 'w1:sales:group:panels')).toEqual([]); - expect(actionNames(list, 'w1:sales:group:variables')).toEqual([]); + expect(actionNames(list, 'w1:sales:tile:t1')) + .toEqual(['Edit Revenue', 'Remove Revenue from dashboard']); }); it('makes every control a real, individually named button with a tooltip', () => { @@ -810,7 +819,7 @@ describe('renderDashboardTree — direct row actions (#494)', () => { it('marks the destructive one so it can be styled apart from the pencil', () => { const { list } = open(); - expect(actionBtn(list, 'w1:sales', 'Add panel to Sales')!.classList.contains('dash-tree-act-danger')) + expect(actionBtn(list, 'w1:sales:group:panels', 'Add panel to Sales')!.classList.contains('dash-tree-act-danger')) .toBe(false); expect(actionBtn(list, 'w1:sales', 'Edit dashboard Sales')!.classList.contains('dash-tree-act-danger')) .toBe(false); @@ -830,7 +839,7 @@ describe('renderDashboardTree — direct row actions (#494)', () => { it('paints the plus, pencil and trash glyphs on their actions', () => { const { list } = open(); // Swapping the two icons would otherwise pass every other assertion here. - expect(actionBtn(list, 'w1:sales', 'Add panel to Sales')!.innerHTML) + expect(actionBtn(list, 'w1:sales:group:panels', 'Add panel to Sales')!.innerHTML) .toBe(Icon.plus().outerHTML); expect(actionBtn(list, 'w1:sales', 'Edit dashboard Sales')!.innerHTML) .toBe(Icon.pencil().outerHTML); @@ -1272,11 +1281,14 @@ describe('renderDashboardTree — Add panel (#515)', () => { genId: vi.fn(() => 'new-' + ++id), ...over, }); + // #553: Add panel lives on the Panels group row now, which only paints + // once the Dashboard is expanded. + openAll(fixture.app, 'sales'); renderDashboardTree(fixture.app); return fixture; }; const plus = (list: HTMLElement): HTMLButtonElement => - actionBtn(list, 'w1:sales', 'Add panel to Sales')!; + actionBtn(list, 'w1:sales:group:panels', 'Add panel to Sales')!; const nameInput = (): HTMLInputElement => document.querySelector('#panel-create-name')!; const descInput = (): HTMLTextAreaElement => @@ -1285,10 +1297,10 @@ describe('renderDashboardTree — Add panel (#515)', () => { document.querySelector('.fm-dialog-confirm')!; const settle = (): Promise => new Promise((resolve) => { setTimeout(resolve, 0); }); - it('renders a real dialog button immediately before the pencil', () => { + it('renders a real dialog button on the Panels group row, and only the Dashboard row\'s pencil/trash', () => { const { list } = open(); - expect(actionNames(list, 'w1:sales')) - .toEqual(['Add panel to Sales', 'Edit dashboard Sales', 'Delete dashboard Sales']); + expect(actionNames(list, 'w1:sales:group:panels')).toEqual(['Add panel to Sales']); + expect(actionNames(list, 'w1:sales')).toEqual(['Edit dashboard Sales', 'Delete dashboard Sales']); expect(plus(list).getAttribute('aria-haspopup')).toBe('dialog'); expect(plus(list).getAttribute('data-act')).toBe('add-panel'); expect(plus(list).getAttribute('title')).toBe('Add panel'); @@ -1439,6 +1451,7 @@ describe('renderDashboardTree — Add panel (#515)', () => { id: 't' + i, queryId: 'q' + i, })); const fixture = treeApp({ currentWorkspace: full }); + openAll(fixture.app, 'sales'); renderDashboardTree(fixture.app); const button = plus(fixture.list); expect(button.getAttribute('aria-disabled')).toBe('true');