diff --git a/CHANGELOG.md b/CHANGELOG.md index bee7702..9fde031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,52 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] +### Fixed +- **Dashboard filter strips no longer wrap, the visible "Clear all" control and + "N active" count are both removed, and the layout switcher moved to the + header as a compact select** (#294 + a same-PR 2026-07-18 owner follow-up — + together reverse the visible clear-all/count UX decisions made in #286/#293). + `.dash-toolbar` and the filter field region (`.dash-filter-host`, which IS + now the horizontally-scrolling viewport — no separate `.filter-strip-scroll` + wrapper) stay `flex-wrap: nowrap` at every viewport width, not just the + ≤768px mobile breakpoint from #248 — several `{name:Type}` params at desktop + width used to wrap the toolbar onto extra rows, permanently shrinking the + dashboard's data area. Both the **Clear all** button (`filterClearAllButton`, + `.dash-filter-clear-all`) and the **"N active"** status (`filterActiveCount`, + `.dash-filter-count`/`.dash-filter-count-host`) are deleted outright — + `DashboardViewerSession.clearAllFilters()`/`activeFilterCount` stay tested + application-level operations/state with no UI trigger or display. The + toolbar itself is now hidden whenever there are no filters, at every width + (previously mobile-only), since it holds nothing else. The flow preset + switcher is now a compact `` (2026-07-18 owner + * override — was a `.dash-seg` segmented button group; restyled to match the + * Workbench's panel-type picker, `renderPanelTypePicker` in panels.ts, so the + * dashboard reuses the same "shared `.result-panel-select` chrome" convention + * rather than its own). `options` are `[value,label,title?]`; `sync()` (called + * on construction and after every layout change) reflects the current preset. */ -type SegOption = [value: string, label: string, title?: string]; -function buildSeg( - cls: string, options: SegOption[], getActive: () => string, onPick: (value: string) => void, ariaLabel: string, -): { el: HTMLElement; sync: () => void } { - const btns = options.map(([, label, title]) => h('button', { class: 'dash-seg-btn', type: 'button', title }, label)); - const sync = (): void => btns.forEach((b, i) => { - const on = options[i][0] === getActive(); - b.classList.toggle('is-active', on); - b.setAttribute('aria-pressed', String(on)); - }); - btns.forEach((b, i) => { b.onclick = () => onPick(options[i][0]); }); - const el = h('div', { class: 'dash-seg ' + cls, role: 'group', 'aria-label': ariaLabel }, ...btns); +type LayoutOption = [value: string, label: string, title?: string]; +function buildLayoutSelect( + options: LayoutOption[], getActive: () => string, onPick: (value: string) => void, ariaLabel: string, +): { el: HTMLSelectElement; sync: () => void } { + const el = h('select', { + class: 'result-panel-select dash-layout-select', 'aria-label': ariaLabel, title: ariaLabel, + onchange: (e: Event) => onPick((e.target as HTMLSelectElement).value), + }) as HTMLSelectElement; + for (const [value, label, title] of options) { + const opt = h('option', { value }, label) as HTMLOptionElement; + if (title) opt.title = title; + el.appendChild(opt); + } + const sync = (): void => { el.value = getActive(); }; sync(); return { el, sync }; } @@ -216,18 +221,12 @@ export async function renderDashboard(app: DashboardApp): Promise { themeBtn.appendChild(state.theme === 'dark' ? Icon.sun() : Icon.moon()); app.dom.themeBtn = themeBtn; - 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', - }, Icon.arrow(), h('span', { class: 'dash-back-label' }, 'SQL Browser')), - h('div', { class: 'dash-title' }, currentDoc.title || state.libraryName.value), - tileCount, - h('div', { class: 'dash-spacer', style: { flex: '1' } }), - h('span', { class: 'dash-chip dash-src', title: app.conn.host() }, h('span', { class: 'dash-dot' }), app.conn.host()), - updated, themeBtn, refreshBtn); - // ── Preset switcher (change-layout command) ─────────────────────────────── - const presetSeg = buildSeg('dash-seg-layout', [ + // 2026-07-18 owner override: moved off the filter toolbar and into the top + // header row (right after the tile-count chip) so the toolbar's whole width + // is available for filters; a compact select needs far less room than the + // four-button segmented control it replaces. + const layoutSelect = buildLayoutSelect([ ['full-width', 'Full width', 'One tile per row using all available width'], ['report', 'Report', 'One centered, taller tile per row'], ['columns-2', '2 columns', 'Arrange tiles in two columns'], @@ -235,12 +234,26 @@ export async function renderDashboard(app: DashboardApp): Promise { ], () => (typeof currentDoc.layout.preset === 'string' ? currentDoc.layout.preset : 'full-width'), (preset) => { runCommand({ type: 'change-layout', layout: { ...currentDoc.layout, preset: preset as FlowPresetV1 } }); }, 'Dashboard layout'); - const layoutWrap = h('div', { class: 'dash-layout-wrap' }, h('span', { class: 'dash-seg-label' }, 'Layout'), presetSeg.el); + const layoutWrap = h('div', { class: 'dash-layout-wrap' }, layoutSelect.el); + + 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', + }, Icon.arrow(), h('span', { class: 'dash-back-label' }, 'SQL Browser')), + h('div', { class: 'dash-title' }, currentDoc.title || state.libraryName.value), + tileCount, layoutWrap, + h('div', { class: 'dash-spacer', style: { flex: '1' } }), + h('span', { class: 'dash-chip dash-src', title: app.conn.host() }, h('span', { class: 'dash-dot' }), app.conn.host()), + updated, themeBtn, refreshBtn); // ── Filter bar (shared buildFilterBar, viewer-backed) ───────────────────── + // #294: the field region scrolls horizontally in its own viewport + // (`.dash-filter-host`) so it never wraps the toolbar onto a second row. + // No visible Clear-all control (reverses the #286/#293 decision) — no + // visible "N active" count either (2026-07-18 owner override, reverses + // #294's own retained-count acceptance criterion) — `session.clearAllFilters()` + // stays a tested application-level operation with no UI trigger. const filterHost = h('div', { class: 'dash-filter-host' }); - const filterCountNode = h('span', { class: 'dash-filter-count-host' }); - const clearAllNode = h('span', { class: 'dash-filter-clear-all-host' }); // The draft value/active bag the shared filter bar reads + mutates; re-seeded // from committed filter state on each (re)build. Recents come from the real // app — the viewer never touches AppState. @@ -274,7 +287,7 @@ export async function renderDashboard(app: DashboardApp): Promise { }; const getField = (name: string, mode: ValidationMode) => session.getFilterField(name, mode, draftValues, draftActive); const bar = buildFilterBar(filterBarApp, session.controls, onCommit, getField, { curatedFields, document: doc }); - filterHost.replaceChildren(bar.el, clearAllNode, filterCountNode); + filterHost.replaceChildren(bar.el); filterBarDispose = bar.dispose; } @@ -296,7 +309,7 @@ export async function renderDashboard(app: DashboardApp): Promise { if (applied.ok) { const normalized = flowLayoutPlugin.normalize(applied.dashboard); currentDoc = normalized; - presetSeg.sync(); + layoutSelect.sync(); session.syncDocument({ ...normalized, filters: [...(normalized.filters || []), ...synthesizeImplicitFilters(normalized, queryById)], }); @@ -458,8 +471,6 @@ export async function renderDashboard(app: DashboardApp): Promise { // progress ticks — so in-progress typing is not disturbed mid-wave. const sig = JSON.stringify(sview.filters.map((f) => [f.id, f.active, valueString(f.value), !!(f.options && f.options.length)])); if (sig !== barSig) { barSig = sig; rebuildFilterBar(sview); } - clearAllNode.replaceChildren(filterClearAllButton({ active: sview.activeFilterCount > 0, onClearAll: () => session.clearAllFilters() })); - filterCountNode.replaceChildren(filterActiveCount(sview.activeFilterCount)); tileCountLabel.textContent = sview.tiles.length + (sview.tiles.length === 1 ? ' tile' : ' tiles'); empty.style.display = sview.tiles.length ? 'none' : ''; // Genuine dashboard-config diagnostics only (a tile whose presentation @@ -476,7 +487,7 @@ export async function renderDashboard(app: DashboardApp): Promise { } }); - const toolbar = h('div', { class: 'dash-toolbar' + (session.state.value.filters.length ? ' has-filters' : '') }, layoutWrap, filterHost); + const toolbar = h('div', { class: 'dash-toolbar' + (session.state.value.filters.length ? ' has-filters' : '') }, filterHost); // `!`: the dashboard renders only into a mounted page. app.root!.replaceChildren(h('div', { class: 'dash-page' }, diff --git a/src/ui/filter-bar.ts b/src/ui/filter-bar.ts index 4b0d3ea..cdb833e 100644 --- a/src/ui/filter-bar.ts +++ b/src/ui/filter-bar.ts @@ -86,37 +86,10 @@ const buildRecentField = _buildRecentField as (opts: { onValueInput: () => void; onCommit: () => void; }) => FilterBarComboField; -// ── #188 filter-bar usability affordances (#286) ───────────────────────────── -// Small, reusable DOM builders the Dashboard viewer drives from its filter -// state (clearFilter / clearAllFilters / activeFilterCount / blocking). Kept -// here, on the shared filter-bar module, so any filter surface can compose the -// same affordances; they are pure element factories (no app/state coupling). - -/** The toolbar clear-all affordance (#188): resets every filter in one wave. - * Hidden (not just disabled) when nothing is active, so it never draws focus - * to a no-op. */ -export function filterClearAllButton(opts: { active: boolean; onClearAll: () => void }): HTMLButtonElement { - const btn = h('button', { - type: 'button', class: 'dash-filter-clear-all', - title: 'Clear all filters', 'aria-label': 'Clear all filters', - onclick: () => opts.onClearAll(), - }, 'Clear all'); - if (!opts.active) btn.style.display = 'none'; - return btn; -} - -/** The "N active" indicator (#188): counts ACTIVE filter definitions, readable - * by assistive tech without a live-region announcement (a plain labeled - * status node, not aria-live). Absent (empty) when nothing is active. */ -export function filterActiveCount(count: number): HTMLElement { - const node = h('span', { - class: 'dash-filter-count', role: 'status', - 'aria-label': `${count} active filter${count === 1 ? '' : 's'}`, - }); - if (count > 0) node.textContent = `${count} active`; - else node.style.display = 'none'; - return node; -} +// #188's clear-all button and "N active" count affordances (#286) were both +// removed from the Dashboard toolbar — clear-all by #294, the count by a +// 2026-07-18 owner override reversing #294's own retained-count acceptance +// criterion. Neither has a remaining UI consumer. // Idle time after the last keystroke in a filter field before it triggers a // re-run (#149 D3) — longer than the FROM-scope column-load debounce diff --git a/tests/e2e/dashboard-mobile.html b/tests/e2e/dashboard-mobile.html index bfd384f..013a45f 100644 --- a/tests/e2e/dashboard-mobile.html +++ b/tests/e2e/dashboard-mobile.html @@ -16,7 +16,14 @@
A deliberately long production operations Dashboard name
6 favorites - 1 not shown +
+ +
clickhouse.example.internal Updated 12:34 @@ -29,21 +36,10 @@
-
- Layout -
- - - - -
-
-
-
Layout-only toolbar
-
+
Empty toolbar (no filters)
Panel one
Panel two
@@ -54,9 +50,14 @@ + + diff --git a/tests/e2e/workbench-var-strip.spec.js b/tests/e2e/workbench-var-strip.spec.js new file mode 100644 index 0000000..2a9409c --- /dev/null +++ b/tests/e2e/workbench-var-strip.spec.js @@ -0,0 +1,47 @@ +import { test, expect } from '@playwright/test'; + +// The Workbench `{name:Type}` variable strip (#134/#248 precedent) already +// implements the never-wrap/horizontally-scrolling interaction contract #294 +// generalizes to Dashboard filters. happy-dom cannot compute CSS layout +// (wrapping/overflow are invisible to the unit suite), so this real-browser +// harness pins the contract directly against `.var-strip`. +async function openAt(page, width, height = 700) { + await page.setViewportSize({ width, height }); + await page.goto('/tests/e2e/workbench-var-strip.html'); + await page.waitForFunction(() => window.__ready === true); +} + +test.describe('Workbench var-strip overflow', () => { + test('stays on one row and scrolls horizontally instead of wrapping', async ({ page }) => { + await openAt(page, 900); + const strip = page.locator('.var-strip'); + const layout = await strip.evaluate((node) => ({ + flexWrap: getComputedStyle(node).flexWrap, + overflowX: getComputedStyle(node).overflowX, + clientWidth: node.clientWidth, + scrollWidth: node.scrollWidth, + fieldTops: [...node.querySelectorAll('.var-field')].map((field) => field.getBoundingClientRect().top), + fieldWidths: [...node.querySelectorAll('.var-field')].map((field) => field.getBoundingClientRect().width), + pageOverflow: document.documentElement.scrollWidth - innerWidth, + })); + expect(layout.flexWrap).toBe('nowrap'); + expect(layout.overflowX).toBe('auto'); + expect(layout.scrollWidth).toBeGreaterThan(layout.clientWidth); + expect(Math.max(...layout.fieldTops) - Math.min(...layout.fieldTops)).toBeLessThan(2); + expect(Math.min(...layout.fieldWidths)).toBeGreaterThan(150); + expect(layout.pageOverflow).toBeLessThanOrEqual(0); + }); + + test('reaches the final field by scroll and by keyboard focus', async ({ page }) => { + await openAt(page, 900); + const strip = page.locator('.var-strip'); + + await strip.evaluate((node) => { node.scrollLeft = node.scrollWidth; }); + expect(await strip.evaluate((node) => node.scrollLeft)).toBeGreaterThan(0); + await strip.evaluate((node) => { node.scrollLeft = 0; }); + + const lastInput = page.locator('.var-strip .var-field').last().locator('input'); + await lastInput.focus(); + await expect(lastInput).toBeInViewport(); + }); +}); diff --git a/tests/unit/dashboard.test.ts b/tests/unit/dashboard.test.ts index 3408861..7a1c221 100644 --- a/tests/unit/dashboard.test.ts +++ b/tests/unit/dashboard.test.ts @@ -33,7 +33,7 @@ const qs = (root: ParentNode | null, selector: (root as ParentNode).querySelector(selector) as T; const qsa = (root: ParentNode | null, selector: string): T[] => [...(root as ParentNode).querySelectorAll(selector)] as T[]; -/** `el.onclick`'s DOM-lib type takes a `MouseEvent`; every `.dash-btn`/`.dash-seg-btn` +/** `el.onclick`'s DOM-lib type takes a `MouseEvent`; every `.dash-btn` * handler this suite exercises is a real zero-arg (often async) closure `ui/dashboard.ts` * assigns directly — narrower than the lib's declared signature (an assignable * direction, not a fixture gap), so calling through it needs no argument. */ @@ -299,8 +299,14 @@ function dashApp(opts: { } const render = (app: TestApp): Promise => renderDashboard(app as unknown as Parameters[0]); -const seg = (root: ParentNode | null, label: string): HTMLElement | undefined => - qsa(root, '.dash-seg-layout .dash-seg-btn').find((b) => b.textContent === label); +/** The flow preset switcher (2026-07-18: a `