diff --git a/CHANGELOG.md b/CHANGELOG.md index e45f1465..13b4aef4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -325,8 +325,21 @@ auto-generated per-PR notes; this file is the curated, human-readable history. builders. The Dashboard File trigger now uses the same downward-chevron treatment as Workbench (no more right-arrow that misread as navigation) and its menu gains EXPORT/IMPORT/OPEN section headings, icons, and `.json` - metadata; read-only Dashboards still expose Export only. Actions and - permission rules are unchanged. + metadata; read-only Dashboards expose Export only at this point (superseded + by #347, below). Actions and permission rules are unchanged. +- **Read-only Dashboard views (detached current-workspace and one-time + session-bundle) no longer render a File menu at all** (#347). The prior + read-only behavior above (#331) still showed an Export button, but + `exportDashboardAction` resolves the latest **committed primary** workspace + via `app.workspace.loadCurrent()` — in a read-only route that primary + workspace can be unrelated to the Dashboard actually on screen, so clicking + Export could silently download the wrong (and potentially private) Dashboard + and its queries. `buildDashboardFileMenu` no longer takes a `readOnly` flag; + the caller now omits the button/menu from the DOM entirely — never disabled + or visually hidden — when the resolved route is read-only, keeping keyboard + tab order and the accessibility tree free of an inoperable control. Editable + current-workspace Dashboards are unaffected and keep the full + Export/Import/Open-for-viewing menu. - **Dashboard tile content stays inside the tile body, above the footer** (#331). `.dash-tile-body` gained `overflow:hidden`, and the tall-panel containment contract (`flex:1 1 auto; min-width/min-height:0; max-height:100%; diff --git a/src/ui/dashboard.ts b/src/ui/dashboard.ts index d6b427c7..44b5a90e 100644 --- a/src/ui/dashboard.ts +++ b/src/ui/dashboard.ts @@ -268,17 +268,21 @@ function renderDashboardNotFound(app: DashboardApp): void { * EXPORT ⭳ Export Dashboard… .json * IMPORT ⭱ Import Dashboard… * OPEN ◇ Open for viewing… - * Edit mode offers all three sections; a read-only (detached) view offers - * EXPORT only (import + re-preview are edit-context operations). Every item - * delegates to an `app.actions.*` seam (dashboard.ts never reaches into - * app.ts). The trigger uses the shared downward-chevron treatment - * (`Icon.chevDown()`, matching the Workbench File button) rather than a - * right-pointing arrow, which would misread as navigation. The trigger owns - * its own open/close TOGGLE (unlike the Workbench menu, which only ever - * opens) — clicking it again while open closes the menu and restores focus, - * tracked here via the returned `MenuHandle` rather than a second - * `openMenu` call. */ -function buildDashboardFileMenu(app: DashboardApp, readOnly: boolean): HTMLElement { + * #347: only ever built for an EDITABLE (non-read-only) Dashboard — the + * caller omits the control entirely for a read-only route. (Previously a + * read-only view got an Export-only menu, but `exportDashboardAction` + * resolves the latest COMMITTED PRIMARY workspace via `workspace.loadCurrent()`, + * which may be unrelated to the read-only Dashboard on screen; hiding rows + * wasn't enough, since the surviving Export button still exported the wrong + * workspace.) Every item delegates to an `app.actions.*` seam (dashboard.ts + * never reaches into app.ts). The trigger uses the shared downward-chevron + * treatment (`Icon.chevDown()`, matching the Workbench File button) rather + * than a right-pointing arrow, which would misread as navigation. The + * trigger owns its own open/close TOGGLE (unlike the Workbench menu, which + * only ever opens) — clicking it again while open closes the menu and + * restores focus, tracked here via the returned `MenuHandle` rather than a + * second `openMenu` call. */ +function buildDashboardFileMenu(app: DashboardApp): HTMLElement { const doc = app.document; const btn = h('button', { class: 'dash-btn dash-file-btn', 'aria-haspopup': 'menu', 'aria-expanded': 'false', @@ -294,21 +298,17 @@ function buildDashboardFileMenu(app: DashboardApp, readOnly: boolean): HTMLEleme kind: 'item', icon: Icon.download(), label: 'Export Dashboard…', meta: '.json', extraClass: 'dash-fm-item', onClick: () => app.actions.exportDashboard(), }, + { kind: 'section', label: 'Import' }, + { + kind: 'item', icon: Icon.upload(), label: 'Import Dashboard…', extraClass: 'dash-fm-item', + onClick: () => app.actions.importDashboard(), + }, + { kind: 'section', label: 'Open' }, + { + kind: 'item', icon: Icon.eye(), label: 'Open for viewing…', extraClass: 'dash-fm-item', + onClick: () => app.actions.openDashboardForViewing(), + }, ]; - if (!readOnly) { - rows.push( - { kind: 'section', label: 'Import' }, - { - kind: 'item', icon: Icon.upload(), label: 'Import Dashboard…', extraClass: 'dash-fm-item', - onClick: () => app.actions.importDashboard(), - }, - { kind: 'section', label: 'Open' }, - { - kind: 'item', icon: Icon.eye(), label: 'Open for viewing…', extraClass: 'dash-fm-item', - onClick: () => app.actions.openDashboardForViewing(), - }, - ); - } handle = openMenu({ document: doc, trigger: btn, rows, menuClass: 'dash-file-menu', onClose: () => { handle = null; }, @@ -553,8 +553,10 @@ export async function renderDashboard(app: DashboardApp): Promise { const showLayoutSelect = !readOnly || currentDoc.layout.type === 'grafana-grid'; // #302: the Dashboard page's own resource-scoped File menu (import/export + - // open-for-viewing). - const fileMenuBtn = buildDashboardFileMenu(app, readOnly); + // open-for-viewing). #347: omitted from the DOM entirely in a read-only + // view — not merely disabled/hidden — so keyboard tab order and the + // accessibility tree carry no File control where no operation is valid. + const fileMenuBtn = readOnly ? null : buildDashboardFileMenu(app); const header = h('div', { class: 'dash-header' }, h('a', { class: 'dash-back', href: app.conn.basePath || '/sql', title: 'Back to SQL Browser', 'aria-label': 'Back to SQL Browser', diff --git a/tests/unit/dashboard.test.ts b/tests/unit/dashboard.test.ts index 31f02b72..9f530c4d 100644 --- a/tests/unit/dashboard.test.ts +++ b/tests/unit/dashboard.test.ts @@ -3044,13 +3044,42 @@ describe('renderDashboard — Dashboard header File menu (#302)', () => { expect(document.querySelector('.dash-file-menu')).toBeNull(); }); - it('view mode: the File menu offers Export only (import + re-preview are edit-context)', async () => { + it('editable current-workspace mode includes the File button (#347)', async () => { + const { app } = editApp(); + await render(app); + expect(qs(app.root, '.dash-file-btn')).not.toBeNull(); + }); + + it('detached view mode omits the File button entirely, not just its Import/Open rows (#347)', async () => { const detached = wsWith({ id: 'd', queries: [q('q1', 'SELECT 1')], tiles: [{ id: 't1', queryId: 'q1' }] }); const { app } = modeApp({ workspace: null, detached, openSource: { kind: 'current-workspace', workspaceId: 'w', dashboardId: 'd' } }); await render(app); - openFileMenuBtn(app.root); - expect(menuSections()).toEqual(['Export']); - expect(menuItems()).toEqual(['Export Dashboard…']); + expect(qs(app.root, '.dash-file-btn')).toBeNull(); + expect(document.querySelector('.dash-file-menu')).toBeNull(); + }); + + it('session-bundle read-only mode omits the File button entirely (#347)', async () => { + const detached = wsWith({ id: 'd', queries: [q('q1', 'SELECT 1')], tiles: [{ id: 't1', queryId: 'q1' }] }); + const consume = vi.fn(async () => detached as never); + const { app } = modeApp({ openSource: { kind: 'session-bundle', token: 'tok', dashboardId: 'd' }, consume }); + await render(app); + expect(qs(app.root, '.dash-file-btn')).toBeNull(); + }); + + it('a different primary workspace existing cannot export it from the read-only page (#347)', async () => { + // The primary store holds an UNRELATED dashboard ('other') — resolveDashboardMode + // won't match it against this route's `dashboardId: 'd'`, so it falls through to + // the detached store, same as production when a primary workspace exists but this + // tab is showing someone else's shared/detached Dashboard. + const primary = wsWith({ id: 'other', queries: [q('secret', 'SELECT 2')], tiles: [{ id: 'ts', queryId: 'secret' }] }); + const detached = wsWith({ id: 'd', queries: [q('q1', 'SELECT 1')], tiles: [{ id: 't1', queryId: 'q1' }] }); + const { app } = modeApp({ workspace: primary, detached, openSource: { kind: 'current-workspace', workspaceId: 'w', dashboardId: 'd' } }); + await render(app); + expect(qs(app.root, '.dash-notfound')).toBeNull(); + // No File control at all — no way to reach exportDashboard() (which reads + // the PRIMARY workspace via app.workspace.loadCurrent(), i.e. `primary` here) + // from this read-only page. + expect(qs(app.root, '.dash-file-btn')).toBeNull(); }); it('an unrelated keydown while the menu is open is ignored', async () => {