diff --git a/CHANGELOG.md b/CHANGELOG.md index c010a12d..fe8907ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,34 @@ auto-generated per-PR notes; this file is the curated, human-readable history. their existing precedence. ### Added +- **Compound time-range control for Dashboard filter bars** (#335). A pair of + scalar date-like filters whose parameter names match #334's recognized table + (`from`/`to`, `from_time`/`to_time`, `start`/`end`, `start_time`/`end_time`, + case-insensitive — never `start`/`stop`) and whose executable consumers agree + on date-like scalar contracts now renders as one compound control in a + **Time** section ahead of the other filters, replacing the pair's two + individual fields (source-backed/curated filters never group; every + non-group filter keeps its existing control). The closed trigger shows the + range resolved against the most recent execution wave's shared clock — no + ticking timers — with the raw tokens carried in the accessible name; the + popover stages token-or-absolute **From**/**To** edits with live resolved + previews, per-field relative-time constants (typing filters the list; + selecting stages, never commits), group-scoped session-only **Recently + used** ranges (immediate apply), and an explicit **Apply** gated on both + bounds resolving with `from ≤ to` (equal instants permitted). Apply commits + both bounds atomically through the new public + `DashboardViewerSession.applyFilters(entries)` batch API — one publish, one + execution wave over the union of both parameters' resolved targets; a + failed or identical draft commits nothing. Pair discovery sits behind a + resolver seam so #334's saved-query `timeRanges` metadata can replace the + interim name inference without touching the UI. Absolute bounds are now + validated locally (`parseAbsoluteInstant`: preview formats, ISO-`T` + variants, epoch digits for DateTime types, real calendar checks, years + from 1900). The popover chrome is a shared primitive + (`ui/popover.ts` `openAnchoredDialog`) extracted from the #189 multiselect + — both controls now consume it — and `fixedAnchor` gained an opt-in pure + viewport clamp for narrow screens. + - **Searchable multiselect for query-backed Dashboard filters** (#189). A source-backed filter whose executable consumers agree on one `Array(T)` parameter type now renders a dedicated searchable-checklist control: the @@ -107,6 +135,13 @@ auto-generated per-PR notes; this file is the curated, human-readable history. the synthesized binding is never written back into the dashboard document. ### Fixed +- **One wall-clock snapshot per Dashboard execution wave** (#335). Each wave + (initial load, Refresh, per-tile refresh, filter commits, dependent + filter-source waves) now captures a single `wallNow()` reading, threads it + through every relative-token resolution it performs, and publishes it as + `DashboardViewState.waveWallNowMs`. Previously one refresh took several + independent clock readings, so relative tokens (`now`, `-1d`) in different + sub-phases of the same wave could resolve to instants seconds apart. - **Saved-query, workspace, and Dashboard persistence stay consistent** (#365). Starring a panel query now creates the workspace's Dashboard and first tile atomically even when the committed aggregate previously held diff --git a/src/core/relative-time.ts b/src/core/relative-time.ts index 3eed3c73..96dd8512 100644 --- a/src/core/relative-time.ts +++ b/src/core/relative-time.ts @@ -245,7 +245,14 @@ function formatDateUTC(epochMs: number): string { // (finding #3 — never rounds into the future), with a fractional suffix only // for `DateTime64(N>0)` and only when the remainder is non-zero — a preview // showing ".000" on every value would be more noise than signal. -function formatPreviewInstant(epochMs: number, t: ParsedParamType): string { +// Exported (additive — #335) so `core/time-range.ts` can format an ABSOLUTE +// bound's resolved instant through this exact same convention, keeping the +// time-range control's "resolved preview" line visually identical whether the +// entered text was a relative token (via `formatPreview` above) or an +// absolute value/epoch digit string (via `parseAbsoluteInstant` below) — +// never a second, drifting formatter. Behavior is unchanged for every +// existing caller of this module; this only adds a new export. +export function formatPreviewInstant(epochMs: number, t: ParsedParamType): string { if (t.base === 'Date' || t.base === 'Date32') return formatDateUTC(epochMs); const d = new Date(Math.floor(epochMs / 1000) * 1000); const base = `${d.getUTCFullYear()}-${pad(d.getUTCMonth() + 1, 2)}-${pad(d.getUTCDate(), 2)} ` @@ -351,6 +358,122 @@ export function formatPreview(expr: string, type: string | ParsedParamType, nowM return { ok: true, display: formatPreviewInstant(instant, t), matched: true }; } +// ── Absolute-value parsing (#335 time-range control) ───────────────────── +// +// The time-range popover's From/To fields accept exactly what a relative +// expression doesn't claim: an absolute value for the bound's declared type. +// This is a NEW acceptance surface (not previously validated anywhere in this +// module — `resolveRelativeValue`/`formatPreview` both treat a non-relative +// string as opaque passthrough) needed so the popover can show a resolved +// preview line and reject `from > to` at the RESOLVED instant, not the raw +// text. It stays in this module (never a second grammar file, per #335's +// pinned contract) because it is exactly the absolute-value complement of the +// relative grammar above, and reuses `formatPreviewInstant` for display so +// both paths render through one convention. +// +// Accepted forms (UTC convention — matches `formatPreview`'s server-time +// convention so a previously-rendered preview round-trips back through this +// parser unchanged): +// - `YYYY-MM-DD` — any date-like type; time defaults to 00:00:00 UTC. +// - `YYYY-MM-DD HH:MM`, `YYYY-MM-DD HH:MM:SS`, and the same with 1-9 +// fractional-second digits (`YYYY-MM-DD HH:MM:SS.fff`) — DateTime/ +// DateTime64 only (a time part on Date/Date32 is an error); the `T` +// separator variant of each of these three forms is accepted too. +// - Bare digits, DateTime/DateTime64 only: 1-10 digits = epoch SECONDS, +// exactly 13 digits = epoch MILLISECONDS (any other digit-only length is +// rejected rather than guessed at). +// - Real calendar/time-of-day validation (`2026-02-30`, `24:00`, … all +// error) — never silently clamped. +// Surrounding whitespace is trimmed. Anything else is a short, human +// diagnostic — never a silent guess. Pure. + +/** `parseAbsoluteInstant`'s successful-parse shape. */ +export interface AbsoluteInstantOk { + ok: true; + instantMs: number; +} + +/** `parseAbsoluteInstant`'s rejection shape — always a short, human-readable + * diagnostic naming what was entered. */ +export interface AbsoluteInstantErr { + ok: false; + error: string; +} + +const RE_DATE_ONLY = /^(\d{4})-(\d{2})-(\d{2})$/; +const RE_DATETIME = /^(\d{4})-(\d{2})-(\d{2})[ T](\d{2}):(\d{2})(?::(\d{2})(?:\.(\d{1,9}))?)?$/; +const RE_DIGITS = /^\d+$/; + +// Real calendar validation: month in [1,12] and day within that month's +// actual length (UTC — `Date.UTC(y, m, 0)` is the last day of month `m` +// 1-indexed, i.e. the day count of month `m` itself), never a fixed 31/30/28 +// guess and never silently clamped by `Date`'s own rollover behavior. +function isValidCalendarDate(y: number, month1: number, day: number): boolean { + // Floor at 1900: no ClickHouse date type reaches lower (Date32 starts + // 1900-01-01), and it keeps `Date.UTC`'s legacy 0–99 → 1900–1999 year remap + // unreachable (a "0050" year would otherwise silently resolve as 1950). + if (y < 1900) return false; + if (month1 < 1 || month1 > 12) return false; + const daysInMonth = new Date(Date.UTC(y, month1, 0)).getUTCDate(); + return day >= 1 && day <= daysInMonth; +} + +/** + * Parse an ABSOLUTE (non-relative) value entered for a date-like `type` into + * a resolved epoch-ms instant — the absolute-input complement of the relative + * grammar above (#335). See this section's header comment for exactly which + * forms are accepted. Pure. + */ +export function parseAbsoluteInstant( + type: ParsedParamType | string, + text: string, +): AbsoluteInstantOk | AbsoluteInstantErr { + const t = parsedType(type); + const s = String(text).trim(); + const dateOnlyMatch = RE_DATE_ONLY.exec(s); + if (dateOnlyMatch) { + const y = Number(dateOnlyMatch[1]); + const month1 = Number(dateOnlyMatch[2]); + const day = Number(dateOnlyMatch[3]); + if (!isValidCalendarDate(y, month1, day)) { + return { ok: false, error: `"${s}" is not a valid calendar date.` }; + } + return { ok: true, instantMs: Date.UTC(y, month1 - 1, day, 0, 0, 0, 0) }; + } + + const dateOnlyType = t.base === 'Date' || t.base === 'Date32'; + if (dateOnlyType) { + return { ok: false, error: `"${s}" is not a valid date (expected YYYY-MM-DD).` }; + } + + const dtMatch = RE_DATETIME.exec(s); + if (dtMatch) { + const y = Number(dtMatch[1]); + const month1 = Number(dtMatch[2]); + const day = Number(dtMatch[3]); + const h = Number(dtMatch[4]); + const mi = Number(dtMatch[5]); + const se = dtMatch[6] !== undefined ? Number(dtMatch[6]) : 0; + const frac = dtMatch[7]; + if (!isValidCalendarDate(y, month1, day)) { + return { ok: false, error: `"${s}" is not a valid calendar date.` }; + } + if (h > 23 || mi > 59 || se > 59) { + return { ok: false, error: `"${s}" is not a valid time of day.` }; + } + const ms = frac ? Number((frac + '000').slice(0, 3)) : 0; + return { ok: true, instantMs: Date.UTC(y, month1 - 1, day, h, mi, se, ms) }; + } + + if (RE_DIGITS.test(s)) { + if (s.length === 13) return { ok: true, instantMs: Number(s) }; + if (s.length >= 1 && s.length <= 10) return { ok: true, instantMs: Number(s) * 1000 }; + return { ok: false, error: `"${s}" is not a recognized epoch value (expected 1-10 digits for seconds, or 13 for milliseconds).` }; + } + + return { ok: false, error: `"${s}" is not a valid absolute value for ${t.base}.` }; +} + /** * Batch helper: resolve every `{name, type}` param's stored value against * `nowMs`, one call. Empty/missing values pass through as an unmatched `ok` diff --git a/src/core/time-range.ts b/src/core/time-range.ts new file mode 100644 index 00000000..f31b3ad8 --- /dev/null +++ b/src/core/time-range.ts @@ -0,0 +1,283 @@ +// Dashboard time-range control (#335) — a pure resolver that (a) discovers +// candidate From/To filter-id pairs by #334's interim name-pair table, (b) +// gates a candidate pair into a `DashboardTimeRangeGroup` only when BOTH +// filters resolve to a scalar, date-like consumer contract (reusing +// `filter-selection.ts`'s #189 consumer-resolution machinery rather than +// re-deriving it), (c) validates a staged From/To draft against the shared +// relative/absolute grammar (`relative-time.ts`), and (d) maintains the +// session-scoped "Recently used" list. No DOM, no globals, no fetch — `nowMs` +// is always injected (the repo's keystroke rule). +// +// Pair discovery is a SEAM (`resolveTimeRangeGroups`'s optional `pairs` +// argument), not a hard dependency on `inferTimeRangePairs`: #334's saved- +// query `timeRanges` metadata is meant to replace the inference source +// without touching this module's gating logic or any UI built on top of it. +// +// Group formation never mutates anything — it's a pure re-derivation over the +// dashboard's current filter defs, analysis, and executable-tile set, exactly +// like `resolveFilterSelection` itself. + +import { resolveFilterSelection } from './filter-selection.js'; +import type { FilterSelectionFilterDef } from './filter-selection.js'; +import type { ParameterAnalysis } from './param-pipeline.js'; +import type { ParsedParamType } from './param-type.js'; +import { parseParamType } from './param-type.js'; +import { + parseRelativeExpr, + resolveInstant, + formatPreviewInstant, + parseAbsoluteInstant, + isDateLikeType, +} from './relative-time.js'; +import type { ParseRelativeResult, RelativeExprError } from './relative-time.js'; + +// A local discriminant for `ParseRelativeResult`, mirroring `relative-time.ts`'s +// own private `isParseError` (not itself exported — a narrow, non-parsing type +// predicate is fine to have twice; the GRAMMAR it discriminates over is not +// duplicated anywhere). Needed because `RelativeExprError.error` is typed +// `string` while `ParsedRelativeExpr.error` is typed `undefined` — an inline +// `typeof` check narrows the local binding but a named predicate keeps +// `resolveBound` below readable. +function isRelativeParseError(r: ParseRelativeResult): r is RelativeExprError { + return r != null && typeof r.error === 'string'; +} + +// ── Group model ────────────────────────────────────────────────────────── + +/** + * One resolved time-range group: a pair of dashboard filters whose agreed + * consumer contracts are both scalar and date-like. `key` is derived purely + * from the two filters' own ids (never labels, never array index) so it's + * stable across a group list re-render as long as the underlying filter ids + * don't change. + */ +export interface DashboardTimeRangeGroup { + key: string; + fromFilterId: string; + toFilterId: string; + fromParameter: string; + toParameter: string; + fromType: ParsedParamType; + toType: ParsedParamType; +} + +/** One candidate pair of filter ids — `inferTimeRangePairs`'s output, and the + * shape #334's saved-query-metadata resolution will eventually produce in + * its place (same seam, see this module's header comment). */ +export interface TimeRangePairCandidate { + fromFilterId: string; + toFilterId: string; +} + +// #334's interim recognized name-pair table (case-insensitive exact match on +// the FULL parameter name — never a prefix/substring match). Order here is +// also the emission order when more than one row matches distinct filters. +// `start`/`stop` is deliberately NOT a recognized pair (owner decision). +const NAME_PAIR_TABLE: ReadonlyArray = [ + ['from', 'to'], + ['from_time', 'to_time'], + ['start', 'end'], + ['start_time', 'end_time'], +]; + +/** + * Interim pair-discovery source (#334's table): infer From/To candidate pairs + * from the dashboard's filter parameter names alone, with no notion of + * consumer contracts yet (that's `resolveTimeRangeGroups`'s job). Rules: + * - a filter with a non-null `sourceQueryId` (curated/source-backed) is + * NEVER a candidate — filtered out before any name matching; + * - matching is case-insensitive but exact (the whole parameter name, not a + * substring) against `NAME_PAIR_TABLE`; + * - a parameter name borne by MORE THAN ONE (non-curated) filter def is + * unusable — any pair that would need it does not form, for either role; + * - a filter id may appear in at most one emitted pair; if the name-pair + * table would place one filter in two candidate pairs, EVERY pair + * involving that filter is dropped (ambiguity → no group, not a guess at + * which pair "wins"). + * Pure. + */ +export function inferTimeRangePairs( + filters: ReadonlyArray<{ id: string; parameter: string; sourceQueryId?: string | null }>, +): TimeRangePairCandidate[] { + const eligible = filters.filter((f) => f.sourceQueryId == null); + + // Group eligible filters by lowercased parameter name so a name borne by + // more than one filter can be recognized as unusable. + const byName = new Map(); + for (const f of eligible) { + const key = f.parameter.toLowerCase(); + const ids = byName.get(key) || []; + ids.push(f.id); + byName.set(key, ids); + } + const soleIdFor = (name: string): string | null => { + const ids = byName.get(name); + return ids && ids.length === 1 ? ids[0] : null; + }; + + const raw: TimeRangePairCandidate[] = []; + for (const [fromName, toName] of NAME_PAIR_TABLE) { + const fromFilterId = soleIdFor(fromName); + const toFilterId = soleIdFor(toName); + if (fromFilterId && toFilterId) raw.push({ fromFilterId, toFilterId }); + } + + // Ambiguity guard: a filter id appearing in more than one raw candidate + // (across either role) invalidates every candidate it appears in. + const usageCount = new Map(); + for (const pair of raw) { + usageCount.set(pair.fromFilterId, (usageCount.get(pair.fromFilterId) || 0) + 1); + usageCount.set(pair.toFilterId, (usageCount.get(pair.toFilterId) || 0) + 1); + } + return raw.filter((pair) => usageCount.get(pair.fromFilterId) === 1 && usageCount.get(pair.toFilterId) === 1); +} + +/** + * Gate candidate pairs into resolved `DashboardTimeRangeGroup`s. `pairs` + * defaults to `inferTimeRangePairs(input.filters)` — the seam #334's metadata + * resolution will supply instead, without this gating logic changing. A pair + * forms a group ONLY when BOTH filters resolve via `resolveFilterSelection` + * with zero diagnostics, a non-null contract, `contract.array === false`, and + * `isDateLikeType(contract.type)` — a filter missing from `input.filters` + * (only possible when a caller supplies its own `pairs`) is skipped rather + * than throwing. Emitted in `pairs`' own order. Pure — never mutates + * `input.filters`/`input.analysis`. + */ +export function resolveTimeRangeGroups(input: { + filters: ReadonlyArray; + analysis: ParameterAnalysis; + executableTileIds: ReadonlySet; + pairs?: TimeRangePairCandidate[]; +}): DashboardTimeRangeGroup[] { + const { filters, analysis, executableTileIds } = input; + const byId = new Map(filters.map((f) => [f.id, f] as const)); + const pairs = input.pairs ?? inferTimeRangePairs(filters); + + const groups: DashboardTimeRangeGroup[] = []; + for (const pair of pairs) { + const fromFilter = byId.get(pair.fromFilterId); + const toFilter = byId.get(pair.toFilterId); + if (!fromFilter || !toFilter) continue; + + const fromRes = resolveFilterSelection(fromFilter, analysis, executableTileIds); + const toRes = resolveFilterSelection(toFilter, analysis, executableTileIds); + if (fromRes.diagnostics.length || toRes.diagnostics.length) continue; + if (!fromRes.contract || !toRes.contract) continue; + if (fromRes.contract.array || toRes.contract.array) continue; + if (!isDateLikeType(fromRes.contract.type) || !isDateLikeType(toRes.contract.type)) continue; + + groups.push({ + key: `${fromFilter.id}\u0000${toFilter.id}`, + fromFilterId: fromFilter.id, + toFilterId: toFilter.id, + fromParameter: fromFilter.parameter, + toParameter: toFilter.parameter, + fromType: fromRes.contract.type, + toType: toRes.contract.type, + }); + } + return groups; +} + +// ── Draft validation ───────────────────────────────────────────────────── + +/** One bound's (From's or To's) staged-draft resolution. `display` and + * `instantMs` are both null exactly when `!ok`. `matchedRelative` is true + * only when the text actually parsed as a relative-time expression (as + * opposed to an absolute value or an empty/invalid entry) — mirrors + * `formatPreview`'s/`resolveRelativeValue`'s own `matched` flag. */ +export interface TimeRangeBoundDraft { + ok: boolean; + display: string | null; + instantMs: number | null; + error: string | null; + matchedRelative: boolean; +} + +function resolveBound(text: string, type: ParsedParamType | string, nowMs: number): TimeRangeBoundDraft { + const t = typeof type === 'string' ? parseParamType(type) : type; + const trimmed = text.trim(); + if (trimmed === '') { + return { ok: false, display: null, instantMs: null, error: 'A value is required.', matchedRelative: false }; + } + + const parsed = parseRelativeExpr(trimmed); + if (isRelativeParseError(parsed)) { + return { ok: false, display: null, instantMs: null, error: parsed.error, matchedRelative: false }; + } + if (parsed) { + const instantMs = resolveInstant(parsed, nowMs); + return { ok: true, display: formatPreviewInstant(instantMs, t), instantMs, error: null, matchedRelative: true }; + } + + const abs = parseAbsoluteInstant(t, trimmed); + if (!abs.ok) { + return { ok: false, display: null, instantMs: null, error: abs.error, matchedRelative: false }; + } + return { ok: true, display: formatPreviewInstant(abs.instantMs, t), instantMs: abs.instantMs, error: null, matchedRelative: false }; +} + +/** + * Validate a staged From/To draft against ONE shared wall-clock snapshot + * (`nowMs`) — the issue's "single preview `now`" rule: both bounds resolve + * their relative tokens (if any) against the same instant, so `now` in From + * and `now` in To always agree within one validation pass. Resolution per + * bound: `parseRelativeExpr` first (a genuine parse resolves via + * `resolveInstant(nowMs)`; a near-miss is `!ok` with that grammar error; + * `null` — not relative at all — falls through to `parseAbsoluteInstant`); + * empty/whitespace-only text is `!ok` with a "required" diagnostic before + * either parser runs. `rangeOk` requires both bounds to resolve AND + * `fromInstant <= toInstant` (equal instants are explicitly permitted); + * `rangeError` is set only when both bounds resolve but `from > to` — a + * per-bound `error` already covers an unresolvable bound, so this is never + * set redundantly alongside one. Pure. + */ +export function validateTimeRangeDraft(input: { + fromText: string; + toText: string; + fromType: ParsedParamType | string; + toType: ParsedParamType | string; + nowMs: number; +}): { + from: TimeRangeBoundDraft; + to: TimeRangeBoundDraft; + rangeOk: boolean; + rangeError: string | null; + applyEnabled: boolean; +} { + const from = resolveBound(input.fromText, input.fromType, input.nowMs); + const to = resolveBound(input.toText, input.toType, input.nowMs); + + let rangeOk = from.ok && to.ok; + let rangeError: string | null = null; + if (rangeOk && from.instantMs! > to.instantMs!) { + rangeOk = false; + rangeError = 'The "from" bound must not be after the "to" bound.'; + } + + return { from, to, rangeOk, rangeError, applyEnabled: from.ok && to.ok && rangeOk }; +} + +// ── Recently used ──────────────────────────────────────────────────────── + +/** One recorded From/To token pair — the RAW committed text of each bound, + * never the resolved instant (a relative pair like `-1d` → `now` must + * re-display and re-apply as that live token, not a frozen absolute). */ +export interface TimeRangeRecent { + from: string; + to: string; +} + +/** + * Push a newly-committed range onto a "Recently used" list: dedupe by EXACT + * token-pair equality (both `from` AND `to` match a stored entry), unshift + * newest-first, cap at 6. Immutable — always returns a NEW array, never + * mutates `list`. Pure. + */ +export function pushRecentRange( + list: ReadonlyArray, + pair: TimeRangeRecent, +): TimeRangeRecent[] { + const deduped = list.filter((r) => !(r.from === pair.from && r.to === pair.to)); + return [pair, ...deduped].slice(0, 6); +} diff --git a/src/dashboard/application/dashboard-viewer-session.ts b/src/dashboard/application/dashboard-viewer-session.ts index 1b2717d0..0d35b90a 100644 --- a/src/dashboard/application/dashboard-viewer-session.ts +++ b/src/dashboard/application/dashboard-viewer-session.ts @@ -40,6 +40,8 @@ import type { FilterSourceAnalysis } from '../../core/filter-execution.js'; import { readFilterOptions } from '../../core/filter-options.js'; import { resolveFilterSelection, sameSelection } from '../../core/filter-selection.js'; import type { FilterSelectionFilterDef } from '../../core/filter-selection.js'; +import { resolveTimeRangeGroups } from '../../core/time-range.js'; +import type { DashboardTimeRangeGroup } from '../../core/time-range.js'; import { mergeDashboardFilterHelpers } from '../../core/dashboard-filters.js'; import type { FilterProvider, FilterHelperOption, FilterDiagnostic, MergeDashboardFilterHelpersResult, @@ -184,6 +186,16 @@ export interface DashboardViewState { * appear once by construction). Reset to `[]` at the start of each wave; * a pre-wave publish (session construction) also reads `[]`. */ filterDiagnostics: FilterDiagnostic[]; + /** #335: the ONE wall-clock snapshot (`deps.wallNow()`) the latest execution + * wave resolved its relative tokens against — `null` before the first wave. + * Every wave that reruns tiles (`refresh`/`runAffectedWave`, and the + * filter-source wave chain in `commitAndRerun`) captures a single snapshot + * at entry and threads it through every `prepareBatch`/filter-source + * resolution in that wave, so a relative token (`now`, `-1d`, …) borne by + * two tiles run in different sub-phases of one wave still resolves to the + * exact same instant. The time-range control's closed-trigger label + * re-resolves against this value on each change — no ticking timers. */ + waveWallNowMs: number | null; } // ── Narrow injected dependencies (no App / AppState / net imports) ──────────── @@ -253,6 +265,12 @@ export interface DashboardViewerSession { readonly state: ReadonlySignal; /** The `{name:Type}` field controls the filter bar renders (structure only). */ readonly controls: FieldControl[]; + /** #335: the resolved time-range groups (pairs of scalar date-like filters, + * curated-excluded) — computed ONCE at construction, AFTER the source- + * fallback resolution loop, so a filter whose source fell back is treated + * per its post-resolution (plain) state. Never recomputed across the + * session; empty when no pair resolves. */ + readonly timeRangeGroups: DashboardTimeRangeGroup[]; /** One field's prepared #170 validation state against the filter bar's DRAFT * values/active (in-progress typing) — for the shared invalid-field affordance. */ getFilterField( @@ -270,6 +288,15 @@ export interface DashboardViewerSession { * which owns activation for optional/curated fields), then run the one * affected-panel wave. */ applyFilter(filterId: string, value: unknown, active: boolean): Promise; + /** #335: commit MULTIPLE filters atomically (the time-range control's + * From/To pair, and #334's drag-to-select) in ONE execution wave over the + * union of every changed parameter's resolved targets. Every `filterId` + * must resolve and be unique — an unknown OR duplicate id makes the whole + * call a silent no-op (nothing mutated, no publish, no wave), matching + * `applyFilter`'s posture (type-level value validation stays the pipeline's + * job). A call in which nothing actually changes (values+active equal the + * committed state) publishes nothing and runs no wave. */ + applyFilters(entries: Array<{ filterId: string; value: string | string[]; active: boolean }>): Promise; /** Deactivate one filter WITHOUT discarding its value (reactivation restores * it); one affected-panel wave (#188 clear-one). */ clearFilter(filterId: string): Promise; @@ -437,6 +464,13 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa // outside `documentRef` — never read/written by any command, never // persisted. A fresh session always starts at 'tiles'. let gridRenderMode: GridRenderMode = 'tiles'; + // #335: the single wall-clock snapshot the LATEST execution wave resolved its + // relative tokens against (published as `state.waveWallNowMs`). `null` until + // the first wave; every wave entry point (`refresh`/`runAffectedWave`/the + // `commitAndRerun` filter-source chain) captures ONE `deps.wallNow()` at + // entry, sets this, and threads that same instant into every `prepareBatch`/ + // filter-source resolution it runs. + let waveWallNowMs: number | null = null; const queryById = new Map(); for (const query of queries) { @@ -723,6 +757,29 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa for (const id of resolveFilterTargets(filter.def)) affectedByFilterWave.add(id); } + // #335: resolve the dashboard's time-range groups ONCE, here — AFTER the + // #189 source-fallback loop above — so `sourceQueryId` reflects each + // filter's POST-resolution state (`filter.state.sourceId`), not the + // structural `def.sourceQueryId`. `inferTimeRangePairs` (inside + // `resolveTimeRangeGroups`) never pairs a filter carrying a non-null + // `sourceQueryId` (curated exclusion — owner decision), so a filter whose + // source SURVIVED resolution is excluded, while one that FELL BACK (stripped + // to `undefined`) is treated as the plain filter it now is and can group + // like any other date-like scalar pair. Every other input matches the #189 + // machinery (`analysis`, `executableTileIds`, and the same selection def) + // so the gate's own `resolveFilterSelection` agrees with construction's. + const timeRangeGroups = resolveTimeRangeGroups({ + filters: filters.map((filter) => ({ + id: filter.def.id, + parameter: filter.def.parameter, + targets: Array.isArray(filter.def.targets) ? filter.def.targets : undefined, + selection: filter.def.selection, + sourceQueryId: filter.state.sourceId ?? null, + })), + analysis, + executableTileIds, + }); + // Curated option bundles from the last filter wave (param name → field). let curated: MergeDashboardFilterHelpersResult['fields'] = {}; // The last filter wave's merge diagnostics (#359) — a closure var like @@ -764,11 +821,17 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa mode: ValidationMode = 'execute', values: Record = rawValues(), active: Record = activeMap(), + // #335: the wave's shared wall-clock snapshot — every relative token in this + // batch resolves against it. Defaults to a FRESH `deps.wallNow()` so the + // keystroke-time `getFilterField` path (which passes no snapshot) keeps + // resolving previews against live wall-now; each execution-wave caller + // passes its own single snapshot instead. + wallNowMs: number = deps.wallNow(), ) => prepareParameterizedBatch(analysis, { values: Object.fromEntries(Object.entries(values).map(([name, value]) => [name, curated[name] && !active[name] ? '' : value])), active: effectiveActive(values, active), - wallNowMs: deps.wallNow(), validationMode: mode, + wallNowMs, validationMode: mode, }); /** One field's prepared #170 state against the caller's draft values/active @@ -811,6 +874,7 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa // `filterDiagnostics` on every publish, rather than merged into that // mutable array, so nothing a wave does can ever drop them. filterDiagnostics: [...staticFilterDiagnostics, ...filterDiagnostics], + waveWallNowMs, }; } @@ -1223,10 +1287,12 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa return applyFilterProviders(plan); } - async function runFilterWave(): Promise { - // One wall-clock reading for the WHOLE wave (mirrors the tile wave's own - // `deps.wallNow()` capture in `refresh()`). - const waveMs = deps.wallNow(); + async function runFilterWave(waveMs: number): Promise { + // #335: `waveMs` is the ONE snapshot the whole refresh shares (tiles + this + // filter wave) — always supplied by `refresh`, never re-read here. Was a + // local `deps.wallNow()` here — that made the + // filter wave resolve relative dependency tokens against a DIFFERENT + // instant than the tiles in the same refresh, the latent bug #335 fixes. // Full refresh: every known source, keep options through loading (#359 // no-flicker invariant), reset diagnostics immediately (see // `executeFilterSourcePlan`'s doc comment for the "why"). Unlike a @@ -1252,14 +1318,17 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa * caller (`commitAndRerun`) uses to fold the flipped parameter names into * ONE combined affected-panel wave alongside `changedParams`, or to skip * that wave entirely when this plan was superseded. */ - async function runFilterSourceWave(changedParams: string[]): Promise { + async function runFilterSourceWave( + changedParams: string[], waveMs: number, + ): Promise { // Entered only from `commitAndRerun`'s affected path, which preflights ONCE // for the whole commit (source wave + affected-panel wave) — avoiding a // double `ensureFreshToken()`/`onAuthFailed` on a stale token — so this wave // never preflights itself. (`runAffectedWave` keeps its own `preflighted` // flag because it is ALSO reached directly on the no-affected-source fast - // path, where nothing has preflighted yet.) - const waveMs = deps.wallNow(); + // path, where nothing has preflighted yet.) #335: the commit's single + // `waveMs` snapshot is passed in so this source wave and its sibling + // affected-panel wave resolve relative tokens against the same instant. const affected = [...filterSources.values()].filter((source) => source.analyzed.dependsOn.some((name) => changedParams.includes(name))); if (affected.length === 0) return { status: 'applied', flipped: [] }; @@ -1286,6 +1355,12 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa async function refresh(): Promise { if (!(await preflight())) return; + // #335: ONE wall-clock snapshot for the WHOLE refresh — the unaffected-tile + // batch, the filter wave, AND the affected-tile batch all resolve their + // relative tokens against this single instant (fixing the prior latent + // inconsistency where each sub-phase took its own `deps.wallNow()`). + const waveMs = deps.wallNow(); + waveWallNowMs = waveMs; markTextAndErrorTiles(); const runnable = runnableTiles(); // Reserve every runnable tile's generation up front (stale-wave guard). @@ -1295,10 +1370,10 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa publish(true); // #235: launch the unaffected tiles NOW (with current values) in parallel // with the filter wave — they never wait for a source query. - const firstBatch = sourcesById(prepareBatch('execute').sources); + const firstBatch = sourcesById(prepareBatch('execute', undefined, undefined, waveMs).sources); const unaffectedWave = runPool(unaffected, VIEWER_TILE_CONCURRENCY, (runtime) => runTile(runtime, firstBatch.get(runtime.tile.id), generations.get(runtime.tile.id)!)); - const filterResult = await runFilterWave(); + const filterResult = await runFilterWave(waveMs); if (destroyed) { await unaffectedWave; return; } if (filterResult.status === 'superseded') { // A concurrent selective commit superseded this refresh's Filter wave @@ -1317,8 +1392,9 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa publish(false, destroyed ? null : deps.now()); return; } - // Affected tiles run AFTER the filter wave, with the merged/blanked values. - const secondBatch = sourcesById(prepareBatch('execute').sources); + // Affected tiles run AFTER the filter wave, with the merged/blanked values — + // against the SAME `waveMs` the unaffected batch and filter wave used. + const secondBatch = sourcesById(prepareBatch('execute', undefined, undefined, waveMs).sources); const affectedWave = runPool(affected, VIEWER_TILE_CONCURRENCY, (runtime) => runTile(runtime, secondBatch.get(runtime.tile.id), generations.get(runtime.tile.id)!)); await Promise.all([unaffectedWave, affectedWave]); @@ -1331,13 +1407,20 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa const runtime = tiles.find((entry) => entry.tile.id === tileId); if (!runtime || !runtime.query || runtime.isText || runtime.presentationError) return; if (!(await preflight())) return; + // A single-tile refresh is a wave of one: it must publish its snapshot + // like every other wave, or the tile's re-resolved relative bounds drift + // from the closed time-range trigger label until the next full wave. + const waveMs = deps.wallNow(); + waveWallNowMs = waveMs; const generation = supersede(runtime); - const prepared = sourcesById(prepareBatch('execute').sources); + const prepared = sourcesById(prepareBatch('execute', undefined, undefined, waveMs).sources); await runTile(runtime, prepared.get(tileId), generation); } // Re-run only the tiles some active filter parameter feeds into. - async function runAffectedWave(parameters: string[], preflighted = false): Promise { + async function runAffectedWave( + parameters: string[], preflighted: boolean, waveMs: number, + ): Promise { // Unconditional destroyed guard: the `preflighted: true` fast path (from // `commitAndRerun`'s affected branch) skips `preflight()` entirely below // — and `preflight()` was the ONLY place on this path that @@ -1364,7 +1447,11 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa } const targets = runnableTiles().filter((runtime) => affectedIds.has(runtime.tile.id)); const generations = new Map(targets.map((runtime) => [runtime.tile.id, supersede(runtime)])); - const prepared = sourcesById(prepareBatch('execute').sources); + // #335: publish this wave's shared snapshot and resolve the batch's + // relative tokens against it (the same instant `commitAndRerun`'s source + // wave used, when there was one). + waveWallNowMs = waveMs; + const prepared = sourcesById(prepareBatch('execute', undefined, undefined, waveMs).sources); publish(); await runPool(targets, VIEWER_TILE_CONCURRENCY, (runtime) => runTile(runtime, prepared.get(runtime.tile.id), generations.get(runtime.tile.id)!)); @@ -1389,18 +1476,23 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa // `destroy()` bumps every source generation too, so a source wave racing a // `destroy()` is caught by the same check. async function commitAndRerun(changed: string[]): Promise { + // #335: ONE wall-clock snapshot for the whole commit — the filter-source + // wave (when there is one) and the affected-panel wave both resolve their + // relative tokens against it, and it is published as `waveWallNowMs`. + const waveMs = deps.wallNow(); + waveWallNowMs = waveMs; const hasAffectedSource = [...filterSources.values()] .some((source) => source.analyzed.dependsOn.some((name) => changed.includes(name))); - if (!hasAffectedSource) { await runAffectedWave(changed); return; } + if (!hasAffectedSource) { await runAffectedWave(changed, false, waveMs); return; } if (!(await preflight())) return; - const result = await runFilterSourceWave(changed); + const result = await runFilterSourceWave(changed, waveMs); // A `'superseded'` result means `applyFilterProviders` returned BEFORE // merging anything (its own stale-wave guard, `applyFilterProviders`'s // doc comment) — no consumer state changed, so there is nothing new to // `publish()`; the wave that superseded this one owns publishing the // eventual settled state. if (destroyed || result.status === 'superseded') return; - await runAffectedWave([...new Set([...changed, ...result.flipped])], true); + await runAffectedWave([...new Set([...changed, ...result.flipped])], true, waveMs); } async function setFilter(filterId: string, value: unknown): Promise { @@ -1430,6 +1522,42 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa await commitAndRerun([filter.def.parameter]); } + async function applyFilters( + entries: Array<{ filterId: string; value: string | string[]; active: boolean }>, + ): Promise { + if (destroyed) return; + // Resolve EVERY id up front, all-or-nothing: an unknown OR duplicate id + // aborts the whole call before any mutation (atomicity — matches + // `applyFilter`'s unknown-id no-op, extended across the batch so a partial + // time-range commit can never leave one bound applied and the other not). + const resolved: { filter: FilterRuntime; value: unknown; active: boolean }[] = []; + const seen = new Set(); + for (const entry of entries) { + const filter = filterById.get(entry.filterId); + if (!filter || seen.has(entry.filterId)) return; + seen.add(entry.filterId); + resolved.push({ filter, value: entry.value, active: entry.active }); + } + // Mutate every resolved entry (the filter bar owns activation, like + // `applyFilter`; `copyValue` defends against aliasing the caller's array), + // collecting the changed parameter names via the same `sameSelection` + + // active comparison `clearAllFilters` uses. + const changed: string[] = []; + for (const { filter, value, active } of resolved) { + const nextValue = copyValue(value); + if (filter.state.active !== active || !sameSelection(filter.state.value, nextValue)) { + changed.push(filter.def.parameter); + } + filter.state.value = nextValue; + filter.state.active = active; + } + // Nothing actually differs from the committed state → no publish, no wave + // (an identical-pair Apply is a true no-op, never a spurious rerun). + if (!changed.length) return; + publish(); + await commitAndRerun(changed); + } + async function clearFilter(filterId: string): Promise { if (destroyed) return; const filter = filterById.get(filterId); @@ -1507,8 +1635,8 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa return { state: stateSignal as ReadonlySignal, - controls, getFilterField, - start, refresh, refreshTile, setFilter, applyFilter, clearFilter, clearAllFilters, cancelTile, syncDocument, - setGridRenderMode, destroy, + controls, timeRangeGroups, getFilterField, + start, refresh, refreshTile, setFilter, applyFilter, applyFilters, clearFilter, clearAllFilters, + cancelTile, syncDocument, setGridRenderMode, destroy, }; } diff --git a/src/styles.css b/src/styles.css index 9aadb8ad..7328184c 100644 --- a/src/styles.css +++ b/src/styles.css @@ -1600,6 +1600,92 @@ body.detached-tab .graph-overlay-panel { .ms-btn-clear { margin-right: auto; } .ms-btn-primary { border: none; background: var(--accent); color: #fff; font-weight: 600; } .ms-btn-primary:hover { filter: brightness(1.08); } +/* Compound time-range control (#335, time-range-field.ts) — the SECOND + consumer of the #364 dialog-popover pattern (openAnchoredDialog). The + trigger reuses .var-input's sizing/border like .ms-trigger; the popover is + its own `position:fixed` two-column panel (staged From/To editors on the + left, a contextual recents/constants column on the right). All colours come + from the shared theme tokens so light + dark both work; D2 (filter-bar + + dashboard) may extend these rules. */ +.var-field.is-time-range { display: inline-flex; align-items: center; } +.trf-trigger { + display: inline-flex; align-items: center; + width: auto; max-width: 340px; + overflow: hidden; text-overflow: ellipsis; white-space: nowrap; + text-align: left; cursor: pointer; +} +.trf-trigger:hover { border-color: var(--accent); } +.trf-popover { + position: fixed; z-index: 70; max-width: calc(100vw - 24px); + display: flex; flex-direction: column; gap: 8px; padding: 10px; + background: var(--bg-panel, var(--bg-editor)); border: 1px solid var(--border); + border-radius: 8px; box-shadow: 0 8px 28px rgba(0,0,0,.4); + font-size: 12px; font-family: var(--mono); +} +.trf-cols { display: flex; gap: 12px; align-items: flex-start; } +.trf-left { display: flex; flex-direction: column; gap: 8px; } +.trf-row { display: flex; flex-direction: column; gap: 3px; } +.trf-field-label { font-size: 11px; color: var(--fg-mute); } +.trf-input-wrap { display: inline-flex; align-items: center; gap: 4px; } +.trf-input { width: 160px; } +.trf-caret { + height: 24px; width: 24px; flex-shrink: 0; + display: inline-flex; align-items: center; justify-content: center; cursor: pointer; + background: transparent; color: var(--fg-mute); + border: 1px solid var(--border); border-radius: 5px; font-size: 11px; font-family: inherit; +} +.trf-caret:hover { background: var(--bg-hover); color: var(--fg); } +.trf-caret[aria-pressed="true"] { border-color: var(--accent); color: var(--accent); } +.trf-preview { + font-size: 10.5px; color: var(--fg-mute); + white-space: nowrap; overflow: hidden; text-overflow: ellipsis; max-width: 190px; +} +.trf-preview.is-error { color: var(--error-fg); } +.trf-right { + width: 200px; display: flex; flex-direction: column; gap: 4px; + border-left: 1px solid var(--border-faint); padding-left: 12px; +} +.trf-right-header { + font-size: 10.5px; text-transform: uppercase; letter-spacing: .04em; color: var(--fg-faint); +} +.trf-right-body { display: flex; flex-direction: column; gap: 1px; max-height: 264px; overflow-y: auto; } +.trf-const, .trf-recent { + display: flex; align-items: center; gap: 8px; justify-content: space-between; width: 100%; + padding: 5px 6px; border-radius: 5px; cursor: pointer; + background: transparent; border: none; color: var(--fg); + font: inherit; text-align: left; +} +.trf-const:hover, .trf-recent:hover { background: var(--bg-hover); } +.trf-const-token { color: var(--accent); flex-shrink: 0; } +.trf-const-label { color: var(--fg-mute); overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } +.trf-empty { font-size: 11px; color: var(--fg-mute); padding: 5px 6px; } +.trf-range-error { font-size: 11px; color: var(--error-fg); } +.trf-range-error[hidden] { display: none; } +.trf-footer { display: flex; justify-content: flex-end; gap: 6px; padding-top: 6px; border-top: 1px solid var(--border-faint); } +.trf-btn { + height: 24px; padding: 0 10px; border-radius: 5px; cursor: pointer; + font: inherit; font-size: 11.5px; + border: 1px solid var(--border); background: transparent; color: var(--fg); +} +.trf-btn:hover { background: var(--bg-hover); } +.trf-btn-primary { border: none; background: var(--accent); color: #fff; font-weight: 600; } +.trf-btn-primary:hover { filter: brightness(1.08); } +.trf-btn-primary:disabled { opacity: .5; cursor: default; } +.trf-btn-primary:disabled:hover { filter: none; } +/* #335 D2 (filter-bar integration): the "Time"/"Filters" section labels and + the separator between the compound time-range control and the per-param + fields. Both sit in the same flex row as the fields (`.dash-filters`), so + they align to the field baseline and stay reachable when the bar scrolls/ + wraps. All colours come from the shared theme tokens (light + dark). */ +.dash-filters .flabel { + align-self: center; flex-shrink: 0; + font-size: 10.5px; text-transform: uppercase; letter-spacing: .04em; + color: var(--fg-faint); font-weight: 600; white-space: nowrap; +} +.dash-filters .trf-sep { + align-self: stretch; flex-shrink: 0; + width: 1px; min-height: 22px; background: var(--border-faint); margin: 2px 0; +} .run-btn { height: 26px; padding: 0 12px 0 10px; diff --git a/src/ui/dashboard.ts b/src/ui/dashboard.ts index 42f253ce..befc7ba4 100644 --- a/src/ui/dashboard.ts +++ b/src/ui/dashboard.ts @@ -55,6 +55,8 @@ import { selectOutputColumns } from '../core/select-columns.js'; import { renderKpiCards, KPI_STREAM_ARIA } from './kpi-panel.js'; import { buildFilterBar } from './filter-bar.js'; import type { FilterBarApp, FilterBarHandle } from './filter-bar.js'; +import { pushRecentRange } from '../core/time-range.js'; +import type { DashboardTimeRangeGroup, TimeRangeRecent } from '../core/time-range.js'; import { createDashboardViewerSession } from '../dashboard/application/dashboard-viewer-session.js'; import type { DashboardViewerSession, DashboardViewState, ViewerTileState, ViewerFilterState, @@ -644,27 +646,35 @@ export async function renderDashboard(app: DashboardApp): Promise { // up. Replaced wholesale after every rebuild (never merged) — a filter that // disappears from `sview.filters` simply drops out. let lastBuiltOptionsRev = new Map(); + // #335: shell-owned, session-lifetime per-group "Recently used" ranges, + // keyed by `group.key`. NOT persisted in v1 (owner decision) and naturally + // discarded when this `renderDashboard` call's session is torn down or the + // dashboard switches (a fresh render builds a fresh map). Each successful, + // changing commit pushes the OUTGOING committed pair (see `onApplyTimeRange` + // in `rebuildFilterBar`). + const timeRangeRecents = new Map(); function rebuildFilterBar(sview: DashboardViewState): void { - // #189-F2b: ask the OUTGOING bar WHICH parameter's multiselect popover is - // open (if any) BEFORE disposing it — disposing while open is that - // field's own silent Cancel (multi-select-field.ts), so this is the only - // chance to notice it, tell an assistive-tech user their popover just - // closed out from under them (the shared `filterRefreshLiveEl`, never - // torn down by the rebuild), and move focus to that SAME parameter's - // trigger on the freshly-built bar below (never left stranded at - // `` — F2 review finding). - const openMultiSelectParam = currentFilterBar?.openMultiSelectParam() ?? null; + // #189-F2b, GENERALIZED (#335): ask the OUTGOING bar WHICH control's + // popover is open (if any) BEFORE disposing it — disposing while open is + // that control's own silent Cancel (multi-select-field.ts / + // time-range-field.ts), so this is the only chance to notice it, tell an + // assistive-tech user their popover just closed out from under them (the + // shared `filterRefreshLiveEl`, never torn down by the rebuild), and move + // focus to that SAME control's trigger on the freshly-built bar below + // (never left stranded at `` — F2 review finding). The key is a + // parameter name for a multiselect field, `group:…` for a time-range one. + const openPopoverKey = currentFilterBar?.openPopoverKey() ?? null; // Maintainer merge-gate fix (#189): an ordinary Apply already closed its - // OWN popover before its `onApply` reached `session.applyFilter` — by the - // time that commit's synchronous `publish()` gets here, `openMultiSelectParam` - // above already reads `null` for it. `focusedMultiSelectParam` still finds - // it (focus sits on that field's about-to-be-detached trigger), so focus + // OWN popover before its commit callback reached the session — by the + // time that commit's synchronous `publish()` gets here, `openPopoverKey` + // above already reads `null` for it. `focusedFieldKey` still finds it + // (focus sits on that control's about-to-be-detached trigger), so focus // restoration below has a signal to work with even when there was no open // popover to speak of — never used for the ANNOUNCE decision (only a // genuinely open popover's cancellation is ever worth announcing). - const focusedMultiSelectParam = currentFilterBar?.focusedMultiSelectParam() ?? null; - const restoreFocusParam = openMultiSelectParam ?? focusedMultiSelectParam; + const focusedFieldKey = currentFilterBar?.focusedFieldKey() ?? null; + const restoreFocusKey = openPopoverKey ?? focusedFieldKey; currentFilterBar?.dispose(); const idByParam = new Map(); // #360: curation is gated on TOPOLOGY (`sourceId != null`, set once at @@ -721,8 +731,58 @@ export async function renderDashboard(app: DashboardApp): Promise { if (id) session.applyFilter(id, next, active); }; const getField = (name: string, mode: ValidationMode) => session.getFilterField(name, mode, draftValues, draftActive); + // #335: assemble the time-range option — one entry per resolved group, + // reading each bound's committed value/active straight off `sview.filters` + // (the from/to filters stay in the view regardless of presentation, so a + // time-range commit still flips `barSig` below and rebuilds this bar). The + // pair's two individual fields are suppressed by parameter name inside + // `buildFilterBar`. `waveNowMs` is this wave's shared `now` snapshot. + const filterById = new Map(sview.filters.map((f) => [f.id, f] as const)); + const timeRange = session.timeRangeGroups.flatMap((group) => { + const fromF = filterById.get(group.fromFilterId); + const toF = filterById.get(group.toFilterId); + if (!fromF || !toF) return []; + return [{ + group, + fromValue: valueString(fromF.value), + toValue: valueString(toF.value), + active: fromF.active && toF.active, + waveNowMs: sview.waveWallNowMs, + recents: (): readonly TimeRangeRecent[] => timeRangeRecents.get(group.key) ?? [], + }]; + }); + // #335: a time-range Apply (or immediate recents pick) commits BOTH bounds + // atomically through the session's batch API (one execution wave over the + // union of the pair's resolved targets), pushes the OUTGOING committed pair + // onto this group's recents, and announces the new range. + const onApplyTimeRange = (group: DashboardTimeRangeGroup, from: string, to: string): void => { + const fromF = filterById.get(group.fromFilterId); + const toF = filterById.get(group.toFilterId); + const outFrom = fromF ? valueString(fromF.value) : ''; + const outTo = toF ? valueString(toF.value) : ''; + const wasActive = !!(fromF?.active && toF?.active); + // Push the OUTGOING pair only when it was active + both bounds non-empty + // AND actually differs from the incoming pair — so a first commit from an + // unset/inactive range, and a no-op re-apply, both push nothing. + if (wasActive && outFrom !== '' && outTo !== '' && (outFrom !== from || outTo !== to)) { + timeRangeRecents.set(group.key, + pushRecentRange(timeRangeRecents.get(group.key) ?? [], { from: outFrom, to: outTo })); + } + void session.applyFilters([ + { filterId: group.fromFilterId, value: from, active: true }, + { filterId: group.toFilterId, value: to, active: true }, + ]); + // Announce through the SAME persistent live region the #189 refresh + // announcement uses (a sibling of `filterHost`, so it survives the + // rebuild `applyFilters` synchronously triggers). Set AFTER the commit so + // the synchronous rebuild's own announce path (which never fires for a + // time-range key anyway — `group:…` is not a multiselect parameter) can + // never clobber it. + filterRefreshLiveEl.textContent = `Time range applied: ${from} → ${to}`; + }; const bar = buildFilterBar( - filterBarApp, session.controls, onCommit, getField, { curatedFields, document: doc, onApplyCurated }, + filterBarApp, session.controls, onCommit, getField, + { curatedFields, document: doc, onApplyCurated, timeRange, onApplyTimeRange }, ); filterHost.replaceChildren(bar.el); currentFilterBar = bar; @@ -735,23 +795,24 @@ export async function renderDashboard(app: DashboardApp): Promise { // commit) never bumps `optionsRev`, so it never announces, even on the // rare chance this param's popover was still genuinely open when some // unrelated commit forced the whole bar to rebuild. - if (openMultiSelectParam) { - const prevRev = lastBuiltOptionsRev.get(openMultiSelectParam); - const nextRev = sview.filters.find((f) => f.parameter === openMultiSelectParam)?.optionsRev; + if (openPopoverKey) { + const prevRev = lastBuiltOptionsRev.get(openPopoverKey); + const nextRev = sview.filters.find((f) => f.parameter === openPopoverKey)?.optionsRev; if (nextRev !== undefined && nextRev !== prevRev) { filterRefreshLiveEl.textContent = 'Filter options were refreshed'; } } lastBuiltOptionsRev = new Map(sview.filters.map((f) => [f.parameter, f.optionsRev])); - // #189-F2b: land focus on the NEW bar's corresponding trigger for - // whichever parameter the OUTGOING bar had open, or (absent that) had - // focus on its trigger (an Apply that already closed its own popover - // before reaching here) — a no-op if that parameter is no longer a - // multiselect field on the fresh bar (e.g. its curation topology itself - // changed) or there was no such parameter at all (a plain field mid-typing - // elsewhere is never disturbed), which simply leaves focus wherever it - // already was rather than throwing. - if (restoreFocusParam) bar.focusMultiSelectTrigger(restoreFocusParam); + // #189-F2b, GENERALIZED (#335): land focus on the NEW bar's corresponding + // trigger for whichever control key the OUTGOING bar had open, or (absent + // that) had focus on its trigger (an Apply that already closed its own + // popover before reaching here) — a no-op if that key is no longer a + // popover-bearing control on the fresh bar (e.g. its topology changed) or + // there was no such control at all (a plain field mid-typing elsewhere is + // never disturbed), which simply leaves focus wherever it already was + // rather than throwing. Works uniformly for multiselect (`param`) and + // time-range (`group:…`) keys. + if (restoreFocusKey) bar.focusFieldTrigger(restoreFocusKey); } const filterDiagnosticsHost = h('div', { class: 'dash-filter-diagnostics' }); @@ -1681,6 +1742,13 @@ export async function renderDashboard(app: DashboardApp): Promise { // is detected here instead and applied to the EXISTING bar via // `filterBarUpdateStatus` (no rebuild). let statusSig = ''; + // #335: the wave `now` the time-range controls' closed labels were last + // resolved against — a NON-rebuild publish whose wave `now` advanced + // re-resolves those labels in place (a live relative range, no timers), + // without disturbing anything else. Tracked separately from `barSig` so a + // tile-progress tick (same wave `now`) never churns the labels. Seeded from + // the session's initial state (`null` before the first wave). + let lastLabelWaveNowMs: number | null = session.state.value.waveWallNowMs; const statusSigOf = (filters: readonly ViewerFilterState[]): string => JSON.stringify(filters.map((f) => [f.parameter, f.status, !!f.stale, f.waitingFor ?? null])); const statesByParam = (filters: readonly ViewerFilterState[]): Record { const sig = JSON.stringify(sview.filters.map((f) => [f.id, f.active, sigValue(f.value), f.optionsRev, f.sourceId != null])); const newStatusSig = statusSigOf(sview.filters); + let rebuilt = false; if (sig !== barSig) { barSig = sig; rebuildFilterBar(sview); + rebuilt = true; // A fresh rebuild already applies the CURRENT status to every curated // field (buildFilterBar applies it at build time) — refresh the stored // status signature too, so this same publish doesn't ALSO fire a @@ -1755,6 +1825,15 @@ export async function renderDashboard(app: DashboardApp): Promise { statusSig = newStatusSig; filterBarUpdateStatus?.(statesByParam(sview.filters)); } + // #335: per-wave time-range label refresh. A rebuild (`sig` change) already + // rebuilt every time-range control against this wave's `now` (assembled + // into its `waveNowMs`); only a NON-rebuild publish whose wave `now` + // advanced needs the closed labels re-resolved in place — a committed + // relative range (`-1d` → `now`) moves per wave without any bar rebuild. + if (!rebuilt && sview.waveWallNowMs != null && sview.waveWallNowMs !== lastLabelWaveNowMs) { + currentFilterBar?.refreshTimeRangeLabels(sview.waveWallNowMs); + } + lastLabelWaveNowMs = sview.waveWallNowMs; // #303: persist committed filter value/active into the isolated per-dashboard // store — isolated from the Workbench's asb:varValues/asb:filterActive keys. const filterBag = persistBagOf(sview.filters); diff --git a/src/ui/dom.ts b/src/ui/dom.ts index f6eb351d..33de9730 100644 --- a/src/ui/dom.ts +++ b/src/ui/dom.ts @@ -84,22 +84,37 @@ export interface FixedAnchorOptions { gap?: number; min?: number; viewportW?: number; + /** Panel width for the left-align right-edge CLAMP (#335). Only consulted + * alongside `viewportW`; when both are given the anchor stays left-aligned + * under the trigger but its left inset is lowered so `left + panelW` never + * crosses `viewportW - min`. `panelW` alone (no `viewportW`) is ignored. */ + panelW?: number; } // Place a fixed-position popover anchored under a button. Returns -// `{ top, left }`, or `{ top, right }` when `viewportW` is given (right-align to -// the anchor's right edge). `gap` is the px below the anchor; `min` floors the -// side inset. Pure arithmetic on a DOMRect-like — the single recipe for the File -// menu, the Save popover and the user menu. +// `{ top, left }`, or `{ top, right }` when `viewportW` is given WITHOUT +// `panelW` (right-align to the anchor's right edge). With BOTH `viewportW` and +// `panelW` it left-aligns but clamps the left inset so a `panelW`-wide panel +// stays inside the viewport's right edge (#335). `gap` is the px below the +// anchor; `min` floors the side inset. Pure arithmetic on a DOMRect-like — the +// single recipe for the File menu, the Save popover, the user menu, and the +// dashboard filter popovers. export function fixedAnchor( rect: AnchorRect, opts: FixedAnchorOptions = {}, ): { top: number; left: number } | { top: number; right: number } { const gap = opts.gap != null ? opts.gap : 6; const min = opts.min != null ? opts.min : 8; const top = rect.bottom + gap; - return opts.viewportW != null - ? { top, right: Math.max(min, opts.viewportW - rect.right!) } - : { top, left: Math.max(min, rect.left!) }; + if (opts.viewportW != null && opts.panelW == null) { + return { top, right: Math.max(min, opts.viewportW - rect.right!) }; + } + let left = Math.max(min, rect.left!); + if (opts.viewportW != null && opts.panelW != null) { + // Furthest-right inset that still fits the panel with a `min` gutter. + const maxLeft = Math.max(min, opts.viewportW - opts.panelW - min); + left = Math.min(left, maxLeft); + } + return { top, left }; } // Wire a modal backdrop's close-on-click without the false positive from a diff --git a/src/ui/filter-bar.ts b/src/ui/filter-bar.ts index a0eaf2b0..5c4ebd6e 100644 --- a/src/ui/filter-bar.ts +++ b/src/ui/filter-bar.ts @@ -24,7 +24,8 @@ import type { ComboField } from './combobox.js'; import { buildFilterOptionField } from './filter-option-field.js'; import type { FilterFieldOption } from './filter-option-field.js'; import { buildMultiSelectField } from './multi-select-field.js'; -import type { MultiSelectFieldHandle } from './multi-select-field.js'; +import { buildTimeRangeField } from './time-range-field.js'; +import type { DashboardTimeRangeGroup, TimeRangeRecent } from '../core/time-range.js'; import type { WorkbenchParameterSession } from '../application/workbench-parameter-session.js'; /** The narrow slice of the real `app` controller this module reads — not the @@ -66,6 +67,28 @@ export interface BuildFilterBarOptions { * all) — `curated.selection` is never present then either, so the seam is * simply never reached. */ onApplyCurated?(name: string, next: string[], active: boolean): void; + /** #335: one entry per resolved `DashboardTimeRangeGroup` — the shell + * (`dashboard.ts`) assembles these from `session.timeRangeGroups` + + * `sview.filters` + `sview.waveWallNowMs`, and this bar renders one compound + * `buildTimeRangeField` control per entry in a "Time" section AHEAD of the + * per-param fields (the pair's own two individual fields are then SUPPRESSED + * from the per-param loop — the compound control represents them). Left + * undefined/empty by a caller with no groups (every workbench/detached + * caller, or a dashboard whose filters form no date-like pair): the bar + * then renders byte-identical DOM to the pre-#335 no-time-range path. */ + timeRange?: Array<{ + group: DashboardTimeRangeGroup; + fromValue: string; + toValue: string; + active: boolean; + waveNowMs: number | null; + recents: () => readonly TimeRangeRecent[]; + }>; + /** #335: fires when a time-range control commits both bounds (its Apply, or + * an immediate recents pick) — the caller (`dashboard.ts`) routes it to + * `session.applyFilters` over the group's from/to filter ids. Only reached + * when `timeRange` built at least one control. */ + onApplyTimeRange?(group: DashboardTimeRangeGroup, from: string, to: string): void; } /** #360: `status`/`stale`/`waitingFor` mirror `ViewerFilterState`'s own @@ -106,10 +129,11 @@ interface CuratedFieldConfig extends CuratedFieldStatus { active?: boolean; } -/** A built curated field's retained handle (#360) — kept in - * `buildFilterBar`'s `curatedHandles` map so a LATER status-only change can - * update this exact field's affordance in place (`applyFieldStatus`, via - * the returned `updateStatus`) without rebuilding the whole input — the +/** A built curated field's retained handle (#360) — referenced by its + * `FieldHandle` adapter in `buildFilterBar`'s unified handle map (#335) so a + * LATER status-only change can update this exact field's affordance in place + * (`applyFieldStatus`, via the returned `updateStatus`) without rebuilding + * the whole input — the * rebuild would otherwise blow away in-progress typing on every other field * in the bar and drop this field's own combobox/focus state. `baseTitle`/ * `basePlaceholder` are this field's non-status tooltip/placeholder @@ -125,6 +149,32 @@ interface CuratedFieldHandle { noteEl: HTMLElement | null; } +/** #335 handle-map unification: the ONE contract every retained field control + * in the bar is addressed through — replacing the two pre-#335 parallel maps + * (`curatedHandles` + `multiSelectFields`) plus the new time-range controls + * with a single `Map` keyed by an OPAQUE string: a + * parameter name for a per-param field, `group:${group.key}` for a time-range + * control (a ClickHouse parameter name can never contain `:`, so the two + * key-spaces never collide). `buildMultiSelectField`/`buildTimeRangeField` + * already return this shape directly; the data-bag curated scalar field is + * wrapped in a small adapter. `el` is present only for controls that + * participate in the popover focus-restore dance (multiselect + time-range) — + * the curated scalar adapter omits it, so `focusedFieldKey` skips it exactly + * as the pre-#335 `focusedMultiSelectParam` did. `refreshLabel` is carried + * only by time-range handles (folded by `refreshTimeRangeLabels`). */ +interface FieldHandle { + el?: HTMLElement; + updateStatus(s: CuratedFieldStatus): void; + /** Present on the popover-bearing controls (multiselect + time-range); the + * curated scalar adapter omits them (it has no popover, no focusable + * trigger a rebuild restores, and nothing to tear down), so every fold over + * the map that needs one uses optional chaining. */ + isOpen?(): boolean; + focusTrigger?(): void; + dispose?(): void; + refreshLabel?(nowMs: number): void; +} + /** * Applies a curated field's status affordance to its already-built DOM * (#360) — the SAME class/disabled/note logic `buildFilterBar` @@ -234,9 +284,9 @@ export const FILTER_DEBOUNCE_MS = 500; * * #360: `updateStatus` applies a per-param `CuratedFieldStatus` * update to whichever curated fields this SAME bar instance already built - * (`curatedHandles`, keyed by parameter) — a param this bar never curated - * (absent from `curatedFields` at build time, or a plain field) is silently - * ignored. The caller (`dashboard.ts`'s `rebuildFilterBar`) uses this for a + * (the unified handle map, keyed by parameter name for a per-param field — + * #335) — a param this bar never curated (absent from `curatedFields` at + * build time, or a plain field) is silently ignored. The caller (`dashboard.ts`'s `rebuildFilterBar`) uses this for a * status-only change (e.g. `loading` → `ready`, no value/active/options * change) instead of tearing down and rebuilding the whole bar — preserving * in-progress typing on every OTHER field, and this field's own combobox/ @@ -245,42 +295,48 @@ export interface FilterBarHandle { el: HTMLElement; dispose(): void; updateStatus(states: Record): void; - /** #189, #189-F2b: the PARAMETER of a curated MULTISELECT field built by - * THIS bar instance that currently has its popover open, or `null` when - * none does (including a bar that built no multiselect field at all — the - * empty-`params` bar too). The caller (`dashboard.ts`) reads this BEFORE - * disposing an outgoing bar (a rebuild always disposes the old bar - * outright) to decide whether a refresh announcement is owed — disposing - * a multiselect field while its popover is open silently Cancels it (no - * `onApply`, see multi-select-field.ts), so without an announcement the - * user's open popover would simply vanish. Replaces the pre-F2b boolean - * `hasOpenMultiSelect()` — the caller needs to know WHICH field, so it can - * move focus to that same parameter's trigger on the freshly-built bar - * (`focusMultiSelectTrigger` below) rather than leaving focus stranded at - * ``. */ - openMultiSelectParam(): string | null; - /** Maintainer merge-gate fix (#189): the parameter of a curated MULTISELECT - * field built by THIS bar instance whose trigger (or error-mode fallback - * input) currently HOLDS FOCUS, popover open or not — or `null` when none - * does. Distinct from `openMultiSelectParam` above: an ordinary Apply - * closes its own popover BEFORE calling `onApply` (multi-select-field.ts), - * so by the time a synchronous commit-triggered rebuild reaches this bar, - * `openMultiSelectParam()` already reads `null` even though focus still - * sits on that field's (about-to-be-detached) trigger — this is the only - * remaining signal for which parameter's fresh trigger a rebuild should - * refocus. The caller (`dashboard.ts`) reads BOTH before disposing the - * outgoing bar and restores focus for whichever one is non-null - * (`openMultiSelectParam() ?? focusedMultiSelectParam()`), so a plain - * field mid-typing (focus outside every multiselect control) is never - * disturbed. */ - focusedMultiSelectParam(): string | null; - /** #189-F2b: focuses the named parameter's multiselect trigger (or its - * error-mode fallback input, if erroring) — a no-op when this bar built no - * multiselect field for that parameter. Used by `dashboard.ts` right after - * building a FRESH bar, for whichever parameter `openMultiSelectParam()` - * (or, absent that, `focusedMultiSelectParam()`) reported on the OUTGOING - * bar just before disposing it. */ - focusMultiSelectTrigger(name: string): void; + /** #189, #189-F2b, GENERALIZED (#335): the opaque KEY of a popover-bearing + * control built by THIS bar instance that currently has its popover open, + * or `null` when none does (including a bar that built no such control at + * all — the empty-`params` bar too). The key is the parameter name for a + * curated MULTISELECT field, `group:${group.key}` for a time-range control. + * The caller (`dashboard.ts`) reads this BEFORE disposing an outgoing bar + * (a rebuild always disposes the old bar outright) to decide whether a + * refresh announcement is owed — disposing a control while its popover is + * open silently Cancels it (no commit, see multi-select-field.ts / + * time-range-field.ts), so without an announcement the user's open popover + * would simply vanish — and to move focus to that same key's trigger on the + * freshly-built bar (`focusFieldTrigger` below) rather than leaving focus + * stranded at ``. */ + openPopoverKey(): string | null; + /** Maintainer merge-gate fix (#189), GENERALIZED (#335): the opaque KEY of a + * field control built by THIS bar instance whose own root (`FieldHandle.el` + * — a multiselect's trigger/error input, or a time-range control's trigger) + * currently HOLDS FOCUS, popover open or not — or `null` when none does. + * Distinct from `openPopoverKey` above: an ordinary Apply closes its own + * popover BEFORE calling its commit callback (multi-select-field.ts / + * time-range-field.ts, the shared `openAnchoredDialog` contract), so by the + * time a synchronous commit-triggered rebuild reaches this bar, + * `openPopoverKey()` already reads `null` even though focus still sits on + * that field's (about-to-be-detached) trigger — this is the only remaining + * signal for which control's fresh trigger a rebuild should refocus. The + * caller (`dashboard.ts`) reads BOTH before disposing the outgoing bar and + * restores focus for whichever is non-null (`openPopoverKey() ?? + * focusedFieldKey()`), so a plain field mid-typing (focus outside every + * popover-bearing control) is never disturbed. */ + focusedFieldKey(): string | null; + /** #189-F2b, GENERALIZED (#335): focuses the keyed control's trigger (a + * multiselect's trigger/error-mode input, or a time-range control's + * trigger) — a no-op when this bar built no such control for that key. Used + * by `dashboard.ts` right after building a FRESH bar, for whichever key + * `openPopoverKey()` (or, absent that, `focusedFieldKey()`) reported on the + * OUTGOING bar just before disposing it. */ + focusFieldTrigger(key: string): void; + /** #335: re-resolve every time-range control's closed-trigger label + aria + * against a new wall-clock snapshot (per execution wave, no timers) — a + * relative range (`-1d` → `now`) re-displays its absolute bounds without a + * bar rebuild. A no-op when this bar built no time-range control. */ + refreshTimeRangeLabels(nowMs: number): void; } /** @@ -317,22 +373,27 @@ export function buildFilterBar( return { el: h('div', { ...attrs, style: { display: 'none' } }), dispose: () => {}, updateStatus: () => {}, - openMultiSelectParam: () => null, focusedMultiSelectParam: () => null, - focusMultiSelectTrigger: () => {}, + openPopoverKey: () => null, focusedFieldKey: () => null, + focusFieldTrigger: () => {}, refreshTimeRangeLabels: () => {}, }; } const timerClears: Array<() => void> = []; - // #360: every curated (scalar single-select) field's retained handle, - // keyed by parameter — see `CuratedFieldHandle` and - // `FilterBarHandle.updateStatus`. - const curatedHandles = new Map(); - // #189: every curated MULTISELECT field's own handle, keyed by parameter — - // a separate map (its `updateStatus`/`isOpen`/`dispose` are its own, not - // `CuratedFieldHandle`'s DOM-patching recipe) that `updateStatus`/`dispose`/ - // `openMultiSelectParam`/`focusMultiSelectTrigger` below all fold in - // alongside `curatedHandles`. - const multiSelectFields = new Map(); - const el = h('div', attrs, ...params.map((p) => { + // #335 handle-map unification: ONE map (see `FieldHandle`) keyed by the + // opaque control key — a parameter name for a per-param field (curated + // scalar, multiselect, or plain), `group:${group.key}` for a time-range + // control — replacing the pre-#335 parallel `curatedHandles` + + // `multiSelectFields` maps. `updateStatus`/`dispose`/`openPopoverKey`/ + // `focusedFieldKey`/`focusFieldTrigger`/`refreshTimeRangeLabels` all fold + // over this one map. + const handles = new Map(); + // #335: the time-range group entries, and the set of parameter names those + // groups OWN — those params are represented by the compound control and so + // are suppressed from the per-param loop below. + const timeRange = options.timeRange ?? []; + const suppressed = new Set(); + for (const tr of timeRange) { suppressed.add(tr.group.fromParameter); suppressed.add(tr.group.toParameter); } + + const buildParamField = (p: FieldControl): HTMLElement => { let timer: ReturnType | null = null; timerClears.push(() => { if (timer != null) clearTimeout(timer); timer = null; }); // #173 acceptance (review F1): a type-conflicted param (declared with @@ -383,7 +444,8 @@ export function buildFilterBar( onCommit(p.name); }, }); - multiSelectFields.set(p.name, msField); + // #335: a multiselect handle already satisfies `FieldHandle` directly. + handles.set(p.name, msField); return h('label', { class: 'var-field is-curated' + (p.optional ? ' is-optional' : '') }, h('span', { class: 'var-name' }, p.name), msField.el); } @@ -431,7 +493,12 @@ export function buildFilterBar( // the handle so a LATER status-only change updates this same field in // place via `updateStatus`, never a rebuild. applyFieldStatus(handle, { status: curated.status, stale: curated.stale, waitingFor: curated.waitingFor }); - curatedHandles.set(p.name, handle); + // #335: the data-bag curated scalar handle wrapped in a minimal + // `FieldHandle` adapter — only `updateStatus` (re-runs `applyFieldStatus` + // in place) is meaningful. No popover, no `el` (so `focusedFieldKey` + // skips it, exactly as the pre-#335 code did), nothing to dispose — the + // optional `isOpen`/`focusTrigger`/`dispose` are simply absent. + handles.set(p.name, { updateStatus: (s) => applyFieldStatus(handle, s) }); return label; } const commitNow = (): void => { @@ -502,44 +569,86 @@ export function buildFilterBar( applyFieldState(input, getField(p.name, 'execute'), baseTitle, combo?.previewEl); return h('label', { class: 'var-field' + (p.optional ? ' is-optional' : '') }, h('span', { class: 'var-name' }, p.name), combo.el); - })); + }; + + // #335: the "Time" section — a `.flabel` heading + one compound time-range + // control per group + a separator — rendered AHEAD of the per-param fields. + // Each control's handle is registered under `group:${group.key}` so it + // participates in the unified map's status/dispose/focus/refresh folds. Its + // Apply (and immediate recents pick) route through `onApplyTimeRange`. + const timeSection: (HTMLElement | null)[] = []; + if (timeRange.length) { + timeSection.push(h('span', { class: 'flabel' }, 'Time')); + for (const tr of timeRange) { + const trField = buildTimeRangeField({ + document, group: tr.group, + fromValue: tr.fromValue, toValue: tr.toValue, active: tr.active, + waveNowMs: tr.waveNowMs, wallNow: app.wallNow, getRecents: tr.recents, + onApply: (from, to) => options.onApplyTimeRange?.(tr.group, from, to), + }); + handles.set(`group:${tr.group.key}`, trField); + timeSection.push(trField.el); + } + timeSection.push(h('span', { class: 'trf-sep', 'aria-hidden': 'true' })); + } + + // The per-param fields (every param NOT owned by a time-range group). + const perParamFields = params.filter((p) => !suppressed.has(p.name)).map(buildParamField); + + // Compose: Time section, then a "Filters" section label (only when BOTH a + // Time section rendered AND at least one non-group field remains), then the + // per-param fields. With no time-range groups `timeSection` is empty and no + // "Filters" label renders, so the child list is byte-identical to the + // pre-#335 `...params.map(...)` output. + const children: (HTMLElement | null)[] = [...timeSection]; + if (timeRange.length && perParamFields.length) children.push(h('span', { class: 'flabel' }, 'Filters')); + children.push(...perParamFields); + const el = h('div', attrs, ...children); return { el, dispose: () => { timerClears.forEach((clear) => clear()); - // Disposing a multiselect field WHILE its popover is open is that - // field's own Cancel (no `onApply` call, see multi-select-field.ts) — - // a bar rebuild/teardown always tears every open popover down this way. - for (const msField of multiSelectFields.values()) msField.dispose(); + // Disposing a control WHILE its popover is open is that control's own + // Cancel (no commit callback, see multi-select-field.ts / + // time-range-field.ts) — a bar rebuild/teardown always tears every open + // popover down this way. The curated scalar adapter has no `dispose`. + for (const handle of handles.values()) handle.dispose?.(); }, updateStatus: (states) => { - for (const [name, handle] of curatedHandles) { - const s = states[name]; - if (s) applyFieldStatus(handle, s); - } - for (const [name, msField] of multiSelectFields) { - const s = states[name]; - if (s) msField.updateStatus(s); + // #335: one loop over the unified map. A per-param field's key IS its + // parameter name, so `states[key]` finds its status; a time-range + // control's key is `group:…` (never a parameter name), so it never + // matches a status entry — and its `updateStatus` is a no-op regardless. + for (const [key, handle] of handles) { + const s = states[key]; + if (s) handle.updateStatus(s); } }, - // #189-F2b: read by the caller BEFORE disposing this bar (a rebuild), to - // decide whether an outgoing popover's forced Cancel deserves a refresh - // announcement AND which parameter's fresh trigger should receive focus - // — see `dashboard.ts`'s `rebuildFilterBar`. - openMultiSelectParam: () => { - for (const [name, msField] of multiSelectFields) if (msField.isOpen()) return name; + // #189-F2b, GENERALIZED (#335): read by the caller BEFORE disposing this + // bar (a rebuild), to decide whether an outgoing popover's forced Cancel + // deserves a refresh announcement AND which control's fresh trigger should + // receive focus — see `dashboard.ts`'s `rebuildFilterBar`. + openPopoverKey: () => { + for (const [key, handle] of handles) if (handle.isOpen?.()) return key; return null; }, - // Maintainer merge-gate fix (#189): `.el` is each field's own control root - // (the single node hosting whichever of trigger/error-input is current — - // see multi-select-field.ts), so `.contains(activeElement)` catches focus - // on either one, regardless of popover state. - focusedMultiSelectParam: () => { + // `.el` is each popover-bearing control's own root (a multiselect's + // trigger/error-input node, a time-range control's trigger wrapper), so + // `.contains(activeElement)` catches focus on it regardless of popover + // state. The curated scalar adapter has no `el` and is skipped (its focus + // was never a rebuild restore target pre-#335 either). + focusedFieldKey: () => { const active = document.activeElement; if (!active) return null; - for (const [name, msField] of multiSelectFields) if (msField.el.contains(active)) return name; + for (const [key, handle] of handles) if (handle.el && handle.el.contains(active)) return key; return null; }, - focusMultiSelectTrigger: (name) => { multiSelectFields.get(name)?.focusTrigger(); }, + focusFieldTrigger: (key) => { handles.get(key)?.focusTrigger?.(); }, + // #335: fold `refreshLabel` over the map — only time-range handles carry + // it; every other handle skips (optional chaining), so this is a no-op for + // a bar with no time-range controls. + refreshTimeRangeLabels: (nowMs) => { + for (const handle of handles.values()) handle.refreshLabel?.(nowMs); + }, }; } diff --git a/src/ui/multi-select-field.ts b/src/ui/multi-select-field.ts index d2603686..0c29dd53 100644 --- a/src/ui/multi-select-field.ts +++ b/src/ui/multi-select-field.ts @@ -4,10 +4,13 @@ // combobox primitive (combobox.ts) into multiselect semantics it was never // built for. This module borrows conventions from TWO existing primitives // rather than inventing new ones: -// - `menu.ts`'s `openMenu` is the model for the popover lifecycle: mount a -// fresh overlay + panel on open, tear both down completely on close -// (never a hidden-but-resident node), Escape closes and refocuses the -// trigger, and `fixedAnchor` places the panel under the trigger. +// - `popover.ts`'s `openAnchoredDialog` (#335) owns the generic dialog +// chrome — mount a fresh overlay + panel on open, tear both down completely +// on close (never a hidden-but-resident node), Escape/backdrop close and +// refocus the trigger, the ARIA `dialog`/`aria-modal`/`aria-expanded` +// lifecycle, the Tab focus trap, and `fixedAnchor` placement under the +// trigger. This module keeps only the multiselect-specific content, draft, +// busy affordance, and Apply/close ordering. // - `filter-bar.ts`'s `applyFieldStatus` is the model for the status // vocabulary (idle/loading/ready/waiting/source-error/helper-error/ // missing-helper, `stale`/`waitingFor`) and its is-waiting/is-error/ @@ -32,7 +35,8 @@ // listeners, all local to `openPopover()` — none of it survives past the // matching `close()`, so there is nothing to leak across repeated opens. -import { h, fixedAnchor, attachBackdropClose } from './dom.js'; +import { h } from './dom.js'; +import { openAnchoredDialog } from './popover.js'; import { idSafe } from './combobox.js'; import { canonicalizeSelection, sameSelection } from '../core/filter-selection.js'; @@ -96,7 +100,7 @@ export interface MultiSelectFieldHandle { isOpen(): boolean; /** Focuses this control's own current interactive element (the trigger, or * the error-mode fallback input when erroring) — #189 F2b: a caller - * (`filter-bar.ts`'s `focusMultiSelectTrigger`) that just rebuilt the bar + * (`filter-bar.ts`'s `focusFieldTrigger`) that just rebuilt the bar * a still-open popover was force-closed out from under uses this to move * focus onto the corresponding field of the FRESH bar (never left at * ``). A no-op-safe call before `applyStatus()` has ever run is not @@ -290,8 +294,11 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi const onTriggerClick = (): void => { if (!trigger.disabled) openPopover(); }; trigger.addEventListener('click', onTriggerClick); - // Mount a fresh popover (menu.ts's own lifecycle convention: build on - // open, tear down completely on close — never a hidden-but-resident node). + // Mount a fresh popover. The generic dialog chrome (overlay/backdrop, + // ARIA dialog + aria-expanded lifecycle, Escape, Tab trap, placement, focus + // return) lives in `openAnchoredDialog` (#335); this function builds only + // the multiselect content + draft and wires the busy affordance and the + // Apply/close ordering on top of it. function openPopover(): void { if (closeCurrent) return; // already open — never stack a second popover // #189 F1: a raw-string committed value (the error-mode fallback commit, @@ -370,7 +377,7 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi for (const row of rows) row.cb.checked = false; syncSelectAll(); }); - cancelBtn.addEventListener('click', () => close()); + cancelBtn.addEventListener('click', () => handle.close()); applyBtn.addEventListener('click', () => { const canonical = canonicalizeSelection([...draft], options); // #189 F1: a raw-string committed value (the error-mode fallback commit) @@ -382,30 +389,34 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi // A no-op Apply (same canonical selection AND same active flag) closes // silently — `onApply` fires exactly once otherwise. const changed = !(sameSelection(canonical, prevCanonical) && activeNext === active); - // Close BEFORE calling `onApply` (maintainer merge-gate finding, #189): - // `onApply` typically routes straight into `session.applyFilter`, which - // mutates state and `publish()`es SYNCHRONOUSLY before its first - // `await` — a caller subscribed to that publish (`dashboard.ts`'s - // `rebuildFilterBar`) can run inside this very call stack, before - // `applyBtn`'s own click handler ever returns. Closing first means that - // synchronous rebuild always observes this popover as already-closed - // (`isOpen()` false, `closeCurrent` cleared) — never mistakes an - // ordinary Apply's own commit for an outgoing bar's popover getting - // force-cancelled out from under the user, which is what used to - // trigger a false "Filter options were refreshed" announcement. `close()` - // (default, non-`skipFocus`) refocuses the trigger; the rebuild that - // `onApply` may synchronously trigger replaces the whole bar out from - // under that focus — restoring it onto the FRESH trigger is - // `rebuildFilterBar`'s own job (`dashboard.ts`), not this module's. - close(); + // Close BEFORE calling `onApply` (maintainer merge-gate finding, #189; + // now the shared `openAnchoredDialog` contract, #335): `onApply` + // typically routes straight into `session.applyFilter`, which mutates + // state and `publish()`es SYNCHRONOUSLY before its first `await` — a + // caller subscribed to that publish (`dashboard.ts`'s `rebuildFilterBar`) + // can run inside this very call stack, before `applyBtn`'s own click + // handler ever returns. Closing first means that synchronous rebuild + // always observes this popover as already-closed (`isOpen()` false, + // `closeCurrent` cleared) — never mistakes an ordinary Apply's own commit + // for an outgoing bar's popover getting force-cancelled out from under + // the user, which is what used to trigger a false "Filter options were + // refreshed" announcement. `handle.close()` (default, non-`skipFocus`) + // refocuses the trigger; the rebuild that `onApply` may synchronously + // trigger replaces the whole bar out from under that focus — restoring it + // onto the FRESH trigger is `rebuildFilterBar`'s own job (`dashboard.ts`), + // not this module's. + handle.close(); if (changed) opts.onApply(canonical, activeNext); }); const footer = h('div', { class: 'ms-footer' }, clearBtn, cancelBtn, applyBtn); - const dialog = h('div', { - class: 'ms-popover', role: 'dialog', 'aria-modal': 'true', 'aria-label': `${label} options`, - }, searchInput, liveEl, selectAllRow, listEl, footer); - const overlay = h('div', { class: 'ms-overlay' }); + // A `display:contents` wrapper: `openAnchoredDialog` appends ONE content + // element into the dialog, but `.ms-popover` is a flex column whose direct + // children (search/live/select-all/options/footer) carry the layout — the + // contents wrapper generates no box, so those children participate in the + // dialog's flex context exactly as they did when they were direct children. + const content = h('div', { style: { display: 'contents' } }, + searchInput, liveEl, selectAllRow, listEl, footer); // #189 F6: while OPEN, a status-only publish that goes // waiting/loading/idle/stale makes the checklist body noninteractive @@ -414,12 +425,13 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi // reports the visible/total count through; `ready` restores both the // controls and the normal count text. The draft itself is never // touched — its values can't change without a rebuild, which only - // happens closed. + // happens closed. This affordance stays in the multiselect (it operates + // on this module's own content) rather than in the shared primitive. let busy = false; function setBusy(next: boolean): void { if (busy === next) return; busy = next; - dialog.setAttribute('aria-busy', String(busy)); + handle.dialog.setAttribute('aria-busy', String(busy)); searchInput.disabled = busy; selectAllCb.disabled = busy; for (const row of rows) row.cb.disabled = busy; @@ -428,83 +440,33 @@ export function buildMultiSelectField(opts: MultiSelectFieldOpts): MultiSelectFi if (busy) liveEl.textContent = 'Loading options…'; else applyFilter(); // restores the normal "N of M options" live text } - openPopoverBusy = setBusy; - - const onKeyDown = (e: KeyboardEvent): void => { - if (e.key === 'Escape') { e.preventDefault(); close(); } - }; - - // #189 F3: a minimal focus trap — `aria-modal="true"` promises assistive - // tech (and sighted keyboard users) that Tab never leaves the dialog. The - // overlay only ever blocked POINTER events; without this, Tab/Shift-Tab - // walked straight out to whatever the document's next/previous tabbable - // happened to be. Recomputed on every Tab press (never cached) since the - // option checklist's visible subset changes with `searchText`. Registered - // on `dialog` itself (not `d`/document, unlike `onKeyDown`'s broad Escape - // catch above) — a stale, already-closed popover's own trap must never - // intercept a Tab dispatched at a DIFFERENT, currently-open dialog; a - // listener scoped to this specific (detached-on-close) node can't reach - // any OTHER dialog's subtree regardless of how many prior popovers a - // caller left open without disposing. - function focusableEls(): HTMLElement[] { - return [...dialog.querySelectorAll('input, button')] - .filter((el) => !el.closest('[hidden]') && !(el as HTMLInputElement | HTMLButtonElement).disabled); - } - const onTabTrap = (e: KeyboardEvent): void => { - if (e.key !== 'Tab') return; - // Cancel is never disabled (F6 keeps it usable even while `busy`), so - // `items` always has at least one entry — no empty-list guard needed. - const items = focusableEls(); - const first = items[0]; - const last = items[items.length - 1]; - const activeEl = d.activeElement as HTMLElement | null; - if (e.shiftKey) { - if (!activeEl || activeEl === first || !dialog.contains(activeEl)) { e.preventDefault(); last.focus(); } - } else if (!activeEl || activeEl === last || !dialog.contains(activeEl)) { - e.preventDefault(); first.focus(); - } - }; - // EVERY dismissal path (Apply, Cancel, Escape, outside-click, dispose) - // funnels through here — the one place that tears the popover down and - // returns focus to the trigger. Idempotent by construction (every step - // is a harmless no-op on an already-detached/already-null target), so no - // separate re-entrancy guard is needed even if a caller somehow reached - // it twice for the same open session. #189 F2a: `skipFocus` lets - // `applyStatus`'s forced error-close skip refocusing a trigger that's - // about to be detached from the DOM anyway (focus moves to the fallback - // input instead, over there). - function close(closeOpts: { skipFocus?: boolean } = {}): void { - d.removeEventListener('keydown', onKeyDown, true); - dialog.removeEventListener('keydown', onTabTrap, true); - detachBackdrop(); - overlay.remove(); - dialog.remove(); - trigger.setAttribute('aria-expanded', 'false'); - closeCurrent = null; - openPopoverBusy = null; - if (!closeOpts.skipFocus) trigger.focus(); - } - closeCurrent = close; - - trigger.setAttribute('aria-expanded', 'true'); - d.body.appendChild(overlay); - d.body.appendChild(dialog); - const detachBackdrop = attachBackdropClose(overlay, close); - d.addEventListener('keydown', onKeyDown, true); - dialog.addEventListener('keydown', onTabTrap, true); - - const rect = trigger.getBoundingClientRect(); - const pos = fixedAnchor(rect) as { top: number; left: number }; - overlay.style.position = 'fixed'; - overlay.style.inset = '0'; - dialog.style.position = 'fixed'; - dialog.style.top = pos.top + 'px'; - dialog.style.left = pos.left + 'px'; - dialog.style.minWidth = rect.width + 'px'; + // The generic dialog chrome (#335): overlay + backdrop-close, the ARIA + // dialog/aria-modal/aria-expanded lifecycle, document-capture Escape, the + // Tab focus trap (recomputed per press, dialog-scoped), placement under the + // trigger, and teardown + focus return. `minWidthFromTrigger: true` floors + // the popover width at the trigger's width; `clampToViewport` is left off + // to preserve the pre-#335 left-align-under-trigger behavior. `onClose` + // clears this module's open-state refs on every dismissal path — the same + // bookkeeping the old inline `close()` did (`isOpen()` reads `closeCurrent`). + const handle = openAnchoredDialog({ + document: d, + trigger, + ariaLabel: `${label} options`, + content, + dialogClassName: 'ms-popover', + overlayClassName: 'ms-overlay', + minWidthFromTrigger: true, + initialFocus: () => searchInput, // focus moves into the dialog on open + onClose: () => { closeCurrent = null; openPopoverBusy = null; }, + }); + // #189 F2a: `skipFocus` flows through to the primitive so `applyStatus`'s + // forced error-close can skip refocusing a trigger that's about to be + // detached (focus moves to the fallback input over there instead). + closeCurrent = (closeOpts) => handle.close(closeOpts); + openPopoverBusy = setBusy; applyFilter(); // seeds the live-region count and the select-visible tri-state - searchInput.focus(); // focus moves into the dialog on open } applyStatus(); diff --git a/src/ui/popover.ts b/src/ui/popover.ts new file mode 100644 index 00000000..b34fa3a3 --- /dev/null +++ b/src/ui/popover.ts @@ -0,0 +1,160 @@ +// #335: the generic anchored-dialog chrome, extracted from +// `multi-select-field.ts`'s `openPopover()` so a SECOND consumer (the +// time-range popover, a later wave) reuses it instead of copying it — the +// CLAUDE.md rule-5 "extract a shared primitive at the second consumer" +// precedent (`EditorPort`/`GraphSurface`/`Drawer`). It owns ONLY the generic +// modal-popover chrome; everything content-specific stays with the caller. +// +// OWNS: +// - a fresh overlay + panel mounted on open, torn down completely on close +// (menu.ts's lifecycle convention: never a hidden-but-resident node); +// - the `dialog` role + `aria-modal="true"` + `aria-label` accessible name; +// - a document-capture Escape that closes; +// - a dialog-scoped Tab trap whose focusable set is RECOMPUTED on every Tab +// press (never cached — the caller's content can hide/disable rows between +// presses) and scoped to THIS dialog node so a stale, already-closed +// popover's trap can never intercept a Tab meant for a different dialog; +// - `fixedAnchor` placement under the trigger (optional right-edge clamp and +// trigger-derived min-width); +// - the `aria-expanded` true/false lifecycle on the trigger; +// - teardown + focus return to the trigger, unless `skipFocus`. +// +// DOES NOT OWN: busy/loading state, live regions, any content semantics, the +// double-open guard (the caller keeps its own handle ref and decides whether +// to open a second one), or commit ordering. +// +// COMMIT ORDERING (the #364 / #189 merge-gate rule — consumers MUST follow it): +// call `close()` BEFORE invoking any commit callback (e.g. an Apply handler's +// `onApply`). A commit typically routes into state that `publish()`es +// synchronously; a subscriber that rebuilds the surrounding UI can run inside +// that very call stack, and it must observe this popover as ALREADY closed +// (`isOpen()` false) — never mistake an ordinary commit for a force-cancelled +// outgoing popover. See `multi-select-field.ts`'s Apply handler. + +import { h, fixedAnchor, attachBackdropClose } from './dom.js'; +import type { FixedAnchorOptions } from './dom.js'; + +/** `openAnchoredDialog`'s options bag. */ +export interface AnchoredDialogOptions { + /** Injected document realm — element creation, capture listeners, the + * focus-trap's `activeElement` read, and the mount point all target this. */ + document: Document; + /** Gets `aria-expanded` true/false and is the focus-return target on close. */ + trigger: HTMLElement; + /** The dialog's accessible name (`aria-label`). */ + ariaLabel: string; + /** Pre-built content, appended into the dialog. The primitive never inspects + * it beyond the Tab trap's `input, button` query. */ + content: HTMLElement; + /** The dialog element's class (e.g. `'ms-popover'` | `'trf-popover'`). */ + dialogClassName: string; + /** The overlay/backdrop element's class — defaults to `'ms-overlay'`. */ + overlayClassName?: string; + /** When true, the dialog's `min-width` is floored at the trigger's width. */ + minWidthFromTrigger?: boolean; + /** When true, the dialog's left inset is clamped so its measured width does + * not overflow the viewport's right edge (pure arithmetic in `fixedAnchor`; + * in a headless realm without a `defaultView` the viewport width reads 0). */ + clampToViewport?: boolean; + /** Returns the element to focus once the dialog is mounted, or null for + * none. Called with the dialog node after placement. */ + initialFocus?: (dialog: HTMLElement) => HTMLElement | null; + /** Fires after teardown + focus return, on EVERY dismissal path, exactly + * once (idempotent close never double-fires it). */ + onClose?: () => void; +} + +/** `openAnchoredDialog`'s return value. */ +export interface AnchoredDialogHandle { + /** The mounted dialog node — the caller reaches its own content through it + * (e.g. to toggle a busy affordance) without the primitive owning that. */ + dialog: HTMLElement; + /** Whether the dialog is still mounted (false once `close()` has run). */ + isOpen(): boolean; + /** Tears the dialog down and returns focus to the trigger unless + * `skipFocus`. Idempotent — every dismissal path funnels here, and a second + * call is a harmless no-op that never re-fires `onClose`. */ + close(opts?: { skipFocus?: boolean }): void; +} + +export function openAnchoredDialog(opts: AnchoredDialogOptions): AnchoredDialogHandle { + const d = opts.document; + const { trigger } = opts; + + const overlay = h('div', { class: opts.overlayClassName ?? 'ms-overlay' }); + const dialog = h('div', { + class: opts.dialogClassName, role: 'dialog', 'aria-modal': 'true', 'aria-label': opts.ariaLabel, + }, opts.content); + + let open = true; + + const onKeyDown = (e: KeyboardEvent): void => { + if (e.key === 'Escape') { e.preventDefault(); close(); } + }; + + // Recomputed on every Tab press (never cached): the caller's content can + // hide (search filter) or disable (busy) rows between presses. Scoped to + // THIS dialog so a leaked, already-closed popover's trap can't reach another + // open dialog's subtree. + function focusableEls(): HTMLElement[] { + return [...dialog.querySelectorAll('input, button')] + .filter((el) => !el.closest('[hidden]') && !(el as HTMLInputElement | HTMLButtonElement).disabled); + } + const onTabTrap = (e: KeyboardEvent): void => { + if (e.key !== 'Tab') return; + const items = focusableEls(); + if (items.length === 0) return; // nothing to trap — let the browser handle it + const first = items[0]; + const last = items[items.length - 1]; + const activeEl = d.activeElement as HTMLElement | null; + if (e.shiftKey) { + if (!activeEl || activeEl === first || !dialog.contains(activeEl)) { e.preventDefault(); last.focus(); } + } else if (!activeEl || activeEl === last || !dialog.contains(activeEl)) { + e.preventDefault(); first.focus(); + } + }; + + // The single teardown funnel. Idempotent: the `open` guard means teardown + + // `onClose` run exactly once no matter how many dismissal paths reach it. + function close(closeOpts: { skipFocus?: boolean } = {}): void { + if (!open) return; + open = false; + d.removeEventListener('keydown', onKeyDown, true); + dialog.removeEventListener('keydown', onTabTrap, true); + detachBackdrop(); + overlay.remove(); + dialog.remove(); + trigger.setAttribute('aria-expanded', 'false'); + if (!closeOpts.skipFocus) trigger.focus(); + opts.onClose?.(); + } + + trigger.setAttribute('aria-expanded', 'true'); + d.body.appendChild(overlay); + d.body.appendChild(dialog); + const detachBackdrop = attachBackdropClose(overlay, () => close()); + d.addEventListener('keydown', onKeyDown, true); + dialog.addEventListener('keydown', onTabTrap, true); + + const rect = trigger.getBoundingClientRect(); + overlay.style.position = 'fixed'; + overlay.style.inset = '0'; + dialog.style.position = 'fixed'; + const anchorOpts: FixedAnchorOptions = {}; + if (opts.clampToViewport) { + const view = d.defaultView; + anchorOpts.viewportW = view ? view.innerWidth : 0; + anchorOpts.panelW = dialog.getBoundingClientRect().width; + } + // Both the plain and the clamped paths left-align (the clamp only lowers the + // left inset), so the result is always `{ top, left }`. + const pos = fixedAnchor(rect, anchorOpts) as { top: number; left: number }; + dialog.style.top = pos.top + 'px'; + dialog.style.left = pos.left + 'px'; + if (opts.minWidthFromTrigger) dialog.style.minWidth = rect.width + 'px'; + + const focusTarget = opts.initialFocus?.(dialog); + if (focusTarget) focusTarget.focus(); + + return { dialog, isOpen: () => open, close }; +} diff --git a/src/ui/relative-time-field.ts b/src/ui/relative-time-field.ts index 17977e84..ccd1704b 100644 --- a/src/ui/relative-time-field.ts +++ b/src/ui/relative-time-field.ts @@ -52,7 +52,9 @@ export interface RelativeTimePreset { label: string; } -/** v1 preset list (#169 spec) — plain combobox option data. */ +/** v1 preset list (#169 spec) — plain combobox option data. Kept EXACTLY as + * it was before #335 — bit-identical entries/order/labels — this field's + * own preset dropdown is untouched by the time-range control's arrival. */ export const RELATIVE_TIME_PRESETS: RelativeTimePreset[] = [ { value: '-15m', label: '-15m — last 15 minutes' }, { value: '-1h', label: '-1h — last hour' }, @@ -65,14 +67,56 @@ export const RELATIVE_TIME_PRESETS: RelativeTimePreset[] = [ { value: 'now', label: 'now — this instant' }, ]; +/** + * #335 time-range popover's per-field "constants" column — one plain- + * language relative-time token per row, distinct from (and a superset in + * spirit of, though not literally overlapping with) `RELATIVE_TIME_PRESETS` + * above, which stays exactly as it was. Order here is the pinned design + * order: `now` first, then ascending offsets grouped by unit (minutes, + * hours, days, the one month entry, then the 90-day outlier the design + * places last). Consumed by the time-range field UI (a later wave); this + * module only owns the data + the shared filter helper below. + */ +export const TIME_RANGE_CONSTANTS: ReadonlyArray<{ value: string; label: string }> = [ + { value: 'now', label: 'now — current time' }, + { value: '-5m', label: '-5m — 5 minutes ago' }, + { value: '-15m', label: '-15m — 15 minutes ago' }, + { value: '-30m', label: '-30m — 30 minutes ago' }, + { value: '-1h', label: '-1h — 1 hour ago' }, + { value: '-3h', label: '-3h — 3 hours ago' }, + { value: '-6h', label: '-6h — 6 hours ago' }, + { value: '-12h', label: '-12h — 12 hours ago' }, + { value: '-1d', label: '-1d — 1 day ago' }, + { value: '-2d', label: '-2d — 2 days ago' }, + { value: '-7d', label: '-7d — 7 days ago' }, + { value: '-30d', label: '-30d — 30 days ago' }, + { value: '-1M', label: '-1M — 1 month ago' }, + { value: '-90d', label: '-90d — 90 days ago' }, +]; + +/** + * Type-to-filter (#174 §1, generalized for #335): a blank query returns + * `list` itself (never a copy — callers/tests rely on referential identity + * when nothing was typed); otherwise a case-insensitive substring match + * against either a row's `value` or its `label`. Pure, and usable over any + * `{value, label}`-shaped list — `filterPresets` below is now a one-line + * wrapper over `RELATIVE_TIME_PRESETS`, behavior unchanged. + */ +export function filterTokenList( + list: readonly T[], + text: string | undefined, +): readonly T[] { + const q = String(text || '').trim().toLowerCase(); + if (!q) return list; + return list.filter((item) => item.value.toLowerCase().includes(q) || item.label.toLowerCase().includes(q)); +} + /** Type-to-filter (#174 §1): a blank query shows every preset; otherwise a * case-insensitive substring match against either the expression or its * label — matching "1" surfaces every preset built from a `1`, matching * "day" surfaces both `-1d` (via its label) and `-1d/d`. Pure. */ export function filterPresets(text: string | undefined): RelativeTimePreset[] { - const q = String(text || '').trim().toLowerCase(); - if (!q) return RELATIVE_TIME_PRESETS; - return RELATIVE_TIME_PRESETS.filter((p) => p.value.toLowerCase().includes(q) || p.label.toLowerCase().includes(q)); + return filterTokenList(RELATIVE_TIME_PRESETS, text) as RelativeTimePreset[]; } /** `buildRelativeTimeField`'s options bag. */ diff --git a/src/ui/time-range-field.ts b/src/ui/time-range-field.ts new file mode 100644 index 00000000..09eb8c05 --- /dev/null +++ b/src/ui/time-range-field.ts @@ -0,0 +1,360 @@ +// #335: the compound Dashboard time-range control — a closed trigger showing +// the wave-resolved absolute range, and a two-column popover with token-based +// From/To editors (relative expressions or absolute datetimes), a live +// resolved preview per bound, a contextual per-field constants column, a +// group-scoped "Recently used" list, and an explicit Apply that commits both +// bounds atomically. It is the SECOND consumer of the #364 dialog-popover +// pattern, so it borrows the same primitives multi-select-field.ts does rather +// than reinventing them: +// - `popover.ts`'s `openAnchoredDialog` (#335) owns ALL the generic dialog +// chrome (overlay/backdrop, the ARIA dialog/aria-modal/aria-expanded +// lifecycle, Escape, the Tab focus trap, placement + viewport clamp, and +// focus return). This module keeps only the time-range-specific content, +// the staged From/To draft, and the Apply/close ordering on top of it. +// - `core/time-range.ts`'s `validateTimeRangeDraft` owns ALL parsing/ +// resolution — the control NEVER reimplements the relative/absolute +// grammar. Every trigger label and every preview line comes from one +// `validateTimeRangeDraft` call against one shared `nowMs` (the issue's +// "single preview now" rule: both bounds resolve `now` against the same +// instant within a validation pass). +// - `relative-time-field.ts`'s `TIME_RANGE_CONSTANTS` + `filterTokenList` +// own the per-field constants data and its type-to-filter behavior. +// +// State ownership mirrors multi-select-field.ts: the COMMITTED +// `fromValue`/`toValue`/`active` are frozen at construction — a caller wanting +// a later committed-value change reflected calls `buildTimeRangeField` again +// (the same convention `buildFilterBar` uses). `refreshLabel(nowMs)` +// re-resolves ONLY the closed trigger's label/aria in place (per execution +// wave, no timers, no rebuild). The OPEN popover owns its own staged draft (the +// two inputs' text) plus its right-column state, all local to `openPopover()`, +// none of which survives the matching `close()`. +// +// COMMIT ORDERING (the #364/#189 merge-gate rule, now `openAnchoredDialog`'s +// documented contract): `close()` runs BEFORE any commit callback (`onApply`, +// whether from the Apply button or a recents pick), so a synchronous rebuild +// reacting to that commit always observes this popover as already closed. +// +// FOCUS/RESTING-STATE ADAPTATION vs multi-select-field.ts: the multiselect +// popover focuses its search input on open. This one focuses the From input +// on open (a11y: focus enters the modal) but its RIGHT COLUMN opens on +// "Recently used" — the pinned design's resting state — because the right +// column is contextual on a field being *active*, and the single programmatic +// open-focus is deliberately NOT treated as a user activation (`openingFocus` +// guard). A genuine later focus, typing, or a caret toggle activates a field +// and swaps the right column to that field's constants. + +import { h } from './dom.js'; +import { idSafe } from './combobox.js'; +import { openAnchoredDialog } from './popover.js'; +import { validateTimeRangeDraft } from '../core/time-range.js'; +import type { DashboardTimeRangeGroup, TimeRangeRecent, TimeRangeBoundDraft } from '../core/time-range.js'; +import { TIME_RANGE_CONSTANTS, filterTokenList } from './relative-time-field.js'; + +/** `buildTimeRangeField`'s options bag. */ +export interface TimeRangeFieldOpts { + /** Injected document realm — defaults to the ambient global. */ + document?: Document; + /** The resolved group whose From/To parameter types drive validation. */ + group: DashboardTimeRangeGroup; + /** Committed raw text of each bound ('' when unset). */ + fromValue: string; + toValue: string; + /** True only when BOTH bounds are committed + active. */ + active: boolean; + /** The last execution wave's shared `now` snapshot; null before the first + * wave (the constructor then falls back to `wallNow()` for the label). */ + waveNowMs: number | null; + /** Live wall clock for the popover's preview `now` (captured once at open, + * then once per input event — never mid-pass). */ + wallNow: () => number; + /** Group-scoped recents, shell-owned; read live each time the right column + * renders (a pick or a commit elsewhere can change it between opens). */ + getRecents: () => readonly TimeRangeRecent[]; + /** Both the Apply button and a recents pick route here, with TRIMMED text. */ + onApply: (from: string, to: string) => void; +} + +/** `buildTimeRangeField`'s return value. */ +export interface TimeRangeFieldHandle { + /** The control's root, a `.var-field.is-time-range` wrapper hosting the + * trigger — dropped straight into the bar's "Time" section by the caller. */ + el: HTMLElement; + /** Present for handle uniformity with the other filter controls. A plain + * (non-source-backed) time-range control has no transport status to show, + * so this is a documented no-op. */ + updateStatus(s: unknown): void; + /** Whether the popover is currently open. */ + isOpen(): boolean; + /** Focus this control's trigger (used by the bar's rebuild focus-restore). */ + focusTrigger(): void; + /** Close the popover if open (a Cancel: no `onApply`) and detach listeners. */ + dispose(): void; + /** Re-resolve ONLY the closed trigger's label + accessible name against a + * new `nowMs` — no popover state, no rebuild. */ + refreshLabel(nowMs: number): void; +} + +export function buildTimeRangeField(opts: TimeRangeFieldOpts): TimeRangeFieldHandle { + const d = opts.document || document; + const { group, fromValue, toValue, active } = opts; + const { fromType, toType } = group; + const suffix = idSafe(group.key); + + // The currently-open popover's own close() — non-null iff open (isOpen() + // reads this directly). `skipFocus` rides through to the primitive for + // parity with multi-select-field.ts (no forced-error close exists here, so + // it is always the default trigger-refocus in practice). + let closeCurrent: ((closeOpts?: { skipFocus?: boolean }) => void) | null = null; + + const trigger = h('button', { + type: 'button', id: 'trf-trigger-' + suffix, class: 'trf-trigger var-input', + 'aria-haspopup': 'dialog', 'aria-expanded': 'false', + }); + + // Re-resolve the closed trigger's label + aria against `nowMs`. Pure read of + // the frozen committed values; never touches the popover. + function computeTriggerLabel(nowMs: number): void { + if (!active) { + trigger.textContent = 'Not set'; + trigger.classList.remove('is-error'); + trigger.setAttribute('aria-label', 'Time range, not set'); + trigger.title = 'Time range, not set'; + return; + } + const res = validateTimeRangeDraft({ fromText: fromValue, toText: toValue, fromType, toType, nowMs }); + const fromDisp = res.from.ok ? res.from.display! : fromValue; + const toDisp = res.to.ok ? res.to.display! : toValue; + const text = `${fromDisp} → ${toDisp}`; + const hasError = !res.from.ok || !res.to.ok; + trigger.textContent = text; + trigger.title = text; + trigger.classList.toggle('is-error', hasError); + trigger.setAttribute('aria-label', hasError + ? `Time range from ${fromValue} to ${toValue}, not resolvable` + : `Time range from ${fromValue} to ${toValue}, resolved ${res.from.display} to ${res.to.display}`); + } + + const onTriggerClick = (): void => openPopover(); + trigger.addEventListener('click', onTriggerClick); + + // Mount a fresh popover. The generic dialog chrome lives in + // `openAnchoredDialog`; this builds the staged From/To editors, the + // contextual right column, and the Apply/close ordering on top of it. + function openPopover(): void { + if (closeCurrent) return; // already open — never stack a second popover + + // The single shared preview `now`: captured once at open, refreshed once + // per input event, and fed to ONE `validateTimeRangeDraft` call so `now` + // in From and `now` in To always agree within a pass. + let previewNow = opts.wallNow(); + // Which field's constants the right column shows, or null → recents. + let activeField: 'from' | 'to' | null = null; + // The constants type-to-filter text for the active field (typing sets it; + // a constant fill or an activation change resets it). + let constFilter = ''; + // The single programmatic open-focus below must NOT count as a user + // activation (so the right column opens on recents); a genuine later focus + // does. Flipped false right after the dialog mounts. + let openingFocus = true; + + const mkInput = (role: 'from' | 'to', seed: string): HTMLInputElement => h('input', { + type: 'text', class: 'trf-input var-input', value: seed, + id: `trf-${role}-${suffix}`, 'aria-label': role === 'from' ? 'From' : 'To', + 'aria-describedby': `trf-${role}-preview-${suffix}`, + }); + const fromInput = mkInput('from', fromValue); + const toInput = mkInput('to', toValue); + const fromPreview = h('div', { class: 'trf-preview', id: `trf-from-preview-${suffix}` }); + const toPreview = h('div', { class: 'trf-preview', id: `trf-to-preview-${suffix}` }); + const fromCaret = h('button', { + type: 'button', class: 'trf-caret', 'aria-pressed': 'false', 'aria-label': 'Show constants for From', + }, '▾'); + const toCaret = h('button', { + type: 'button', class: 'trf-caret', 'aria-pressed': 'false', 'aria-label': 'Show constants for To', + }, '▾'); + + const mkRow = (label: string, input: HTMLInputElement, caret: HTMLElement, preview: HTMLElement): HTMLElement => + h('div', { class: 'trf-row' }, + h('label', { class: 'trf-field-label', for: input.id }, label), + h('div', { class: 'trf-input-wrap' }, input, caret), + preview); + const left = h('div', { class: 'trf-left' }, + mkRow('From', fromInput, fromCaret, fromPreview), + mkRow('To', toInput, toCaret, toPreview)); + + const rightHeader = h('div', { class: 'trf-right-header' }); + const rightBody = h('div', { class: 'trf-right-body' }); + const right = h('div', { class: 'trf-right' }, rightHeader, rightBody); + const cols = h('div', { class: 'trf-cols' }, left, right); + + const rangeErrEl = h('div', { class: 'trf-range-error', hidden: true }); + // Polite failure announcer: `aria-describedby` reads the preview lines on + // focus but does not re-announce as they change mid-typing — validation + // FAILURES must be announced (issue #335 accessibility), successes must + // not turn every keystroke into speech. + const liveEl = h('div', { class: 'sr-only', 'aria-live': 'polite' }); + const cancelBtn = h('button', { type: 'button', class: 'trf-btn' }, 'Cancel'); + const applyBtn = h('button', { type: 'button', class: 'trf-btn trf-btn-primary' }, 'Apply'); + const footer = h('div', { class: 'trf-footer' }, cancelBtn, applyBtn); + + const paintPreview = (el: HTMLElement, bd: TimeRangeBoundDraft): void => { + if (bd.ok) { + el.textContent = '= ' + bd.display; + el.classList.remove('is-error'); + } else { + el.textContent = bd.error; + el.classList.add('is-error'); + } + }; + + // ONE validation pass drives both preview lines, the range error, and the + // Apply gate — never two separate `now` readings. + function revalidate(): void { + const res = validateTimeRangeDraft({ + fromText: fromInput.value, toText: toInput.value, fromType, toType, nowMs: previewNow, + }); + paintPreview(fromPreview, res.from); + paintPreview(toPreview, res.to); + if (res.rangeError) { + rangeErrEl.textContent = res.rangeError; + rangeErrEl.hidden = false; + } else { + rangeErrEl.textContent = ''; + rangeErrEl.hidden = true; + } + applyBtn.disabled = !res.applyEnabled; + const failure = !res.from.ok ? `From: ${res.from.error}` + : !res.to.ok ? `To: ${res.to.error}` + : res.rangeError ?? ''; + // Only write on change — re-setting identical text would re-announce it + // on every keystroke. + if (liveEl.textContent !== failure) liveEl.textContent = failure; + } + + // Render the contextual right column: recents (no field active) or the + // active field's filtered constants. + function renderRight(): void { + fromCaret.setAttribute('aria-pressed', String(activeField === 'from')); + toCaret.setAttribute('aria-pressed', String(activeField === 'to')); + if (activeField === null) { + rightHeader.textContent = 'Recently used'; + const recents = opts.getRecents(); + if (recents.length === 0) { + rightBody.replaceChildren(h('div', { class: 'trf-empty' }, 'No recent ranges yet')); + return; + } + rightBody.replaceChildren(...recents.map((r) => { + const b = h('button', { type: 'button', class: 'trf-recent' }, `${r.from} → ${r.to}`); + // A recents pick is an immediate apply: close FIRST (commit ordering), + // then route through the same onApply the Apply button uses. + b.addEventListener('click', () => { handle.close(); opts.onApply(r.from, r.to); }); + return b; + })); + return; + } + const input = activeField === 'from' ? fromInput : toInput; + rightHeader.textContent = activeField === 'from' ? 'From · constants' : 'To · constants'; + const matches = filterTokenList(TIME_RANGE_CONSTANTS, constFilter); + if (matches.length === 0) { + rightBody.replaceChildren( + h('div', { class: 'trf-empty' }, 'No match — absolute datetimes like 2026-07-21 09:00 are accepted')); + return; + } + rightBody.replaceChildren(...matches.map((c) => { + const b = h('button', { type: 'button', class: 'trf-const' }, + h('span', { class: 'trf-const-token' }, c.value), + h('span', { class: 'trf-const-label' }, c.label)); + // A constant fill is STAGED: fills the field's input, resets the filter, + // re-validates, keeps the popover open. No apply. + b.addEventListener('click', () => { + input.value = c.value; + constFilter = ''; + revalidate(); + renderRight(); + // renderRight just detached the clicked button — without this the + // focus falls to ; the field input is where editing continues. + input.focus(); + }); + return b; + })); + } + + const onFieldInput = (field: 'from' | 'to'): void => { + previewNow = opts.wallNow(); + activeField = field; + constFilter = (field === 'from' ? fromInput : toInput).value; + revalidate(); + renderRight(); + }; + const onFieldFocus = (field: 'from' | 'to'): void => { + if (openingFocus) return; // the single programmatic open-focus is not a user activation + activeField = field; + constFilter = ''; + renderRight(); + }; + const onCaret = (field: 'from' | 'to'): void => { + activeField = activeField === field ? null : field; + constFilter = ''; + renderRight(); + }; + fromInput.addEventListener('input', () => onFieldInput('from')); + toInput.addEventListener('input', () => onFieldInput('to')); + fromInput.addEventListener('focus', () => onFieldFocus('from')); + toInput.addEventListener('focus', () => onFieldFocus('to')); + fromCaret.addEventListener('click', () => onCaret('from')); + toCaret.addEventListener('click', () => onCaret('to')); + + cancelBtn.addEventListener('click', () => handle.close()); + // Apply is only reachable enabled (a real browser never fires click on a + // disabled button; the disabled attr is the gate — see the multiselect + // precedent, whose Apply handler is likewise unguarded). + applyBtn.addEventListener('click', () => { + const fromT = fromInput.value.trim(); + const toT = toInput.value.trim(); + // An identical (trimmed) draft is a no-op ONLY while the pair is already + // active: with committed-but-inactive bounds (clearFilter keeps the typed + // value and just flips `active` off) an unchanged draft still activates + // the range, so it must commit — the session counts the active flip as a + // real change and runs the wave. + const identical = opts.active && fromT === fromValue.trim() && toT === toValue.trim(); + handle.close(); + if (!identical) opts.onApply(fromT, toT); + }); + + // display:contents wrapper — `openAnchoredDialog` appends ONE content + // element, but `.trf-popover` is the flex column whose direct children + // (cols/range-error/footer) carry the layout. + const content = h('div', { style: { display: 'contents' } }, cols, rangeErrEl, liveEl, footer); + + const handle = openAnchoredDialog({ + document: d, + trigger, + ariaLabel: 'Time range', + content, + dialogClassName: 'trf-popover', + clampToViewport: true, + minWidthFromTrigger: false, + initialFocus: () => fromInput, + onClose: () => { closeCurrent = null; }, + }); + closeCurrent = (closeOpts) => handle.close(closeOpts); + openingFocus = false; + + revalidate(); // seed both preview lines + the Apply gate from the committed seed + renderRight(); // resting state: recents + } + + computeTriggerLabel(opts.waveNowMs ?? opts.wallNow()); + + return { + el: h('div', { class: 'var-field is-time-range' }, trigger), + updateStatus: () => { /* no-op: a plain time-range control has no source status */ }, + isOpen: () => closeCurrent !== null, + focusTrigger: () => { trigger.focus(); }, + refreshLabel: (nowMs) => computeTriggerLabel(nowMs), + dispose: () => { + closeCurrent?.(); // dispose-while-open is a Cancel: no writes + trigger.removeEventListener('click', onTriggerClick); + }, + }; +} diff --git a/tests/e2e/dashboard-mobile.html b/tests/e2e/dashboard-mobile.html index 6447bd39..fdc7623b 100644 --- a/tests/e2e/dashboard-mobile.html +++ b/tests/e2e/dashboard-mobile.html @@ -52,6 +52,8 @@ + + + + +
+ + + diff --git a/tests/e2e/time-range.spec.js b/tests/e2e/time-range.spec.js new file mode 100644 index 00000000..ba26e244 --- /dev/null +++ b/tests/e2e/time-range.spec.js @@ -0,0 +1,208 @@ +import { test, expect } from '@playwright/test'; + +// #335: the compound Dashboard time-range control, driven in a REAL browser +// against the actual DashboardViewerSession + buildFilterBar + buildTimeRangeField +// + openAnchoredDialog + validateTimeRangeDraft pipeline (see time-range.html). +// happy-dom cannot exercise the popover placement/viewport clamp, the real +// focus trap / focus return, or the `.fill()`-driven input revalidation, so +// these live here. + +const open = async (page) => { + await page.locator('.trf-trigger').click(); + await expect(page.locator('.trf-popover')).toBeVisible(); +}; +const fromBox = (page) => page.getByRole('textbox', { name: 'From' }); +const toBox = (page) => page.getByRole('textbox', { name: 'To' }); +const applyBtn = (page) => page.locator('.trf-btn-primary'); + +test.describe('Dashboard compound time-range control', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/tests/e2e/time-range.html'); + await page.waitForFunction(() => window.__ready === true); + }); + + test('renders one resolved-range trigger, suppresses the pair’s own fields, keeps the non-group field', async ({ page }) => { + // The from/to pair resolved into exactly one date-like group. + expect(await page.evaluate(() => window.__groups())) + .toEqual([{ key: 'f-from\u0000f-to', from: 'from', to: 'to' }]); + + const trigger = page.locator('.trf-trigger'); + await expect(trigger).toBeVisible(); + // The closed trigger shows the wave-resolved absolute range and names it. + await expect(trigger).toContainText(' → '); + await expect(trigger).toContainText('2026-07-21 12:00:00'); + await expect(trigger).toContainText('2026-07-22 12:00:00'); + await expect(trigger).toHaveAttribute('aria-label', /from -1d to now, resolved/); + await expect(trigger).toHaveAttribute('aria-haspopup', 'dialog'); + + // The "Time" section label sits ahead of the per-param fields. + await expect(page.locator('.dash-filters .flabel', { hasText: 'Time' })).toBeVisible(); + + // The pair's own individual fields are gone (the compound control + // represents them); the non-group `service` filter keeps its field. + await expect(page.getByRole('combobox', { name: 'from' })).toHaveCount(0); + await expect(page.getByRole('combobox', { name: 'to' })).toHaveCount(0); + await expect(page.getByRole('combobox', { name: 'service' })).toHaveCount(1); + }); + + test('opens with both bounds seeded, resolved previews, and Recently used as the resting state', async ({ page }) => { + await open(page); + const dialog = page.getByRole('dialog', { name: 'Time range' }); + await expect(dialog).toBeVisible(); + + await expect(fromBox(page)).toHaveValue('-1d'); + await expect(toBox(page)).toHaveValue('now'); + + // Both preview lines resolved against the ONE shared wave `now`. + const previews = page.locator('.trf-preview'); + await expect(previews.nth(0)).toHaveText('= 2026-07-21 12:00:00'); + await expect(previews.nth(1)).toHaveText('= 2026-07-22 12:00:00'); + + // The right column's resting state (the single programmatic open-focus is + // NOT a user activation): Recently used, empty on first open. + await expect(page.locator('.trf-right-header')).toHaveText('Recently used'); + await expect(page.locator('.trf-empty')).toHaveText('No recent ranges yet'); + await expect(page.locator('.trf-const')).toHaveCount(0); + }); + + test('shows a field’s constants on focus; staging a constant fills the input without committing', async ({ page }) => { + await open(page); + // A genuine focus (not the open-focus) activates a field → its constants. + await toBox(page).focus(); + await expect(page.locator('.trf-right-header')).toHaveText('To · constants'); + await expect(page.locator('.trf-const').first()).toBeVisible(); + + await page.evaluate(() => window.__resetExec()); + await page.locator('.trf-const', { hasText: '-6h' }).click(); + // Staged: the input holds the token, the popover stays open, nothing ran. + await expect(toBox(page)).toHaveValue('-6h'); + await expect(page.locator('.trf-popover')).toBeVisible(); + expect(await page.evaluate(() => window.__execLog.length)).toBe(0); + }); + + test('disables Apply on unparseable input and on an inverted range', async ({ page }) => { + await open(page); + await fromBox(page).fill('garbage'); + await expect(applyBtn(page)).toBeDisabled(); + await expect(page.locator('.trf-preview.is-error').first()).toBeVisible(); + + // now (12:00) > -1d (yesterday) → an inverted range: a range error, Apply gated. + await fromBox(page).fill('now'); + await toBox(page).fill('-1d'); + const rangeErr = page.locator('.trf-range-error'); + await expect(rangeErr).toBeVisible(); + await expect(rangeErr).toContainText('must not be after'); + await expect(applyBtn(page)).toBeDisabled(); + }); + + test('a valid Apply closes the popover, commits both bounds in exactly one wave, and updates the trigger', async ({ page }) => { + await open(page); + await fromBox(page).fill('-7d'); + await toBox(page).fill('now'); + await expect(applyBtn(page)).toBeEnabled(); + + await page.evaluate(() => window.__resetExec()); + await applyBtn(page).click(); + await page.evaluate(() => window.__lastApply); + + // Closed first, then committed — the popover is gone by the time the + // commit-driven rebuild ran. + await expect(page.locator('.trf-popover')).toHaveCount(0); + + // Exactly ONE wave: each dependent tile (a + b consume from/to) ran once; + // the non-dependent service tile (c) did not run. + const log = await page.evaluate(() => window.__execLog); + expect(log.filter((s) => s.includes('tile-a')).length).toBe(1); + expect(log.filter((s) => s.includes('tile-b')).length).toBe(1); + expect(log.filter((s) => s.includes('tile-c')).length).toBe(0); + expect(log.length).toBe(2); + + // The trigger re-resolved its label + aria against the committed pair. + await expect(page.locator('.trf-trigger')).toHaveAttribute('aria-label', /from -7d to now, resolved/); + expect(await page.evaluate(() => window.__live())).toBe('Time range applied: -7d → now'); + }); + + test('records the outgoing range in Recently used across changes; picking a recent applies it immediately', async ({ page }) => { + // Change #1: -1d/now → -7d/now records the outgoing -1d/now. + await open(page); + await fromBox(page).fill('-7d'); + await toBox(page).fill('now'); + await applyBtn(page).click(); + await page.evaluate(() => window.__lastApply); + + // Change #2: -7d/now → -30d/now records the outgoing -7d/now (newest first). + await open(page); + await fromBox(page).fill('-30d'); + await toBox(page).fill('now'); + await applyBtn(page).click(); + await page.evaluate(() => window.__lastApply); + + expect(await page.evaluate(() => window.__recents().map((r) => [r.from, r.to]))) + .toEqual([['-7d', 'now'], ['-1d', 'now']]); + + // Reopen — Recently used now lists both, newest first. + await open(page); + const recents = page.locator('.trf-recent'); + await expect(recents).toHaveCount(2); + await expect(recents.nth(0)).toHaveText('-7d → now'); + await expect(recents.nth(1)).toHaveText('-1d → now'); + + // Picking a recent is an immediate apply: closes first, one wave, trigger updates. + await page.evaluate(() => window.__resetExec()); + await recents.nth(0).click(); + await page.evaluate(() => window.__lastApply); + await expect(page.locator('.trf-popover')).toHaveCount(0); + const log = await page.evaluate(() => window.__execLog); + expect(log.filter((s) => s.includes('tile-a')).length).toBe(1); + expect(log.filter((s) => s.includes('tile-b')).length).toBe(1); + expect(log.length).toBe(2); + await expect(page.locator('.trf-trigger')).toHaveAttribute('aria-label', /from -7d to now, resolved/); + }); + + test('Escape closes and returns focus to the trigger; a backdrop click closes', async ({ page }) => { + await open(page); + await page.keyboard.press('Escape'); + await expect(page.locator('.trf-popover')).toHaveCount(0); + // Focus returned to the control's trigger, not stranded on . + expect(await page.evaluate(() => document.activeElement?.classList.contains('trf-trigger'))).toBe(true); + + // Backdrop click (a genuine mousedown+click whose target is the overlay). + await open(page); + await page.locator('.ms-overlay').click({ position: { x: 4, y: 4 } }); + await expect(page.locator('.trf-popover')).toHaveCount(0); + }); + + test('renders the control and popover in both light and dark themes', async ({ page }) => { + const bg = async () => page.locator('.trf-popover').evaluate((n) => getComputedStyle(n).backgroundColor); + + await page.evaluate(() => window.__setTheme('dark')); + await open(page); + await expect(page.getByRole('dialog', { name: 'Time range' })).toBeVisible(); + const darkBg = await bg(); + await page.keyboard.press('Escape'); + + await page.evaluate(() => window.__setTheme('light')); + await open(page); + await expect(page.getByRole('dialog', { name: 'Time range' })).toBeVisible(); + const lightBg = await bg(); + + // The theme tokens actually apply to the popover chrome in both directions. + expect(darkBg).not.toBe(lightBg); + await expect(page.locator('.trf-trigger')).toBeVisible(); + }); + + test('keeps the popover inside the viewport at a 360px width', async ({ page }) => { + await page.setViewportSize({ width: 360, height: 800 }); + await page.reload(); + await page.waitForFunction(() => window.__ready === true); + + await open(page); + const geom = await page.locator('.trf-popover').evaluate((n) => { + const r = n.getBoundingClientRect(); + return { left: r.left, right: r.right, innerWidth, pageOverflow: document.documentElement.scrollWidth - innerWidth }; + }); + expect(geom.left).toBeGreaterThanOrEqual(0); + expect(geom.right).toBeLessThanOrEqual(geom.innerWidth + 1); + expect(geom.pageOverflow).toBeLessThanOrEqual(0); + }); +}); diff --git a/tests/unit/dashboard-viewer-session.test.ts b/tests/unit/dashboard-viewer-session.test.ts index df283f60..0c24e568 100644 --- a/tests/unit/dashboard-viewer-session.test.ts +++ b/tests/unit/dashboard-viewer-session.test.ts @@ -2527,3 +2527,338 @@ describe('searchable multiselect filter contract (#189)', () => { expect(session.state.value.tiles.find((t) => t.tileId === 't2')!.status).toBe('unfilled'); }); }); + +// #335: the public batch-commit surface. `applyFilters` commits several filters +// atomically (the time-range control's From/To pair, #334 drag-to-select) in +// ONE execution wave over the union of every changed parameter's targets. Any +// unknown OR duplicate id aborts the whole call before any mutation; an +// all-identical call is a true no-op (no publish, no wave). +describe('applyFilters batch commit (#335)', () => { + const bothDoc = () => doc({ + tiles: [tile('ta', 'qa'), tile('tb', 'qb'), tile('tboth', 'qboth')], + filters: [ + { id: 'f1', parameter: 'p', defaultActive: false, defaultValue: '' }, + { id: 'f2', parameter: 'q', defaultActive: false, defaultValue: '' }, + ], + }); + const bothQueries = () => [ + query('qa', 'SELECT {p:String} AS n'), + query('qb', 'SELECT {q:String} AS n'), + query('qboth', 'SELECT {p:String} AS a, {q:String} AS b'), + ]; + + it('is atomic: an unknown id among the entries mutates nothing and runs no wave', async () => { + const { exec, calls } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ document: bothDoc(), exec, queries: bothQueries() })); + await session.start(); + const base = calls.length; + const snapshot = session.state.value; + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, + { filterId: 'nope', value: 'y', active: true }, + ]); + expect(calls.length).toBe(base); // no wave + expect(session.state.value).toBe(snapshot); // no publish + expect(session.state.value.filters[0]).toMatchObject({ value: '', active: false }); + expect(session.state.value.filters[1]).toMatchObject({ value: '', active: false }); + }); + + it('is atomic: a duplicate id in the call mutates nothing and runs no wave', async () => { + const { exec, calls } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ document: bothDoc(), exec, queries: bothQueries() })); + await session.start(); + const base = calls.length; + const snapshot = session.state.value; + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, + { filterId: 'f1', value: 'z', active: true }, + ]); + expect(calls.length).toBe(base); + expect(session.state.value).toBe(snapshot); + expect(session.state.value.filters[0]).toMatchObject({ value: '', active: false }); + }); + + it('commits both filters active and reruns the UNION of their targets in exactly one wave (a tile consuming both runs once)', async () => { + const { exec, calls } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ document: bothDoc(), exec, queries: bothQueries() })); + await session.start(); + const base = calls.length; + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, + { filterId: 'f2', value: 'y', active: true }, + ]); + // Both bounds committed + active. + expect(session.state.value.filters[0]).toMatchObject({ value: 'x', active: true }); + expect(session.state.value.filters[1]).toMatchObject({ value: 'y', active: true }); + const added = calls.slice(base); + // Union of p's targets {ta, tboth} and q's targets {tb, tboth} = 3 tiles, + // each run EXACTLY once — the tile consuming BOTH params never runs twice. + expect(added.length).toBe(3); + const bothParamCalls = added.filter((c) => 'param_p' in c.params && 'param_q' in c.params); + expect(bothParamCalls.length).toBe(1); + expect(bothParamCalls[0].params).toMatchObject({ param_p: 'x', param_q: 'y' }); + expect(added.filter((c) => 'param_p' in c.params && !('param_q' in c.params)).length).toBe(1); + expect(added.filter((c) => 'param_q' in c.params && !('param_p' in c.params)).length).toBe(1); + }); + + it('an identical-pair call (values + active equal to the committed state) publishes nothing and runs no wave', async () => { + const { exec, calls } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ document: bothDoc(), exec, queries: bothQueries() })); + await session.start(); + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, + { filterId: 'f2', value: 'y', active: true }, + ]); + const base = calls.length; + const snapshot = session.state.value; + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, + { filterId: 'f2', value: 'y', active: true }, + ]); + expect(calls.length).toBe(base); // no wave + expect(session.state.value).toBe(snapshot); // no publish + }); + + it('a mixed call (one entry changed, one identical) reruns ONLY the changed parameter\'s targets', async () => { + const { exec, calls } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ document: bothDoc(), exec, queries: bothQueries() })); + await session.start(); + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, + { filterId: 'f2', value: 'y', active: true }, + ]); + const base = calls.length; + await session.applyFilters([ + { filterId: 'f1', value: 'x', active: true }, // identical — not in `changed` + { filterId: 'f2', value: 'z', active: true }, // changed + ]); + const added = calls.slice(base); + // Only q's targets {tb, tboth} rerun; ta (p only) does NOT. + expect(added.every((c) => 'param_q' in c.params)).toBe(true); + expect(added.some((c) => 'param_p' in c.params && !('param_q' in c.params))).toBe(false); + expect(added.length).toBe(2); + expect(session.state.value.filters[1].value).toBe('z'); + }); + + it('a concurrent applyFilters wave is superseded by a newer commit (stale-wave guard) — the newer value wins', async () => { + let releaseFirst!: () => void; + const gate = new Promise((resolve) => { releaseFirst = resolve; }); + let n = 0; + const { exec } = makeExec(async (_sql, req) => { + n += 1; + const rows = [[req.params?.param_p]]; + if (n === 1) { await gate; return { columns: [{ name: 'n' }], rows }; } + return { columns: [{ name: 'n' }], rows }; + }); + const session = createDashboardViewerSession(makeDeps({ + document: doc({ tiles: [tile('t', 'q')], filters: [{ id: 'f1', parameter: 'p', defaultActive: false, defaultValue: '' }] }), + exec, queries: [query('q', 'SELECT {p:String} AS n')], + })); + await session.start(); + const first = session.applyFilters([{ filterId: 'f1', value: 'A', active: true }]); + await flush(); + const second = session.applyFilters([{ filterId: 'f1', value: 'B', active: true }]); + releaseFirst(); + await Promise.all([first, second]); + // The superseded 'A' run's result is discarded; the tile reflects 'B'. + expect(session.state.value.filters[0].value).toBe('B'); + expect(session.state.value.tiles[0].rows).toEqual([['B']]); + }); + + it('is a no-op after destroy', async () => { + const { exec, calls } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ document: bothDoc(), exec, queries: bothQueries() })); + await session.start(); + session.destroy(); + const base = calls.length; + await session.applyFilters([{ filterId: 'f1', value: 'x', active: true }]); + expect(calls.length).toBe(base); + expect(session.state.value.filters[0].value).toBe(''); + }); +}); + +// #335: `waveWallNowMs` — one wall-clock snapshot per execution wave, published +// on state and threaded into every relative-token resolution the wave runs +// (tiles AND filter sources), fixing the prior inconsistency where one refresh +// took several independent `deps.wallNow()` snapshots. +describe('waveWallNowMs single wave snapshot (#335)', () => { + // Two relative DateTime filters bound to tiles that run in DIFFERENT sub- + // phases of one refresh: `ts1` on an UNAFFECTED tile (first batch), `ts2` on + // a tile the source-backed `region` filter makes AFFECTED (second batch). + // With per-phase clock reads (the bug) they would resolve to different + // instants; a single snapshot makes them agree. + const splitDoc = () => doc({ + tiles: [tile('una', 'qUna'), tile('aff', 'qAff')], + filters: [ + { id: 'fts1', parameter: 'ts1', defaultActive: true, defaultValue: 'now' }, + { id: 'fts2', parameter: 'ts2', defaultActive: true, defaultValue: 'now' }, + { id: 'freg', parameter: 'region', sourceQueryId: 'srcq', defaultActive: true, defaultValue: 'R' }, + ], + }); + const splitQueries = () => [ + query('qUna', 'SELECT {ts1:DateTime} AS s'), + query('qAff', 'SELECT {region:String} AS r, {ts2:DateTime} AS e'), + query('srcq', "SELECT ['R'] AS region /* source */", { dashboard: { role: 'filter' } }), + ]; + const splitExec = () => makeExec((sql) => (sql.includes('/* source */') + ? { columns: [{ name: 'region', type: 'Array(String)' }], rows: [[['R']]] } + : { columns: [{ name: 'n' }], rows: [[1]] })); + const incWall = () => { + let n = 1_700_000_000_000; + const calls: number[] = []; + const wallNow = () => { n += 3_600_000; calls.push(n); return n; }; + return { wallNow, calls }; + }; + + it('is null before the first wave', () => { + const { exec } = splitExec(); + const session = createDashboardViewerSession(makeDeps({ document: splitDoc(), exec, queries: splitQueries() })); + expect(session.state.value.waveWallNowMs).toBeNull(); + }); + + it('captures ONE snapshot per refresh; both tiles\' relative tokens resolve against the same instant, published as waveWallNowMs', async () => { + const { exec, calls } = splitExec(); + const { wallNow, calls: wallCalls } = incWall(); + const session = createDashboardViewerSession(makeDeps({ document: splitDoc(), exec, queries: splitQueries(), wallNow })); + await session.start(); + // Exactly ONE wall-clock read for the whole refresh (first batch, filter + // wave, second batch all share it) — the fix. + expect(wallCalls.length).toBe(1); + expect(session.state.value.waveWallNowMs).toBe(wallCalls[0]); + const unaCall = calls.find((c) => 'param_ts1' in c.params)!; + const affCall = calls.find((c) => 'param_ts2' in c.params)!; + expect(unaCall).toBeDefined(); + expect(affCall).toBeDefined(); + // `now` in the first-batch tile and `now` in the second-batch tile resolved + // to the SAME serialized instant. + expect(unaCall.params.param_ts1).toBe(affCall.params.param_ts2); + }); + + it('an applyFilters wave updates waveWallNowMs to its own fresh snapshot', async () => { + const { exec } = splitExec(); + const { wallNow } = incWall(); + const session = createDashboardViewerSession(makeDeps({ document: splitDoc(), exec, queries: splitQueries(), wallNow })); + await session.start(); + const afterRefresh = session.state.value.waveWallNowMs!; + await session.applyFilters([{ filterId: 'fts1', value: '-1h', active: true }]); + const afterApply = session.state.value.waveWallNowMs!; + expect(afterApply).toBeGreaterThan(afterRefresh); + }); + + it('refreshTile is a wave of one: fresh snapshot published and bound into the tile', async () => { + const { exec, calls } = splitExec(); + const { wallNow } = incWall(); + const session = createDashboardViewerSession(makeDeps({ document: splitDoc(), exec, queries: splitQueries(), wallNow })); + await session.start(); + const afterStart = session.state.value.waveWallNowMs!; + calls.length = 0; + await session.refreshTile('una'); + const afterTile = session.state.value.waveWallNowMs!; + expect(afterTile).toBeGreaterThan(afterStart); + // The refreshed tile's relative token bound against the published snapshot, + // not a second untethered clock read. + const run = calls.find((c) => 'param_ts1' in c.params)!; + expect(run.params.param_ts1).toBe(String(Math.floor(afterTile / 1000))); + }); + + it('a commit with a dependent filter source shares one snapshot across the source wave and the affected-panel wave', async () => { + // `anchor` (root) feeds `srcq` (which depends on {anchor:DateTime}); the + // tile declares BOTH {anchor} (so the affected-panel wave targets it) and + // {reg} (the curated source-backed param). Committing `anchor` reruns the + // source AND the tile — both must bind `anchor` resolved against the SAME + // instant. + const { exec, calls } = makeExec((sql) => (sql.includes('/* source */') + ? { columns: [{ name: 'reg', type: 'Array(String)' }], rows: [[['R']]] } + : { columns: [{ name: 'n' }], rows: [[1]] })); + const { wallNow } = incWall(); + const session = createDashboardViewerSession(makeDeps({ + document: doc({ + tiles: [tile('taff', 'qaff')], + filters: [ + { id: 'fanchor', parameter: 'anchor', defaultActive: true, defaultValue: 'now' }, + { id: 'freg', parameter: 'reg', sourceQueryId: 'srcq', defaultActive: true, defaultValue: 'R' }, + ], + }), + exec, wallNow, + queries: [ + query('qaff', 'SELECT {anchor:DateTime} AS a, {reg:String} AS r'), + query('srcq', "SELECT ['R'] AS reg FROM x WHERE ts >= {anchor:DateTime} /* source */", { dashboard: { role: 'filter' } }), + ], + })); + await session.start(); + const base = calls.length; + await session.applyFilters([{ filterId: 'fanchor', value: '-1h', active: true }]); + const added = calls.slice(base); + const srcCall = added.find((c) => c.sql.includes('/* source */'))!; + const tileCall = added.find((c) => c.sql.includes('AS a'))!; + expect(srcCall).toBeDefined(); + expect(tileCall).toBeDefined(); + // The source's {anchor} and the tile's {anchor}, resolved in the source + // wave and the affected-panel wave of ONE commit, share the snapshot. + expect(srcCall.params.param_anchor).toBe(tileCall.params.param_anchor); + expect(session.state.value.waveWallNowMs).not.toBeNull(); + }); +}); + +// #335: the resolver seam wired into the session — `timeRangeGroups`, computed +// ONCE at construction AFTER the #189 source-fallback loop, so `sourceQueryId` +// is read post-fallback (a stripped source is a plain, groupable filter). +describe('timeRangeGroups resolution (#335)', () => { + it('resolves a plain scalar date-like from/to pair into one group', () => { + const session = createDashboardViewerSession(makeDeps({ + document: doc({ + tiles: [tile('t', 'q')], + filters: [{ id: 'ff', parameter: 'from' }, { id: 'ft', parameter: 'to' }], + }), + queries: [query('q', 'SELECT {from:DateTime} AS f, {to:DateTime} AS t2')], + })); + expect(session.timeRangeGroups.length).toBe(1); + expect(session.timeRangeGroups[0]).toMatchObject({ + fromFilterId: 'ff', toFilterId: 'ft', fromParameter: 'from', toParameter: 'to', + }); + }); + + it('excludes a curated (surviving source-backed) date-like filter from grouping', () => { + const session = createDashboardViewerSession(makeDeps({ + document: doc({ + tiles: [tile('t', 'q')], + filters: [ + { id: 'ff', parameter: 'from', sourceQueryId: 'srcf', defaultActive: true, defaultValue: 'x' }, + { id: 'ft', parameter: 'to' }, + ], + }), + queries: [ + query('q', 'SELECT {from:DateTime} AS f, {to:DateTime} AS t2'), + query('srcf', "SELECT ['x'] AS opt /* source */", { dashboard: { role: 'filter' } }), + ], + })); + // `from` keeps its source contract (curated) → never paired → no group. + expect(session.timeRangeGroups).toEqual([]); + }); + + it('does not group a non-date-like pair', () => { + const session = createDashboardViewerSession(makeDeps({ + document: doc({ + tiles: [tile('t', 'q')], + filters: [{ id: 'ff', parameter: 'from' }, { id: 'ft', parameter: 'to' }], + }), + queries: [query('q', 'SELECT {from:String} AS f, {to:String} AS t2')], + })); + expect(session.timeRangeGroups).toEqual([]); + }); + + it('is computed once at construction and not recomputed by a filter commit', async () => { + const { exec } = makeExec(() => ({ columns: [{ name: 'n' }], rows: [[1]] })); + const session = createDashboardViewerSession(makeDeps({ + document: doc({ + tiles: [tile('t', 'q')], + filters: [{ id: 'ff', parameter: 'from' }, { id: 'ft', parameter: 'to' }], + }), + exec, queries: [query('q', 'SELECT {from:DateTime} AS f, {to:DateTime} AS t2')], + })); + await session.start(); + const groups = session.timeRangeGroups; + await session.applyFilters([{ filterId: 'ff', value: '-1d', active: true }]); + expect(session.timeRangeGroups).toBe(groups); // same reference — never recomputed + }); +}); diff --git a/tests/unit/dashboard.test.ts b/tests/unit/dashboard.test.ts index a18b9f7b..96e7a92d 100644 --- a/tests/unit/dashboard.test.ts +++ b/tests/unit/dashboard.test.ts @@ -2765,6 +2765,143 @@ describe('renderDashboard — searchable multiselect + array-wrapped curated fil }); }); +// #335: the compound time-range control, integrated end to end through the +// REAL session — a from/to date-like filter pair forms a `DashboardTimeRangeGroup` +// (session.timeRangeGroups), the bar renders one compound control in its "Time" +// section (suppressing the pair's two individual fields), Apply commits both +// bounds atomically via `session.applyFilters` in one wave, per-group recents +// accumulate the OUTGOING committed pairs, and the closed trigger re-resolves +// its label per wave (a live relative range) without a bar rebuild. +describe('renderDashboard — compound time-range control (#335)', () => { + const PAIR = 'SELECT k, v FROM a WHERE ts >= {from:DateTime} AND ts < {to:DateTime}'; + const clickEv = (): MouseEvent => new MouseEvent('click', { bubbles: true }); + const inputEv = (): Event => new Event('input', { bubbles: true }); + // Drive one full time-range Apply on the CURRENTLY-rendered trigger. + const applyRange = async (app: TestApp, from: string, to: string): Promise => { + qs(app.root, '.trf-trigger').dispatchEvent(clickEv()); + const inputs = qsa(document.body, '.trf-input'); + inputs[0].value = from; inputs[0].dispatchEvent(inputEv()); + inputs[1].value = to; inputs[1].dispatchEvent(inputEv()); + qs(document.body, '.trf-btn-primary').dispatchEvent(clickEv()); + await flush(); + }; + + it('renders one compound control for the from/to pair, suppressing the two individual fields, with Time/Filters labels', async () => { + const { app } = dashApp({ + workspace: wsWith({ + queries: [q('q1', PAIR + ' AND r = {region:String}')], + tiles: [{ id: 't1', queryId: 'q1' }], + }), + }); + await render(app); + expect(qs(app.root, '.trf-trigger')).not.toBeNull(); + expect(qsa(app.root, '.dash-filter-host .flabel').map((n) => n.textContent)).toEqual(['Time', 'Filters']); + // The pair's own two fields are gone; only the non-group field remains. + const names = qsa(app.root, '.dash-filter-host .var-field:not(.is-time-range) .var-name').map((n) => n.textContent); + expect(names).toEqual(['region']); + }); + + it('Apply commits BOTH bounds through session.applyFilters in one wave and announces the range', async () => { + const { app, calls } = dashApp({ + workspace: wsWith({ queries: [q('q1', PAIR)], tiles: [{ id: 't1', queryId: 'q1' }] }), + }); + await render(app); + document.body.appendChild(rootEl(app)); + const before = calls.length; + await applyRange(app, '-1d', 'now'); + const added = calls.slice(before).filter((c) => 'param_from' in c.params || 'param_to' in c.params); + expect(added.length).toBeGreaterThanOrEqual(1); + // One atomic wave binds BOTH parameters on every affected tile call. + expect(added.every((c) => 'param_from' in c.params && 'param_to' in c.params)).toBe(true); + expect(qs(app.root, '.dash-toolbar > .sr-only').textContent).toBe('Time range applied: -1d → now'); + // The bar rebuilt on the committed-value change and now shows a resolved, + // active range (not "Not set"). + expect(qs(app.root, '.trf-trigger').textContent).not.toBe('Not set'); + rootEl(app).remove(); + }); + + it('pushes the OUTGOING committed pair to per-group recents on a changing re-apply, never on the first commit from unset', async () => { + const { app } = dashApp({ + workspace: wsWith({ queries: [q('q1', PAIR)], tiles: [{ id: 't1', queryId: 'q1' }] }), + }); + await render(app); + document.body.appendChild(rootEl(app)); + // First commit — the outgoing pair was unset/inactive, so nothing is pushed. + await applyRange(app, '-1d', 'now'); + qs(app.root, '.trf-trigger').dispatchEvent(clickEv()); + expect(qs(document.body, '.trf-empty')?.textContent).toContain('No recent ranges yet'); + // Cancel out (never pushes). + qsa(document.body, '.trf-btn')[0].dispatchEvent(clickEv()); // Cancel + // Second, CHANGING commit — the outgoing active pair (-1d → now) is pushed. + await applyRange(app, '-7d', 'now'); + qs(app.root, '.trf-trigger').dispatchEvent(clickEv()); + expect(qs(document.body, '.trf-recent').textContent).toBe('-1d → now'); + rootEl(app).remove(); + }); + + it('re-resolves the closed trigger label per wave WITHOUT rebuilding the bar (a relative range moves as `now` advances)', async () => { + let clock = 1000; + const { app } = dashApp({ + workspace: wsWith({ + queries: [q('q1', PAIR)], + tiles: [{ id: 't1', queryId: 'q1' }], + filters: [ + { id: 'from', parameter: 'from', defaultValue: '-1d', defaultActive: true }, + { id: 'to', parameter: 'to', defaultValue: 'now', defaultActive: true }, + ], + }), + }); + app.wallNow = () => clock; + await render(app); + const trigger = qs(app.root, '.trf-trigger'); + const before = trigger.textContent; + expect(trigger.classList.contains('is-error')).toBe(false); + clock = 1000 + 3 * 86_400_000; // three days later + await (runOnclick(qs(app.root, '.dash-refresh')) as Promise); + // No rebuild — the SAME trigger node, its label re-resolved in place. + expect(qs(app.root, '.trf-trigger')).toBe(trigger); + expect(trigger.textContent).not.toBe(before); + rootEl(app).remove(); + }); + + it('restores focus onto the fresh time-range trigger after a commit-triggered rebuild', async () => { + const { app } = dashApp({ + workspace: wsWith({ queries: [q('q1', PAIR)], tiles: [{ id: 't1', queryId: 'q1' }] }), + }); + await render(app); + document.body.appendChild(rootEl(app)); + const oldTrigger = qs(app.root, '.trf-trigger'); + oldTrigger.dispatchEvent(clickEv()); + const inputs = qsa(document.body, '.trf-input'); + inputs[0].value = '-1d'; inputs[0].dispatchEvent(inputEv()); + inputs[1].value = 'now'; inputs[1].dispatchEvent(inputEv()); + qs(document.body, '.trf-btn-primary').dispatchEvent(clickEv()); + // The synchronous applyFilters publish rebuilt the bar, detaching the old + // trigger — focus lands on the fresh one (never stranded at ). + const newTrigger = qs(app.root, '.trf-trigger'); + expect(newTrigger).not.toBe(oldTrigger); + expect(document.activeElement).toBe(newTrigger); + rootEl(app).remove(); + }); + + it('renders and commits the time-range control in a read-only (detached) dashboard', async () => { + const detached = wsWith({ + id: 'd', queries: [q('q1', PAIR)], tiles: [{ id: 't1', queryId: 'q1' }], + }); + const { app, calls } = modeApp({ + workspace: null, detached, openSource: { kind: 'current-workspace', workspaceId: 'w', dashboardId: 'd' }, + }); + await render(app); + document.body.appendChild(rootEl(app)); + expect(qs(app.root, '.trf-trigger')).not.toBeNull(); + const before = calls.length; + await applyRange(app, '-1d', 'now'); + const added = calls.slice(before).filter((c) => 'param_from' in c.params && 'param_to' in c.params); + expect(added.length).toBeGreaterThanOrEqual(1); + rootEl(app).remove(); + }); +}); + // #359: the shared-source filter wave now publishes `optionsRev` (bumped ONLY // when a curated source's option VALUE CONTENT changes — including a clear to // null — never on an unchanged republish) and `filterDiagnostics` (its own diff --git a/tests/unit/dom.test.ts b/tests/unit/dom.test.ts index 58e01aa9..124409ff 100644 --- a/tests/unit/dom.test.ts +++ b/tests/unit/dom.test.ts @@ -71,6 +71,30 @@ describe('fixedAnchor', () => { it('handles zero and fractional coordinates', () => { expect(fixedAnchor({ bottom: 10.5, left: 50.25 }, { gap: 0, min: 0 })).toMatchObject({ top: 10.5, left: 50.25 }); }); + + // #335: viewportW + panelW is the left-align right-edge CLAMP (distinct from + // viewportW alone, which still right-aligns). Pure arithmetic — testable with + // synthetic numbers since a headless realm's rects are all-zero anyway. + it('clamps the left inset so a panelW-wide panel stays inside the right edge', () => { + const a: AnchorResult = fixedAnchor({ bottom: 40, left: 900 }, { viewportW: 1000, panelW: 200 }); + expect(a.top).toBe(46); + expect(a.left).toBe(792); // 1000 - 200 - 8(min); trigger.left 900 would overflow + expect(a.right).toBeUndefined(); + }); + it('does not lower the left inset when the panel already fits', () => { + const a: AnchorResult = fixedAnchor({ bottom: 40, left: 100 }, { viewportW: 1000, panelW: 200 }); + expect(a.left).toBe(100); // min(100, 792) — the natural left wins + }); + it('floors the clamped left inset at `min` even when the panel cannot fit', () => { + const a: AnchorResult = fixedAnchor({ bottom: 0, left: 5000 }, { viewportW: 100, panelW: 200 }); + expect(a.left).toBe(8); // maxLeft floors at min(8); left = min(5000, 8) + }); + it('ignores panelW when no viewportW is given (plain left-align, no clamp)', () => { + const a: AnchorResult = fixedAnchor({ bottom: 40, left: 100 }, { panelW: 50 }); + expect(a.top).toBe(46); + expect(a.left).toBe(100); + expect(a.right).toBeUndefined(); + }); }); describe('s (SVG namespace)', () => { diff --git a/tests/unit/filter-bar.test.ts b/tests/unit/filter-bar.test.ts index 6f8caffb..7de667b1 100644 --- a/tests/unit/filter-bar.test.ts +++ b/tests/unit/filter-bar.test.ts @@ -3,6 +3,8 @@ import { analyzeParameterizedSources, fieldControls } from '../../src/core/param import type { FieldControl, PreparedFieldState } from '../../src/core/param-pipeline.js'; import { buildFilterBar, FILTER_DEBOUNCE_MS } from '../../src/ui/filter-bar.js'; import { emptyRecentMap, recordRecent } from '../../src/core/recent-values.js'; +import { parseParamType } from '../../src/core/param-type.js'; +import type { DashboardTimeRangeGroup, TimeRangeRecent } from '../../src/core/time-range.js'; import { makeApp } from '../helpers/fake-app.js'; // The field-family construction, debounce, commit, conflict, and optional @@ -38,8 +40,8 @@ describe('buildFilterBar (shared filter row)', () => { expect(bar.el.querySelectorAll('.var-field').length).toBe(0); expect(() => bar.dispose()).not.toThrow(); // no fields, no timers — a no-op expect(() => bar.updateStatus({})).not.toThrow(); // no curated fields — a no-op - expect(bar.openMultiSelectParam()).toBeNull(); // no multiselect fields at all — always null - expect(() => bar.focusMultiSelectTrigger('x')).not.toThrow(); // unknown param — a no-op + expect(bar.openPopoverKey()).toBeNull(); // no multiselect fields at all — always null + expect(() => bar.focusFieldTrigger('x')).not.toThrow(); // unknown param — a no-op }); it('defaults to app.document and no group role when no options are passed', () => { @@ -454,7 +456,7 @@ describe('buildFilterBar (shared filter row)', () => { expect(trigger.disabled).toBe(true); }); - it('openMultiSelectParam() reflects an open popover\'s parameter, and dispose() cancels it with no onApplyCurated call', () => { + it('openPopoverKey() reflects an open popover\'s parameter, and dispose() cancels it with no onApplyCurated call', () => { const app = makeApp(); const onApplyCurated = vi.fn(); const bar = buildFilterBar(app, paramsFor('SELECT {x:String}'), () => {}, okField, { @@ -467,9 +469,9 @@ describe('buildFilterBar (shared filter row)', () => { onApplyCurated, }); document.body.appendChild(bar.el); - expect(bar.openMultiSelectParam()).toBeNull(); + expect(bar.openPopoverKey()).toBeNull(); bar.el.querySelector('.ms-trigger')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); - expect(bar.openMultiSelectParam()).toBe('x'); + expect(bar.openPopoverKey()).toBe('x'); expect(document.body.querySelector('.ms-popover')).not.toBeNull(); bar.dispose(); expect(document.body.querySelector('.ms-popover')).toBeNull(); @@ -477,7 +479,7 @@ describe('buildFilterBar (shared filter row)', () => { bar.el.remove(); }); - it('focusMultiSelectTrigger(name) focuses that parameter\'s trigger (#189 F2b)', () => { + it('focusFieldTrigger(key) focuses that parameter\'s trigger (#189 F2b)', () => { const app = makeApp(); const bar = buildFilterBar(app, paramsFor('SELECT {x:String}'), () => {}, okField, { curatedFields: { @@ -489,7 +491,7 @@ describe('buildFilterBar (shared filter row)', () => { }); document.body.appendChild(bar.el); const trigger = bar.el.querySelector('.ms-trigger') as HTMLButtonElement; - bar.focusMultiSelectTrigger('x'); + bar.focusFieldTrigger('x'); expect(document.activeElement).toBe(trigger); bar.el.remove(); }); @@ -555,6 +557,137 @@ describe('buildFilterBar (shared filter row)', () => { }); }); + // #335: the compound time-range control section + the handle-map + // unification's new/renamed seams. `buildTimeRangeField`'s own behavior + // (popover columns, staged editing, validation) is covered exhaustively by + // time-range-field.test.ts — these exercise buildFilterBar's INTEGRATION of + // it: the "Time" section ahead of the fields, pair suppression, the unified + // key-space (`group:…`), and `refreshTimeRangeLabels` delegation. + describe('time-range section + unified handle map (#335)', () => { + const dt = parseParamType('DateTime'); + const dtGroup = (): DashboardTimeRangeGroup => ({ + key: 'fromto', fromFilterId: 'from', toFilterId: 'to', + fromParameter: 'from', toParameter: 'to', fromType: dt, toType: dt, + }); + const trEntry = (over: Partial<{ + fromValue: string; toValue: string; active: boolean; waveNowMs: number | null; + recents: () => readonly TimeRangeRecent[]; + }> = {}) => ({ + group: dtGroup(), fromValue: '', toValue: '', active: false, waveNowMs: 0 as number | null, + recents: (): readonly TimeRangeRecent[] => [], ...over, + }); + // from/to (grouped) + region (plain) — the group owns from/to. + const groupParams = paramsFor('SELECT k FROM t WHERE d >= {from:DateTime} AND d < {to:DateTime} AND r = {region:String}'); + + it('renders a "Time" section (label + control + separator) AHEAD of the fields, suppresses the pair, and labels the rest "Filters"', () => { + const app = makeApp(); + const bar = buildFilterBar(app, groupParams, () => {}, okField, { timeRange: [trEntry()] }); + // Two section labels, in order: Time then Filters. + expect([...bar.el.querySelectorAll('.flabel')].map((n) => n.textContent)).toEqual(['Time', 'Filters']); + // Exactly one compound control, one separator. + expect(bar.el.querySelectorAll('.var-field.is-time-range').length).toBe(1); + expect(bar.el.querySelector('.trf-trigger')).not.toBeNull(); + expect(bar.el.querySelectorAll('.trf-sep').length).toBe(1); + // The pair's own two individual fields are gone; only the non-group field remains. + const names = [...bar.el.querySelectorAll('.dash-filters > .var-field:not(.is-time-range) .var-name')].map((n) => n.textContent); + expect(names).toEqual(['region']); + // DOM order: Time label, control, separator, Filters label, region field. + const order = [...bar.el.children].map((c) => c.className.split(' ')[0] + (c.classList.contains('flabel') ? ':' + c.textContent : '')); + expect(order).toEqual(['flabel:Time', 'var-field', 'trf-sep', 'flabel:Filters', 'var-field']); + }); + + it('omits the "Filters" label when every remaining param is grouped (no non-group field left)', () => { + const app = makeApp(); + const params = paramsFor('SELECT k FROM t WHERE d >= {from:DateTime} AND d < {to:DateTime}'); + const bar = buildFilterBar(app, params, () => {}, okField, { timeRange: [trEntry()] }); + expect([...bar.el.querySelectorAll('.flabel')].map((n) => n.textContent)).toEqual(['Time']); + expect(bar.el.querySelector('.var-field:not(.is-time-range)')).toBeNull(); + }); + + it('renders no time section (no flabel/trf-sep) when timeRange is absent or empty — the plain path', () => { + const app = makeApp(); + const absent = buildFilterBar(app, groupParams, () => {}, okField); + expect(absent.el.querySelector('.flabel')).toBeNull(); + expect(absent.el.querySelector('.trf-sep')).toBeNull(); + expect(absent.el.querySelector('.trf-trigger')).toBeNull(); + // All three params render as ordinary fields (nothing suppressed). + expect([...absent.el.querySelectorAll('.var-name')].map((n) => n.textContent)).toEqual(['from', 'to', 'region']); + const empty = buildFilterBar(app, groupParams, () => {}, okField, { timeRange: [] }); + expect(empty.el.querySelector('.flabel')).toBeNull(); + }); + + it('openPopoverKey()/focusFieldTrigger() speak the group key-space; dispose() cancels an open time-range popover', () => { + const app = makeApp(); + const onApplyTimeRange = vi.fn(); + const bar = buildFilterBar(app, groupParams, () => {}, okField, { timeRange: [trEntry()], onApplyTimeRange }); + document.body.appendChild(bar.el); + const key = 'group:fromto'; + expect(bar.openPopoverKey()).toBeNull(); + const trigger = bar.el.querySelector('.trf-trigger') as HTMLButtonElement; + trigger.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(document.body.querySelector('.trf-popover')).not.toBeNull(); + expect(bar.openPopoverKey()).toBe(key); + // focusFieldTrigger addresses the same key-space. + bar.focusFieldTrigger(key); + expect(document.activeElement).toBe(trigger); + // dispose while open is a silent Cancel: no onApply, popover gone. + bar.dispose(); + expect(document.body.querySelector('.trf-popover')).toBeNull(); + expect(onApplyTimeRange).not.toHaveBeenCalled(); + bar.el.remove(); + }); + + it('an Apply routes through onApplyTimeRange with the group + trimmed bounds', () => { + const app = makeApp(); + const onApplyTimeRange = vi.fn(); + const bar = buildFilterBar(app, groupParams, () => {}, okField, { timeRange: [trEntry()], onApplyTimeRange }); + document.body.appendChild(bar.el); + (bar.el.querySelector('.trf-trigger') as HTMLButtonElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); + const inputs = [...document.body.querySelectorAll('.trf-input')] as HTMLInputElement[]; + inputs[0].value = '-1d'; inputs[0].dispatchEvent(new Event('input', { bubbles: true })); + inputs[1].value = 'now'; inputs[1].dispatchEvent(new Event('input', { bubbles: true })); + (document.body.querySelector('.trf-btn-primary') as HTMLButtonElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(onApplyTimeRange).toHaveBeenCalledTimes(1); + const [group, from, to] = onApplyTimeRange.mock.calls[0]; + expect(group.key).toBe('fromto'); + expect(from).toBe('-1d'); + expect(to).toBe('now'); + bar.el.remove(); + }); + + it('refreshTimeRangeLabels(nowMs) re-resolves every time-range control label in place; a no-op with no controls', () => { + const app = makeApp(); + const bar = buildFilterBar(app, groupParams, () => {}, okField, { + timeRange: [trEntry({ fromValue: '-1d', toValue: 'now', active: true, waveNowMs: 0 })], + }); + const trigger = bar.el.querySelector('.trf-trigger') as HTMLButtonElement; + const before = trigger.textContent; + // A day later — the relative range's resolved absolute bounds move. + bar.refreshTimeRangeLabels(86_400_000); + expect(trigger.textContent).not.toBe(before); + // No time-range controls at all → a harmless no-op. + const plain = buildFilterBar(app, paramsFor('SELECT {x:String}'), () => {}, okField); + expect(() => plain.refreshTimeRangeLabels(1)).not.toThrow(); + }); + + it('recents pick applies immediately through onApplyTimeRange after closing', () => { + const app = makeApp(); + const onApplyTimeRange = vi.fn(); + const recents: TimeRangeRecent[] = [{ from: '-7d', to: 'now' }]; + const bar = buildFilterBar(app, groupParams, () => {}, okField, { + timeRange: [trEntry({ recents: () => recents })], onApplyTimeRange, + }); + document.body.appendChild(bar.el); + (bar.el.querySelector('.trf-trigger') as HTMLButtonElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); + const recentBtn = document.body.querySelector('.trf-recent') as HTMLButtonElement; + expect(recentBtn.textContent).toBe('-7d → now'); + recentBtn.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(document.body.querySelector('.trf-popover')).toBeNull(); // closed first + expect(onApplyTimeRange).toHaveBeenCalledWith(expect.objectContaining({ key: 'fromto' }), '-7d', 'now'); + bar.el.remove(); + }); + }); + it('dispose() clears a pending debounce timer so a later value edit never fires the stale commit (#276)', () => { vi.useFakeTimers(); try { diff --git a/tests/unit/popover.test.ts b/tests/unit/popover.test.ts new file mode 100644 index 00000000..f226fb5a --- /dev/null +++ b/tests/unit/popover.test.ts @@ -0,0 +1,277 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { openAnchoredDialog } from '../../src/ui/popover.js'; +import type { AnchoredDialogOptions } from '../../src/ui/popover.js'; +import { h } from '../../src/ui/dom.js'; + +afterEach(() => document.body.replaceChildren()); + +const key = (target: EventTarget, k: string, shiftKey = false): boolean => + target.dispatchEvent(new KeyboardEvent('keydown', { key: k, shiftKey, bubbles: true, cancelable: true })); + +// A trigger already mounted in the body, plus a content element carrying a +// couple of focusable rows (input + button) — the primitive's Tab trap keys +// off `input, button`. +function setup(overrides: Partial = {}): { + trigger: HTMLButtonElement; + content: HTMLElement; + input: HTMLInputElement; + button: HTMLButtonElement; + open: () => ReturnType; + onClose: ReturnType; +} { + const trigger = h('button', { type: 'button', 'aria-expanded': 'false' }) as HTMLButtonElement; + document.body.appendChild(trigger); + const input = h('input', { type: 'text', class: 'pv-input' }) as HTMLInputElement; + const button = h('button', { type: 'button', class: 'pv-button' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, input, button); + const onClose = vi.fn(); + const open = (): ReturnType => + openAnchoredDialog({ + document, + trigger, + ariaLabel: 'Test dialog', + content, + dialogClassName: 'pv-popover', + onClose, + ...overrides, + }); + return { trigger, content, input, button, open, onClose }; +} + +const dialogEl = (): HTMLElement | null => document.body.querySelector('.pv-popover'); +const overlayEl = (cls = '.ms-overlay'): HTMLElement | null => document.body.querySelector(cls); + +describe('openAnchoredDialog — mount + ARIA', () => { + it('mounts an overlay and a dialog with role/aria-modal/aria-label, and appends the content', () => { + const { content, open } = setup(); + const handle = open(); + const dialog = dialogEl()!; + expect(dialog).not.toBeNull(); + expect(dialog.getAttribute('role')).toBe('dialog'); + expect(dialog.getAttribute('aria-modal')).toBe('true'); + expect(dialog.getAttribute('aria-label')).toBe('Test dialog'); + expect(dialog.contains(content)).toBe(true); + expect(overlayEl()).not.toBeNull(); // default overlay class + expect(handle.dialog).toBe(dialog); + expect(handle.isOpen()).toBe(true); + }); + + it('honors a custom overlayClassName', () => { + const { open } = setup({ overlayClassName: 'trf-overlay' }); + open(); + expect(overlayEl('.ms-overlay')).toBeNull(); + expect(overlayEl('.trf-overlay')).not.toBeNull(); + }); + + it('sets aria-expanded true on open and false on close', () => { + const { trigger, open } = setup(); + expect(trigger.getAttribute('aria-expanded')).toBe('false'); + const handle = open(); + expect(trigger.getAttribute('aria-expanded')).toBe('true'); + handle.close(); + expect(trigger.getAttribute('aria-expanded')).toBe('false'); + }); +}); + +describe('openAnchoredDialog — placement', () => { + it('positions the dialog under the trigger rect via fixedAnchor (fixed, min-floored in a zero-rect realm)', () => { + const { open } = setup(); + open(); + const dialog = dialogEl()!; + expect(dialog.style.position).toBe('fixed'); + // happy-dom rects are all-zero → fixedAnchor floors to top=6 (default gap), + // left=8 (default min). + expect(dialog.style.top).toBe('6px'); + expect(dialog.style.left).toBe('8px'); + }); + + it('minWidthFromTrigger true sets a min-width from the trigger rect; false/omit leaves it unset', () => { + const withMin = setup({ minWidthFromTrigger: true }); + withMin.open(); + expect(dialogEl()!.style.minWidth).toBe('0px'); // trigger rect width is 0 in happy-dom + document.body.replaceChildren(); + + const without = setup(); + without.open(); + expect(dialogEl()!.style.minWidth).toBe(''); + }); + + it('overlay is fixed and covers the viewport', () => { + const { open } = setup(); + open(); + const overlay = overlayEl()!; + expect(overlay.style.position).toBe('fixed'); + expect(overlay.style.inset).toBe('0'); + }); + + it('clampToViewport left-aligns via the fixedAnchor clamp path (reads defaultView width)', () => { + const { open } = setup({ clampToViewport: true }); + open(); + const dialog = dialogEl()!; + // Clamp path always yields a left inset (never a right one). + expect(dialog.style.left).not.toBe(''); + expect(dialog.style.right).toBe(''); + }); + + it('clampToViewport tolerates a realm without a defaultView (viewport width reads 0)', () => { + const doc = document.implementation.createHTMLDocument(''); + const trigger = doc.createElement('button'); + doc.body.appendChild(trigger); + const content = doc.createElement('div'); + const handle = openAnchoredDialog({ + document: doc, trigger, ariaLabel: 'x', content, + dialogClassName: 'pv-popover', clampToViewport: true, + }); + const dialog = doc.querySelector('.pv-popover') as HTMLElement; + expect(dialog).not.toBeNull(); + expect(dialog.style.left).not.toBe(''); + handle.close(); + }); +}); + +describe('openAnchoredDialog — dismissal paths', () => { + it('Escape closes and returns focus to the trigger; onClose fires', () => { + const { trigger, open, onClose } = setup(); + const handle = open(); + key(dialogEl()!, 'Escape'); + expect(handle.isOpen()).toBe(false); + expect(dialogEl()).toBeNull(); + expect(document.activeElement).toBe(trigger); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('a non-Escape key does not close', () => { + const { open } = setup(); + const handle = open(); + key(dialogEl()!, 'a'); + expect(handle.isOpen()).toBe(true); + }); + + it('a backdrop mousedown+click closes; onClose fires', () => { + const { open, onClose } = setup(); + const handle = open(); + const overlay = overlayEl()!; + overlay.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + overlay.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(handle.isOpen()).toBe(false); + expect(dialogEl()).toBeNull(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('close() is idempotent — a second call neither throws nor re-fires onClose', () => { + const { open, onClose } = setup(); + const handle = open(); + handle.close(); + expect(() => handle.close()).not.toThrow(); + expect(handle.isOpen()).toBe(false); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('close({ skipFocus: true }) tears down but leaves the trigger unfocused', () => { + const { trigger, open } = setup(); + const handle = open(); + // Move focus off the trigger first so a skipped focus-return is observable. + const input = document.querySelector('.pv-input') as HTMLInputElement; + input.focus(); + handle.close({ skipFocus: true }); + expect(dialogEl()).toBeNull(); + expect(document.activeElement).not.toBe(trigger); + }); +}); + +describe('openAnchoredDialog — Tab focus trap', () => { + const tab = (target: EventTarget, shiftKey = false): boolean => key(target, 'Tab', shiftKey); + + it('Tab from the last focusable wraps to the first', () => { + const { open, input, button } = setup(); + open(); + button.focus(); // last focusable + tab(dialogEl()!); + expect(document.activeElement).toBe(input); // first + }); + + it('Shift+Tab from the first focusable wraps to the last', () => { + const { open, input, button } = setup(); + open(); + input.focus(); // first focusable + tab(dialogEl()!, true); + expect(document.activeElement).toBe(button); // last + }); + + it('recomputes the focusable set on every press: disabling the last row moves the wrap target', () => { + // Three rows; disable the last so the trap must recompute rather than reuse + // a cached list. + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const a = h('input', { class: 'r-a' }) as HTMLInputElement; + const b = h('button', { class: 'r-b' }) as HTMLButtonElement; + const c = h('button', { class: 'r-c' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, a, b, c); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + c.disabled = true; // now the last focusable is b + b.focus(); + tab(dialogEl()!); // from the (new) last → wraps to first (a) + expect(document.activeElement).toBe(a); + }); + + it('a hidden row is excluded from the trap', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const a = h('input', { class: 'r-a' }) as HTMLInputElement; + const wrap = h('div', { hidden: true }, h('button', { class: 'r-b' })); + const c = h('button', { class: 'r-c' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, a, wrap, c); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + c.focus(); // last visible focusable + tab(dialogEl()!); + expect(document.activeElement).toBe(a); + }); + + it('Tab from a middle element does not trap (browser default order applies)', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const a = h('input', { class: 'r-a' }) as HTMLInputElement; + const b = h('input', { class: 'r-b' }) as HTMLInputElement; + const c = h('button', { class: 'r-c' }) as HTMLButtonElement; + const content = h('div', { style: { display: 'contents' } }, a, b, c); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + b.focus(); // middle + const forward = tab(dialogEl()!); + const backward = tab(dialogEl()!, true); + expect(forward).toBe(true); // not preventDefault-ed + expect(backward).toBe(true); + }); + + it('a non-Tab key inside the dialog is ignored by the trap', () => { + const { open, button } = setup(); + open(); + button.focus(); + const handled = key(dialogEl()!, 'ArrowDown'); + expect(handled).toBe(true); // not preventDefault-ed + expect(document.activeElement).toBe(button); // focus unchanged + }); + + it('Tab with no focusable content is a no-op (empty-set guard)', () => { + const trigger = h('button', {}) as HTMLButtonElement; + document.body.appendChild(trigger); + const content = h('div', {}, h('span', {}, 'no focusables here')); + openAnchoredDialog({ document, trigger, ariaLabel: 'x', content, dialogClassName: 'pv-popover' }); + const handled = tab(dialogEl()!); + expect(handled).toBe(true); // untrapped — browser handles it + }); +}); + +describe('openAnchoredDialog — initialFocus', () => { + it('focuses the element the callback returns', () => { + const { open, input } = setup({ initialFocus: (dialog) => dialog.querySelector('.pv-input') }); + open(); + expect(document.activeElement).toBe(input); + }); + + it('omitting initialFocus (or returning null) leaves focus where it was', () => { + const { trigger, open } = setup({ initialFocus: () => null }); + trigger.focus(); + open(); + expect(document.activeElement).toBe(trigger); + }); +}); diff --git a/tests/unit/relative-time-field.test.ts b/tests/unit/relative-time-field.test.ts index 3f3ae80b..6c26cb96 100644 --- a/tests/unit/relative-time-field.test.ts +++ b/tests/unit/relative-time-field.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect, vi } from 'vitest'; -import { buildRelativeTimeField, filterPresets, RELATIVE_TIME_PRESETS } from '../../src/ui/relative-time-field.js'; +import { + buildRelativeTimeField, + filterPresets, + RELATIVE_TIME_PRESETS, + TIME_RANGE_CONSTANTS, + filterTokenList, +} from '../../src/ui/relative-time-field.js'; import type { BuildRelativeTimeFieldOpts } from '../../src/ui/relative-time-field.js'; const qs = (root: ParentNode, selector: string): T => root.querySelector(selector) as T; @@ -40,6 +46,47 @@ describe('RELATIVE_TIME_PRESETS / filterPresets', () => { }); }); +// #335: TIME_RANGE_CONSTANTS is the popover's per-field constants column — +// pinned exact 14-entry order from the contract, distinct from (and untouched +// by) RELATIVE_TIME_PRESETS above. +describe('TIME_RANGE_CONSTANTS', () => { + it('exports exactly the pinned 14 entries, in order', () => { + expect(TIME_RANGE_CONSTANTS.map((c) => c.value)).toEqual([ + 'now', '-5m', '-15m', '-30m', '-1h', '-3h', '-6h', '-12h', + '-1d', '-2d', '-7d', '-30d', '-1M', '-90d', + ]); + }); + it('never overlaps with RELATIVE_TIME_PRESETS being touched — that list stays bit-identical', () => { + expect(RELATIVE_TIME_PRESETS.map((p) => p.value)).toEqual([ + '-15m', '-1h', '-6h', '-1d', '-7d', '-1M', 'now/d', '-1d/d', 'now', + ]); + }); +}); + +// #335: filterTokenList generalizes filterPresets' matching over any +// {value,label} list; filterPresets itself becomes a one-line wrapper with +// unchanged behavior (already covered above). +describe('filterTokenList', () => { + it('a blank/whitespace/undefined query returns the SAME list reference (no copy)', () => { + expect(filterTokenList(TIME_RANGE_CONSTANTS, '')).toBe(TIME_RANGE_CONSTANTS); + expect(filterTokenList(TIME_RANGE_CONSTANTS, ' ')).toBe(TIME_RANGE_CONSTANTS); + expect(filterTokenList(TIME_RANGE_CONSTANTS, undefined)).toBe(TIME_RANGE_CONSTANTS); + }); + it('filters case-insensitively by value substring', () => { + expect(filterTokenList(TIME_RANGE_CONSTANTS, '-1').map((c) => c.value)).toEqual(['-15m', '-1h', '-12h', '-1d', '-1M']); + }); + it('filters by label substring too', () => { + expect(filterTokenList(TIME_RANGE_CONSTANTS, 'current')).toEqual([{ value: 'now', label: 'now — current time' }]); + }); + it('no match returns an empty list', () => { + expect(filterTokenList(TIME_RANGE_CONSTANTS, 'zzz-nope')).toEqual([]); + }); + it('works over an arbitrary {value,label} list, not just the module\'s own constants', () => { + const custom = [{ value: 'a', label: 'Alpha' }, { value: 'b', label: 'Beta' }]; + expect(filterTokenList(custom, 'bet')).toEqual([{ value: 'b', label: 'Beta' }]); + }); +}); + describe('buildRelativeTimeField — DOM shape', () => { it('builds an accessible combobox input with the expected ARIA wiring', () => { const { field } = build(); diff --git a/tests/unit/relative-time.test.ts b/tests/unit/relative-time.test.ts index c8de16d8..edc125f0 100644 --- a/tests/unit/relative-time.test.ts +++ b/tests/unit/relative-time.test.ts @@ -30,6 +30,7 @@ import { formatPreview, isDateLikeType, resolveVarValues, + parseAbsoluteInstant, } from '../../src/core/relative-time.js'; import type { ParsedRelativeExpr } from '../../src/core/relative-time.js'; import type { ParsedParamType } from '../../src/core/param-type.js'; @@ -360,3 +361,110 @@ describe('formatPreview — human-readable UTC/server-time preview (review findi expect(formatPreview('now', 'Nullable(DateTime)', now).display).toBe('2026-07-11 13:23:45'); }); }); + +// #335: the absolute-value complement of the relative grammar above — the +// time-range control's From/To fields accept both. UTC convention throughout +// (never the runtime's local `TZ`), matching `formatPreview`'s own preview +// convention so a previously-rendered preview round-trips back through this +// parser unchanged. +describe('parseAbsoluteInstant — Date/Date32 (date-only)', () => { + it('accepts a bare YYYY-MM-DD, resolved at UTC midnight', () => { + expect(parseAbsoluteInstant('Date', '2026-07-11')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 0, 0, 0, 0) }); + expect(parseAbsoluteInstant('Date32', '2026-07-11')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11) }); + }); + it('trims surrounding whitespace', () => { + expect(parseAbsoluteInstant('Date', ' 2026-07-11 ')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11) }); + }); + it('rejects a time part present on a date-only type', () => { + const r = parseAbsoluteInstant('Date', '2026-07-11 09:00:00'); + expect(r.ok).toBe(false); + expect((r as { error: string }).error).toMatch(/expected YYYY-MM-DD/); + }); + it('rejects an invalid calendar date (Feb 30 does not exist)', () => { + const r = parseAbsoluteInstant('Date', '2026-02-30'); + expect(r.ok).toBe(false); + expect((r as { error: string }).error).toMatch(/not a valid calendar date/); + }); + it('rejects day 0 and a day beyond the month\'s length', () => { + expect(parseAbsoluteInstant('Date', '2026-07-00').ok).toBe(false); + expect(parseAbsoluteInstant('Date', '2026-04-31').ok).toBe(false); // April has 30 days + }); + it('rejects an out-of-range month', () => { + expect(parseAbsoluteInstant('Date', '2026-13-01').ok).toBe(false); + expect(parseAbsoluteInstant('Date', '2026-00-01').ok).toBe(false); + }); + it('a leap-year Feb 29 is valid; the following (non-leap) year is not', () => { + expect(parseAbsoluteInstant('Date', '2028-02-29').ok).toBe(true); + expect(parseAbsoluteInstant('Date', '2029-02-29').ok).toBe(false); + }); + it('rejects years below 1900 — no ClickHouse date type reaches lower, and Date.UTC would silently remap 0–99 to 1900–1999', () => { + expect(parseAbsoluteInstant('Date', '0050-07-11').ok).toBe(false); + expect(parseAbsoluteInstant('DateTime', '1899-12-31 23:59:59').ok).toBe(false); + expect(parseAbsoluteInstant('Date32', '1900-01-01')).toEqual({ ok: true, instantMs: Date.UTC(1900, 0, 1) }); + }); + it('garbage text is rejected with a diagnostic naming the expected format', () => { + const r = parseAbsoluteInstant('Date', 'not-a-date'); + expect(r.ok).toBe(false); + expect((r as { error: string }).error).toMatch(/expected YYYY-MM-DD/); + }); +}); + +describe('parseAbsoluteInstant — DateTime/DateTime64 (date + time)', () => { + it('accepts a bare date, defaulting the time to UTC midnight', () => { + expect(parseAbsoluteInstant('DateTime', '2026-07-11')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11) }); + }); + it('accepts YYYY-MM-DD HH:MM (no seconds)', () => { + expect(parseAbsoluteInstant('DateTime', '2026-07-11 09:23')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 0, 0) }); + }); + it('accepts YYYY-MM-DD HH:MM:SS', () => { + expect(parseAbsoluteInstant('DateTime', '2026-07-11 09:23:45')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 45, 0) }); + }); + it('accepts the "T" separator variant of each form', () => { + expect(parseAbsoluteInstant('DateTime', '2026-07-11T09:23')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 0, 0) }); + expect(parseAbsoluteInstant('DateTime', '2026-07-11T09:23:45')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 45, 0) }); + }); + it('DateTime64: fractional seconds, 1 to 9 digits, padded/truncated to ms resolution', () => { + expect(parseAbsoluteInstant('DateTime64(3)', '2026-07-11 09:23:45.1')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 45, 100) }); + expect(parseAbsoluteInstant('DateTime64(3)', '2026-07-11 09:23:45.123')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 45, 123) }); + expect(parseAbsoluteInstant('DateTime64(9)', '2026-07-11 09:23:45.123456789')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 45, 123) }); + expect(parseAbsoluteInstant('DateTime64(3)', '2026-07-11T09:23:45.123')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 23, 45, 123) }); + }); + it('rejects an invalid calendar date carried in a datetime form', () => { + const r = parseAbsoluteInstant('DateTime', '2026-02-30 09:00:00'); + expect(r.ok).toBe(false); + expect((r as { error: string }).error).toMatch(/not a valid calendar date/); + }); + it('rejects an out-of-range hour, minute, or second', () => { + expect(parseAbsoluteInstant('DateTime', '2026-07-11 24:00:00').ok).toBe(false); + expect(parseAbsoluteInstant('DateTime', '2026-07-11 09:60:00').ok).toBe(false); + expect(parseAbsoluteInstant('DateTime', '2026-07-11 09:00:60').ok).toBe(false); + }); + it('accepts a valid boundary hour/minute/second (23:59:59)', () => { + expect(parseAbsoluteInstant('DateTime', '2026-07-11 23:59:59')).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 23, 59, 59, 0) }); + }); + + describe('bare-digit epoch forms', () => { + it('1-10 digits resolve as epoch SECONDS', () => { + expect(parseAbsoluteInstant('DateTime', '1')).toEqual({ ok: true, instantMs: 1000 }); + expect(parseAbsoluteInstant('DateTime', '1783772625')).toEqual({ ok: true, instantMs: 1783772625000 }); + }); + it('exactly 13 digits resolves as epoch MILLISECONDS', () => { + expect(parseAbsoluteInstant('DateTime64(3)', '1783772625123')).toEqual({ ok: true, instantMs: 1783772625123 }); + }); + it('11 or 12 digits is neither recognized length and is rejected', () => { + expect(parseAbsoluteInstant('DateTime', '17837726251').ok).toBe(false); + expect(parseAbsoluteInstant('DateTime', '178377262512').ok).toBe(false); + }); + }); + + it('garbage text is rejected with a diagnostic naming the type', () => { + const r = parseAbsoluteInstant('DateTime', 'not-a-timestamp-at-all'); + expect(r.ok).toBe(false); + expect((r as { error: string }).error).toBeTruthy(); + }); + + it('accepts a ParsedParamType object directly, not just a raw string', () => { + const r = parseAbsoluteInstant({ base: 'DateTime' } as ParsedParamType, '2026-07-11 09:00:00'); + expect(r).toEqual({ ok: true, instantMs: Date.UTC(2026, 6, 11, 9, 0, 0, 0) }); + }); +}); diff --git a/tests/unit/time-range-field.test.ts b/tests/unit/time-range-field.test.ts new file mode 100644 index 00000000..eb47d423 --- /dev/null +++ b/tests/unit/time-range-field.test.ts @@ -0,0 +1,473 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { buildTimeRangeField } from '../../src/ui/time-range-field.js'; +import type { TimeRangeFieldOpts } from '../../src/ui/time-range-field.js'; +import type { DashboardTimeRangeGroup, TimeRangeRecent } from '../../src/core/time-range.js'; +import { validateTimeRangeDraft } from '../../src/core/time-range.js'; +import { TIME_RANGE_CONSTANTS } from '../../src/ui/relative-time-field.js'; +import { parseParamType } from '../../src/core/param-type.js'; + +afterEach(() => document.body.replaceChildren()); + +// A fixed wall-clock instant (UTC noon-ish) so resolved previews are stable; +// the test suite runs under TZ=America/New_York (see the memory note) so +// relative calendar offsets resolve in that zone, but every assertion routes +// the expected display through the same `validateTimeRangeDraft` the control +// uses, so no assertion hard-codes a formatted string. +const NOW = Date.UTC(2026, 6, 21, 12, 35, 45); +const DAY = 86400000; + +const DT = parseParamType('DateTime'); + +function grp(overrides: Partial = {}): DashboardTimeRangeGroup { + return { + key: 'f1\u0000f2', fromFilterId: 'f1', toFilterId: 'f2', + fromParameter: 'from', toParameter: 'to', + fromType: DT, toType: DT, + ...overrides, + }; +} + +function baseOpts(overrides: Partial = {}): TimeRangeFieldOpts { + return { + group: grp(), + fromValue: '-1d', toValue: 'now', active: true, + waveNowMs: NOW, + wallNow: () => NOW, + getRecents: () => [], + onApply: vi.fn(), + ...overrides, + }; +} + +const expectDraft = (fromText: string, toText: string, nowMs = NOW) => + validateTimeRangeDraft({ fromText, toText, fromType: DT, toType: DT, nowMs }); + +const click = (el: Element): boolean => + el.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); +const key = (target: EventTarget, k: string): boolean => + target.dispatchEvent(new KeyboardEvent('keydown', { key: k, bubbles: true, cancelable: true })); +const type = (input: HTMLInputElement, text: string): void => { + input.value = text; + input.dispatchEvent(new Event('input', { bubbles: true })); +}; +const focus = (input: HTMLElement): void => { input.dispatchEvent(new Event('focus')); }; + +const trigger = (el: HTMLElement): HTMLButtonElement => el.querySelector('.trf-trigger') as HTMLButtonElement; +const popover = (): HTMLElement | null => document.body.querySelector('.trf-popover'); +const fromInput = (): HTMLInputElement => document.body.querySelector('input[aria-label="From"]') as HTMLInputElement; +const toInput = (): HTMLInputElement => document.body.querySelector('input[aria-label="To"]') as HTMLInputElement; +const fromCaret = (): HTMLButtonElement => + document.body.querySelector('button[aria-label="Show constants for From"]') as HTMLButtonElement; +const toCaret = (): HTMLButtonElement => + document.body.querySelector('button[aria-label="Show constants for To"]') as HTMLButtonElement; +const rightHeader = (): HTMLElement => document.body.querySelector('.trf-right-header') as HTMLElement; +const previews = (): HTMLElement[] => [...document.body.querySelectorAll('.trf-preview')] as HTMLElement[]; +const constBtns = (): HTMLElement[] => [...document.body.querySelectorAll('.trf-const')] as HTMLElement[]; +const recentBtns = (): HTMLElement[] => [...document.body.querySelectorAll('.trf-recent')] as HTMLElement[]; +const emptyEl = (): HTMLElement | null => document.body.querySelector('.trf-empty'); +const applyBtn = (): HTMLButtonElement => document.body.querySelector('.trf-btn-primary') as HTMLButtonElement; +const cancelBtn = (): HTMLButtonElement => + document.body.querySelector('.trf-btn:not(.trf-btn-primary)') as HTMLButtonElement; +const rangeErr = (): HTMLElement => document.body.querySelector('.trf-range-error') as HTMLElement; + +const open = (handle: { el: HTMLElement }): void => { click(trigger(handle.el)); }; + +describe('buildTimeRangeField — closed trigger states', () => { + it('is a #364 dialog-pattern button', () => { + const handle = buildTimeRangeField(baseOpts()); + const t = trigger(handle.el); + expect(handle.el.className).toBe('var-field is-time-range'); + expect(t.getAttribute('aria-haspopup')).toBe('dialog'); + expect(t.getAttribute('aria-expanded')).toBe('false'); + expect(t.classList.contains('var-input')).toBe(true); + }); + + it('active + both bounds resolve: label is the resolved range and aria carries tokens AND resolved range', () => { + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now', active: true })); + const t = trigger(handle.el); + const res = expectDraft('-1d', 'now'); + expect(t.textContent).toBe(`${res.from.display} → ${res.to.display}`); + expect(t.classList.contains('is-error')).toBe(false); + expect(t.getAttribute('aria-label')).toBe( + `Time range from -1d to now, resolved ${res.from.display} to ${res.to.display}`); + }); + + it('inactive → neutral "Not set"', () => { + const handle = buildTimeRangeField(baseOpts({ active: false })); + const t = trigger(handle.el); + expect(t.textContent).toBe('Not set'); + expect(t.getAttribute('aria-label')).toBe('Time range, not set'); + expect(t.classList.contains('is-error')).toBe(false); + }); + + it('active but a committed bound fails to parse → raw text + error class, aria says not resolvable', () => { + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1x', toValue: 'now', active: true })); + const t = trigger(handle.el); + const res = expectDraft('-1x', 'now'); + expect(res.from.ok).toBe(false); // sanity: the near-miss really is unresolvable + expect(t.textContent).toBe(`-1x → ${res.to.display}`); // raw for the broken bound, resolved for the good one + expect(t.classList.contains('is-error')).toBe(true); + expect(t.getAttribute('aria-label')).toBe('Time range from -1x to now, not resolvable'); + }); + + it('a failing TO bound also shows raw text for that bound with the error class', () => { + const handle = buildTimeRangeField(baseOpts({ fromValue: 'now', toValue: '-1x', active: true })); + const t = trigger(handle.el); + const res = expectDraft('now', '-1x'); + expect(res.to.ok).toBe(false); + expect(t.textContent).toBe(`${res.from.display} → -1x`); + expect(t.classList.contains('is-error')).toBe(true); + }); + + it('waveNowMs null falls back to wallNow() for the initial label', () => { + const handle = buildTimeRangeField(baseOpts({ waveNowMs: null, wallNow: () => NOW, fromValue: 'now', toValue: 'now' })); + const res = expectDraft('now', 'now'); + expect(trigger(handle.el).textContent).toBe(`${res.from.display} → ${res.to.display}`); + }); +}); + +describe('buildTimeRangeField — refreshLabel re-resolves in place', () => { + it('recomputes the trigger label against a new nowMs without rebuilding', () => { + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now', active: true })); + const t = trigger(handle.el); + const before = t.textContent; + handle.refreshLabel(NOW + DAY); + const res = expectDraft('-1d', 'now', NOW + DAY); + expect(t.textContent).toBe(`${res.from.display} → ${res.to.display}`); + expect(t.textContent).not.toBe(before); // a day later resolves to a different instant + expect(trigger(handle.el)).toBe(t); // same node — no rebuild + }); +}); + +describe('buildTimeRangeField — popover open/close/focus-return', () => { + it('opens under the trigger with aria-expanded, and Cancel closes + returns focus', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + const t = trigger(handle.el); + open(handle); + expect(popover()).not.toBeNull(); + expect(t.getAttribute('aria-expanded')).toBe('true'); + expect(handle.isOpen()).toBe(true); + expect(popover()!.getAttribute('role')).toBe('dialog'); + expect(popover()!.getAttribute('aria-label')).toBe('Time range'); + click(cancelBtn()); + expect(popover()).toBeNull(); + expect(t.getAttribute('aria-expanded')).toBe('false'); + expect(handle.isOpen()).toBe(false); + expect(document.activeElement).toBe(t); + }); + + it('Escape closes the popover', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + key(popover()!, 'Escape'); + expect(handle.isOpen()).toBe(false); + }); + + it('clicking the trigger while already open does not stack a second popover', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + const t = trigger(handle.el); + click(t); + click(t); + expect(document.body.querySelectorAll('.trf-popover').length).toBe(1); + }); + + it('opens with staged inputs seeded from the committed values and both preview lines resolved', () => { + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now' })); + document.body.appendChild(handle.el); + open(handle); + expect(fromInput().value).toBe('-1d'); + expect(toInput().value).toBe('now'); + const res = expectDraft('-1d', 'now'); + expect(previews()[0].textContent).toBe('= ' + res.from.display); + expect(previews()[1].textContent).toBe('= ' + res.to.display); + expect(fromInput().getAttribute('aria-describedby')).toBe(previews()[0].id); + }); +}); + +describe('buildTimeRangeField — right column: recents (no field active)', () => { + it('opens showing "Recently used" with one button per recent (raw tokens)', () => { + const recents: TimeRangeRecent[] = [{ from: '-1d', to: 'now' }, { from: '2026-07-01', to: 'now' }]; + const handle = buildTimeRangeField(baseOpts({ getRecents: () => recents })); + document.body.appendChild(handle.el); + open(handle); + expect(rightHeader().textContent).toBe('Recently used'); + expect(recentBtns().map((b) => b.textContent)).toEqual(['-1d → now', '2026-07-01 → now']); + }); + + it('empty recents → "No recent ranges yet"', () => { + const handle = buildTimeRangeField(baseOpts({ getRecents: () => [] })); + document.body.appendChild(handle.el); + open(handle); + expect(emptyEl()!.textContent).toBe('No recent ranges yet'); + }); + + it('clicking a recent closes the popover FIRST, then applies its raw pair', () => { + const recents: TimeRangeRecent[] = [{ from: '-2d', to: '-1h' }]; + let openWhenCalled: boolean | null = null; + const onApply = vi.fn(() => { openWhenCalled = handle.isOpen(); }); + const handle = buildTimeRangeField(baseOpts({ getRecents: () => recents, onApply })); + document.body.appendChild(handle.el); + open(handle); + click(recentBtns()[0]); + expect(onApply).toHaveBeenCalledWith('-2d', '-1h'); + expect(openWhenCalled).toBe(false); // close ran before onApply + expect(popover()).toBeNull(); + }); +}); + +describe('buildTimeRangeField — right column: constants (a field active)', () => { + it('a caret toggles the field constants column on and off (recents when off)', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + expect(rightHeader().textContent).toBe('Recently used'); // resting state despite From being focused on open + click(fromCaret()); + expect(fromCaret().getAttribute('aria-pressed')).toBe('true'); + expect(rightHeader().textContent).toBe('From · constants'); + expect(constBtns().length).toBe(TIME_RANGE_CONSTANTS.length); + click(fromCaret()); // toggle off → back to recents + expect(fromCaret().getAttribute('aria-pressed')).toBe('false'); + expect(rightHeader().textContent).toBe('Recently used'); + }); + + it('switches between From and To constants per caret', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + click(fromCaret()); + expect(rightHeader().textContent).toBe('From · constants'); + click(toCaret()); + expect(rightHeader().textContent).toBe('To · constants'); + expect(toCaret().getAttribute('aria-pressed')).toBe('true'); + expect(fromCaret().getAttribute('aria-pressed')).toBe('false'); + }); + + it('focusing a field (a genuine, post-open focus) activates its constants column', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + focus(toInput()); + expect(rightHeader().textContent).toBe('To · constants'); + expect(toCaret().getAttribute('aria-pressed')).toBe('true'); + }); + + it('typing in a field filters the constants via filterTokenList', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), '-1h'); + expect(rightHeader().textContent).toBe('From · constants'); + const tokens = constBtns().map((b) => b.querySelector('.trf-const-token')!.textContent); + expect(tokens).toEqual(['-1h']); + }); + + it('no matches shows the absolute-datetimes hint', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), 'zzzz'); + expect(constBtns().length).toBe(0); + expect(emptyEl()!.textContent).toContain('absolute datetimes'); + }); + + it('clicking a constant fills that field (staged), keeps the popover open, resets the filter, no apply', () => { + const onApply = vi.fn(); + const handle = buildTimeRangeField(baseOpts({ onApply })); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), '-1h'); // filter down to one + click(constBtns()[0]); + expect(fromInput().value).toBe('-1h'); + expect(onApply).not.toHaveBeenCalled(); + expect(handle.isOpen()).toBe(true); + expect(constBtns().length).toBe(TIME_RANGE_CONSTANTS.length); // filter reset after fill + }); + + it('a constant fills the To field when To is active', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + click(toCaret()); + const nowConst = constBtns().find((b) => b.querySelector('.trf-const-token')!.textContent === 'now')!; + click(nowConst); + expect(toInput().value).toBe('now'); + }); +}); + +describe('buildTimeRangeField — Apply / Cancel semantics', () => { + it('staged editing does not commit (onApply only fires on Apply)', () => { + const onApply = vi.fn(); + const handle = buildTimeRangeField(baseOpts({ onApply })); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), '-3d'); + expect(onApply).not.toHaveBeenCalled(); + }); + + it('Cancel discards the draft: no onApply, popover closed', () => { + const onApply = vi.fn(); + const handle = buildTimeRangeField(baseOpts({ onApply })); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), '-3d'); + click(cancelBtn()); + expect(onApply).not.toHaveBeenCalled(); + expect(handle.isOpen()).toBe(false); + }); + + it('Apply is disabled while a bound is unresolvable', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), '-1x'); // near-miss, unresolvable + expect(applyBtn().disabled).toBe(true); + expect(previews()[0].classList.contains('is-error')).toBe(true); + }); + + it('Apply is disabled and the range error shows when from > to; equal instants are permitted', () => { + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now' })); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), 'now'); + type(toInput(), '-1d'); // now > -1d → from after to + expect(applyBtn().disabled).toBe(true); + expect(rangeErr().hidden).toBe(false); + expect(rangeErr().textContent).toBe(expectDraft('now', '-1d').rangeError); + type(toInput(), 'now'); // equal instants → permitted + expect(applyBtn().disabled).toBe(false); + expect(rangeErr().hidden).toBe(true); + }); + + it('Apply with a draft identical (after trim) to the committed pair closes without onApply', () => { + const onApply = vi.fn(); + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now', onApply })); + document.body.appendChild(handle.el); + open(handle); + expect(applyBtn().disabled).toBe(false); + click(applyBtn()); + expect(onApply).not.toHaveBeenCalled(); + expect(handle.isOpen()).toBe(false); + }); + + it('Apply with unchanged valid values on an INACTIVE pair still commits — activation is the change', () => { + // clearFilter keeps the typed value and only flips `active` off, so a + // committed-but-inactive pair seeds the popover with valid text; Apply is + // the only activation path for a grouped pair and must not no-op here. + const onApply = vi.fn(); + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now', active: false, onApply })); + document.body.appendChild(handle.el); + open(handle); + expect(applyBtn().disabled).toBe(false); + click(applyBtn()); + expect(onApply).toHaveBeenCalledWith('-1d', 'now'); + expect(handle.isOpen()).toBe(false); + }); + + it('Apply commits the TRIMMED drafts, closing BEFORE onApply', () => { + let openWhenCalled: boolean | null = null; + const onApply = vi.fn(() => { openWhenCalled = handle.isOpen(); }); + const handle = buildTimeRangeField(baseOpts({ fromValue: '-1d', toValue: 'now', onApply })); + document.body.appendChild(handle.el); + open(handle); + type(fromInput(), ' -2d '); // padded — must be trimmed on commit + click(applyBtn()); + expect(onApply).toHaveBeenCalledWith('-2d', 'now'); + expect(openWhenCalled).toBe(false); + expect(popover()).toBeNull(); + }); +}); + +describe('buildTimeRangeField — handle surface', () => { + it('updateStatus is a no-op that does not throw or change the trigger', () => { + const handle = buildTimeRangeField(baseOpts()); + const before = trigger(handle.el).textContent; + expect(() => handle.updateStatus({ status: 'anything' })).not.toThrow(); + expect(trigger(handle.el).textContent).toBe(before); + }); + + it('focusTrigger focuses the trigger button', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + handle.focusTrigger(); + expect(document.activeElement).toBe(trigger(handle.el)); + }); + + it('isOpen reflects the popover lifecycle', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + expect(handle.isOpen()).toBe(false); + open(handle); + expect(handle.isOpen()).toBe(true); + click(cancelBtn()); + expect(handle.isOpen()).toBe(false); + }); + + it('dispose while open is a Cancel: no onApply, popover removed, trigger listener detached', () => { + const onApply = vi.fn(); + const handle = buildTimeRangeField(baseOpts({ onApply })); + document.body.appendChild(handle.el); + const t = trigger(handle.el); + open(handle); + type(fromInput(), '-9d'); + handle.dispose(); + expect(onApply).not.toHaveBeenCalled(); + expect(handle.isOpen()).toBe(false); + expect(popover()).toBeNull(); + click(t); // listener removed — must not reopen + expect(popover()).toBeNull(); + }); + + it('dispose while closed does not throw and still detaches the trigger listener', () => { + const handle = buildTimeRangeField(baseOpts()); + document.body.appendChild(handle.el); + const t = trigger(handle.el); + expect(() => handle.dispose()).not.toThrow(); + click(t); + expect(popover()).toBeNull(); + }); +}); + +describe('review-round fixes', () => { + const liveEl = (): HTMLElement => + document.body.querySelector('.trf-popover .sr-only[aria-live="polite"]') as HTMLElement; + + it('a constant pick returns focus to the field input (the click detaches the picked button)', () => { + const handle = buildTimeRangeField(baseOpts()); + open(handle); + focus(fromInput()); // synthetic activation: show the From constants column + const btn = constBtns()[0]; + btn.focus(); // real focus — the clicked button truly holds focus + expect(document.activeElement).toBe(btn); + click(btn); + expect(document.activeElement).toBe(fromInput()); + }); + + it('announces validation failures politely — per field, range errors deduped, cleared on valid', () => { + const handle = buildTimeRangeField(baseOpts()); + open(handle); + expect(liveEl().textContent).toBe(''); // valid committed seed announces nothing + + type(fromInput(), 'banana'); + expect(liveEl().textContent).toBe(`From: ${expectDraft('banana', 'now').from.error}`); + + type(fromInput(), '-1d'); + type(toInput(), 'garbage'); + expect(liveEl().textContent).toBe(`To: ${expectDraft('-1d', 'garbage').to.error}`); + + // Range error (both bounds resolve, from > to) announces the range text… + type(fromInput(), 'now'); + type(toInput(), '-1d'); + const rangeText = expectDraft('now', '-1d').rangeError as string; + expect(liveEl().textContent).toBe(rangeText); + // …and an edit producing the SAME failure text does not rewrite the region + // (rewriting identical text would re-announce it every keystroke). + type(toInput(), '-2d'); + expect(liveEl().textContent).toBe(rangeText); + + type(toInput(), 'now'); + expect(liveEl().textContent).toBe(''); + }); +}); diff --git a/tests/unit/time-range.test.ts b/tests/unit/time-range.test.ts new file mode 100644 index 00000000..3591ccf5 --- /dev/null +++ b/tests/unit/time-range.test.ts @@ -0,0 +1,348 @@ +import { describe, it, expect } from 'vitest'; +import { analyzeParameterizedSources } from '../../src/core/param-pipeline.js'; +import type { ParameterAnalysis } from '../../src/core/param-pipeline.js'; +import type { FilterSelectionFilterDef } from '../../src/core/filter-selection.js'; +import { + inferTimeRangePairs, + resolveTimeRangeGroups, + validateTimeRangeDraft, + pushRecentRange, +} from '../../src/core/time-range.js'; +import type { TimeRangeRecent } from '../../src/core/time-range.js'; + +// Same fixture convention as tests/unit/filter-selection.test.ts: round-trip +// through the real `analyzeParameterizedSources` rather than a hand-crafted +// `ParameterAnalysis`. +const analysisFor = (sources: { id: string; sql: string }[]): ParameterAnalysis => + analyzeParameterizedSources(sources.map((s) => ({ id: s.id, kind: 'tab', sql: s.sql, bindPolicy: 'row-returning' }))); + +type TRFilterDef = FilterSelectionFilterDef & { sourceQueryId?: string | null }; + +describe('inferTimeRangePairs', () => { + it('recognizes from/to (case-insensitive)', () => { + const pairs = inferTimeRangePairs([{ id: 'f1', parameter: 'From' }, { id: 'f2', parameter: 'TO' }]); + expect(pairs).toEqual([{ fromFilterId: 'f1', toFilterId: 'f2' }]); + }); + it('recognizes from_time/to_time, start/end, start_time/end_time', () => { + expect(inferTimeRangePairs([{ id: 'a', parameter: 'from_time' }, { id: 'b', parameter: 'to_time' }])) + .toEqual([{ fromFilterId: 'a', toFilterId: 'b' }]); + expect(inferTimeRangePairs([{ id: 'a', parameter: 'start' }, { id: 'b', parameter: 'end' }])) + .toEqual([{ fromFilterId: 'a', toFilterId: 'b' }]); + expect(inferTimeRangePairs([{ id: 'a', parameter: 'start_time' }, { id: 'b', parameter: 'end_time' }])) + .toEqual([{ fromFilterId: 'a', toFilterId: 'b' }]); + }); + it('never recognizes start/stop', () => { + expect(inferTimeRangePairs([{ id: 'a', parameter: 'start' }, { id: 'b', parameter: 'stop' }])).toEqual([]); + }); + it('a filter with a non-null sourceQueryId (curated) is never a candidate', () => { + const pairs = inferTimeRangePairs([ + { id: 'a', parameter: 'from', sourceQueryId: 'q1' }, + { id: 'b', parameter: 'to' }, + ]); + expect(pairs).toEqual([]); + }); + it('a null sourceQueryId is NOT curated — still eligible', () => { + const pairs = inferTimeRangePairs([ + { id: 'a', parameter: 'from', sourceQueryId: null }, + { id: 'b', parameter: 'to' }, + ]); + expect(pairs).toEqual([{ fromFilterId: 'a', toFilterId: 'b' }]); + }); + it('a parameter name borne by more than one filter def is unusable — no pair forms at all', () => { + const pairs = inferTimeRangePairs([ + { id: 'f1', parameter: 'from' }, + { id: 'f2', parameter: 'from' }, + { id: 'f3', parameter: 'to' }, + ]); + expect(pairs).toEqual([]); + }); + it('multiple independent groups: rows are emitted in NAME_PAIR_TABLE order regardless of input array order', () => { + const pairs = inferTimeRangePairs([ + { id: 'se-start', parameter: 'start' }, + { id: 'se-end', parameter: 'end' }, + { id: 'ft-to', parameter: 'to' }, + { id: 'ft-from', parameter: 'from' }, + ]); + expect(pairs).toEqual([ + { fromFilterId: 'ft-from', toFilterId: 'ft-to' }, + { fromFilterId: 'se-start', toFilterId: 'se-end' }, + ]); + }); + it('ambiguity: a filter id used across two would-be pairs drops BOTH pairs (defensive general rule)', () => { + // Contrived (a filter def with a reused/duplicated id is a data-integrity + // bug upstream), but exercises the general "at most one emitted pair per + // filter id" rule directly: 'shared' appears once as a `from`-role match + // and once as an `end`-role match, so both candidate pairs involving it + // are dropped. + const pairs = inferTimeRangePairs([ + { id: 'shared', parameter: 'from' }, + { id: 'other1', parameter: 'to' }, + { id: 'other2', parameter: 'start' }, + { id: 'shared', parameter: 'end' }, + ]); + expect(pairs).toEqual([]); + }); + it('no filters at all → no pairs', () => { + expect(inferTimeRangePairs([])).toEqual([]); + }); +}); + +describe('resolveTimeRangeGroups — contract gating', () => { + it('both bounds scalar + date-like across their executable consumers → one group', () => { + const filters: TRFilterDef[] = [ + { id: 'f-from', parameter: 'from' }, + { id: 'f-to', parameter: 'to' }, + ]; + const analysis = analysisFor([ + { id: 'a', sql: 'SELECT * FROM t WHERE ts >= {from:DateTime} AND ts < {to:DateTime}' }, + ]); + const groups = resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) }); + expect(groups).toHaveLength(1); + expect(groups[0]).toMatchObject({ + key: 'f-from\u0000f-to', + fromFilterId: 'f-from', + toFilterId: 'f-to', + fromParameter: 'from', + toParameter: 'to', + }); + expect(groups[0].fromType.base).toBe('DateTime'); + expect(groups[0].toType.base).toBe('DateTime'); + }); + + it('a non-date-like consumer type → no group', () => { + const filters: TRFilterDef[] = [{ id: 'f-from', parameter: 'from' }, { id: 'f-to', parameter: 'to' }]; + const analysis = analysisFor([{ id: 'a', sql: 'SELECT * FROM t WHERE x = {from:String} AND y = {to:DateTime}' }]); + expect(resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) })).toEqual([]); + }); + + it('an Array(...) contract (arity: multiple) → no group', () => { + const filters: TRFilterDef[] = [{ id: 'f-from', parameter: 'from' }, { id: 'f-to', parameter: 'to' }]; + const analysis = analysisFor([ + { id: 'a', sql: 'SELECT * FROM t WHERE ts IN {from:Array(DateTime)} AND ts2 = {to:DateTime}' }, + ]); + expect(resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) })).toEqual([]); + }); + + it('any resolution diagnostics (e.g. conflicting consumer types) → no group', () => { + const filters: TRFilterDef[] = [{ id: 'f-from', parameter: 'from' }, { id: 'f-to', parameter: 'to' }]; + const analysis = analysisFor([ + { id: 'a', sql: 'SELECT * FROM t WHERE ts = {from:DateTime} AND te = {to:DateTime}' }, + { id: 'b', sql: 'SELECT * FROM u WHERE ts = {from:String}' }, + ]); + // Both 'a' and 'b' are executable, so `from`'s consumers conflict + // (DateTime vs String) — resolveFilterSelection surfaces a diagnostic, + // and the group must not form even though `to` alone would qualify. + expect(resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a', 'b']) })).toEqual([]); + }); + + it('a curated (sourceQueryId-backed) filter never becomes a candidate pair, so no group forms', () => { + const filters: TRFilterDef[] = [ + { id: 'f-from', parameter: 'from', sourceQueryId: 'saved-query-1' }, + { id: 'f-to', parameter: 'to' }, + ]; + const analysis = analysisFor([{ id: 'a', sql: 'SELECT * FROM t WHERE ts >= {from:DateTime} AND ts < {to:DateTime}' }]); + expect(resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) })).toEqual([]); + }); + + it('multiple independent groups resolve together, in pair-table order', () => { + const filters: TRFilterDef[] = [ + { id: 'se-start', parameter: 'start' }, + { id: 'se-end', parameter: 'end' }, + { id: 'ft-from', parameter: 'from' }, + { id: 'ft-to', parameter: 'to' }, + ]; + const analysis = analysisFor([ + { + id: 'a', + sql: 'SELECT * FROM t WHERE s >= {start:Date} AND s < {end:Date} AND f >= {from:DateTime} AND f < {to:DateTime}', + }, + ]); + const groups = resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) }); + expect(groups.map((g) => g.key)).toEqual(['ft-from\u0000ft-to', 'se-start\u0000se-end']); + }); + + it('key stability: recomputing over the same input yields an identical key', () => { + const filters: TRFilterDef[] = [{ id: 'f-from', parameter: 'from' }, { id: 'f-to', parameter: 'to' }]; + const analysis = analysisFor([{ id: 'a', sql: 'SELECT * FROM t WHERE ts >= {from:DateTime} AND ts < {to:DateTime}' }]); + const g1 = resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) }); + const g2 = resolveTimeRangeGroups({ filters, analysis, executableTileIds: new Set(['a']) }); + expect(g1[0].key).toBe(g2[0].key); + expect(g1[0].key).toBe('f-from\u0000f-to'); + }); + + it('an explicit `pairs` seam (e.g. a future #334 resolution) is used verbatim instead of inference', () => { + const filters: TRFilterDef[] = [ + { id: 'weird-from', parameter: 'not_from_at_all' }, + { id: 'weird-to', parameter: 'not_to_at_all' }, + ]; + const analysis = analysisFor([ + { id: 'a', sql: 'SELECT * FROM t WHERE ts >= {not_from_at_all:DateTime} AND ts < {not_to_at_all:DateTime}' }, + ]); + const groups = resolveTimeRangeGroups({ + filters, + analysis, + executableTileIds: new Set(['a']), + pairs: [{ fromFilterId: 'weird-from', toFilterId: 'weird-to' }], + }); + expect(groups).toHaveLength(1); + expect(groups[0].key).toBe('weird-from\u0000weird-to'); + }); + + it('a pair referencing a filter id absent from `filters` is skipped rather than throwing', () => { + const filters: TRFilterDef[] = [{ id: 'f-from', parameter: 'from' }, { id: 'f-to', parameter: 'to' }]; + const analysis = analysisFor([{ id: 'a', sql: 'SELECT * FROM t WHERE ts >= {from:DateTime} AND ts < {to:DateTime}' }]); + const groups = resolveTimeRangeGroups({ + filters, + analysis, + executableTileIds: new Set(['a']), + pairs: [ + { fromFilterId: 'missing-from', toFilterId: 'f-to' }, + { fromFilterId: 'f-from', toFilterId: 'missing-to' }, + ], + }); + expect(groups).toEqual([]); + }); + + it('an empty filters/pairs list resolves to no groups', () => { + const analysis = analysisFor([]); + expect(resolveTimeRangeGroups({ filters: [], analysis, executableTileIds: new Set() })).toEqual([]); + }); +}); + +describe('validateTimeRangeDraft', () => { + const NOW = Date.UTC(2026, 6, 21, 12, 0, 0, 0); // 2026-07-21 12:00:00 UTC + + it('relative token forms resolve both bounds against ONE shared nowMs', () => { + const r = validateTimeRangeDraft({ fromText: '-1h', toText: '+1h', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(r.from).toEqual({ ok: true, display: '2026-07-21 11:00:00', instantMs: NOW - 3600000, error: null, matchedRelative: true }); + expect(r.to).toEqual({ ok: true, display: '2026-07-21 13:00:00', instantMs: NOW + 3600000, error: null, matchedRelative: true }); + expect(r.rangeOk).toBe(true); + expect(r.rangeError).toBeNull(); + expect(r.applyEnabled).toBe(true); + }); + + it('absolute forms per type: Date, DateTime, DateTime64 fractional, and the "T" variant', () => { + const dateResult = validateTimeRangeDraft({ fromText: '2026-07-11', toText: '2026-07-12', fromType: 'Date', toType: 'Date', nowMs: NOW }); + expect(dateResult.from).toEqual({ ok: true, display: '2026-07-11', instantMs: Date.UTC(2026, 6, 11), error: null, matchedRelative: false }); + expect(dateResult.to.instantMs).toBe(Date.UTC(2026, 6, 12)); + + const dtResult = validateTimeRangeDraft({ + fromText: '2026-07-11 09:00:00', toText: '2026-07-11T10:00:00', + fromType: 'DateTime', toType: 'DateTime', nowMs: NOW, + }); + expect(dtResult.from.instantMs).toBe(Date.UTC(2026, 6, 11, 9, 0, 0)); + expect(dtResult.to.instantMs).toBe(Date.UTC(2026, 6, 11, 10, 0, 0)); + expect(dtResult.rangeOk).toBe(true); + + const dt64Result = validateTimeRangeDraft({ + fromText: '2026-07-11 09:00:00.123', toText: '2026-07-11 09:00:00.500', + fromType: 'DateTime64(3)', toType: 'DateTime64(3)', nowMs: NOW, + }); + expect(dt64Result.from.instantMs).toBe(Date.UTC(2026, 6, 11, 9, 0, 0, 123)); + expect(dt64Result.to.instantMs).toBe(Date.UTC(2026, 6, 11, 9, 0, 0, 500)); + }); + + it('bare epoch digits are accepted for DateTime/DateTime64', () => { + const r = validateTimeRangeDraft({ fromText: '1783772625', toText: '1783772625123', fromType: 'DateTime', toType: 'DateTime64(3)', nowMs: NOW }); + expect(r.from).toEqual({ ok: true, display: expect.any(String), instantMs: 1783772625000, error: null, matchedRelative: false }); + expect(r.to.instantMs).toBe(1783772625123); + }); + + it('invalid calendar dates are rejected', () => { + const r = validateTimeRangeDraft({ fromText: '2026-02-30', toText: 'now', fromType: 'Date', toType: 'DateTime', nowMs: NOW }); + expect(r.from.ok).toBe(false); + expect(r.from.error).toMatch(/not a valid calendar date/); + expect(r.from.display).toBeNull(); + expect(r.from.instantMs).toBeNull(); + expect(r.rangeOk).toBe(false); + expect(r.rangeError).toBeNull(); // only one bound resolved — no from>to comparison to make + expect(r.applyEnabled).toBe(false); + }); + + it('garbage text is rejected', () => { + const r = validateTimeRangeDraft({ fromText: 'garbage', toText: 'now', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(r.from.ok).toBe(false); + expect(r.from.error).toBeTruthy(); + expect(r.applyEnabled).toBe(false); + }); + + it('a near-miss relative expression surfaces the grammar error, not a silent absolute-parse attempt', () => { + const r = validateTimeRangeDraft({ fromText: 'now/q', toText: 'now', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(r.from.ok).toBe(false); + expect(r.from.error).toMatch(/Not a valid relative time expression/); + }); + + it('empty/whitespace-only text is rejected as required, before either parser runs', () => { + const empty = validateTimeRangeDraft({ fromText: '', toText: 'now', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(empty.from).toEqual({ ok: false, display: null, instantMs: null, error: 'A value is required.', matchedRelative: false }); + const whitespace = validateTimeRangeDraft({ fromText: ' ', toText: 'now', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(whitespace.from.ok).toBe(false); + expect(whitespace.from.error).toBe('A value is required.'); + }); + + it('from > to at resolved instants is rejected even when both bounds parse fine', () => { + const r = validateTimeRangeDraft({ fromText: 'now', toText: '-1h', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(r.from.ok).toBe(true); + expect(r.to.ok).toBe(true); + expect(r.rangeOk).toBe(false); + expect(r.rangeError).toMatch(/must not be after/); + expect(r.applyEnabled).toBe(false); + }); + + it('equal resolved instants are explicitly permitted', () => { + const r = validateTimeRangeDraft({ fromText: 'now', toText: 'now', fromType: 'DateTime', toType: 'DateTime', nowMs: NOW }); + expect(r.from.instantMs).toBe(r.to.instantMs); + expect(r.rangeOk).toBe(true); + expect(r.rangeError).toBeNull(); + expect(r.applyEnabled).toBe(true); + }); + + it('accepts a ParsedParamType object directly for fromType/toType, not just a raw string', () => { + const r = validateTimeRangeDraft({ + fromText: 'now', toText: 'now', + fromType: { base: 'DateTime' } as never, toType: { base: 'DateTime' } as never, + nowMs: NOW, + }); + expect(r.applyEnabled).toBe(true); + }); +}); + +describe('pushRecentRange', () => { + it('pushes onto an empty list', () => { + expect(pushRecentRange([], { from: '-1d', to: 'now' })).toEqual([{ from: '-1d', to: 'now' }]); + }); + it('unshifts newest-first ahead of existing entries', () => { + const list: TimeRangeRecent[] = [{ from: '-7d', to: 'now' }]; + expect(pushRecentRange(list, { from: '-1d', to: 'now' })).toEqual([ + { from: '-1d', to: 'now' }, + { from: '-7d', to: 'now' }, + ]); + }); + it('dedupes by EXACT token-pair equality — a repeat pair moves to the front rather than duplicating', () => { + const list: TimeRangeRecent[] = [{ from: '-1d', to: 'now' }, { from: '-7d', to: 'now' }]; + expect(pushRecentRange(list, { from: '-1d', to: 'now' })).toEqual([ + { from: '-1d', to: 'now' }, + { from: '-7d', to: 'now' }, + ]); + }); + it('a pair sharing one bound but not the other is NOT deduped (exact-pair equality only)', () => { + const list: TimeRangeRecent[] = [{ from: '-1d', to: 'now' }]; + expect(pushRecentRange(list, { from: '-1d', to: '-1h' })).toEqual([ + { from: '-1d', to: '-1h' }, + { from: '-1d', to: 'now' }, + ]); + }); + it('caps at 6 entries, dropping the oldest', () => { + const list: TimeRangeRecent[] = Array.from({ length: 6 }, (_, i) => ({ from: `-${i}d`, to: 'now' })); + const result = pushRecentRange(list, { from: '-100d', to: 'now' }); + expect(result).toHaveLength(6); + expect(result[0]).toEqual({ from: '-100d', to: 'now' }); + expect(result).not.toContainEqual({ from: '-5d', to: 'now' }); // the oldest was dropped + }); + it('is immutable — never mutates the input list', () => { + const list: TimeRangeRecent[] = [{ from: '-1d', to: 'now' }]; + const snapshot = [...list]; + pushRecentRange(list, { from: '-7d', to: 'now' }); + expect(list).toEqual(snapshot); + }); +});