diff --git a/CHANGELOG.md b/CHANGELOG.md index cdef9f62..f3c5079e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,26 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] ### Changed +- **Compact, type-aware widths for every `{name:Type}` variable/filter + input** (#345). `.var-input` no longer reserves a flat 150px for every + field regardless of its declared ClickHouse type — a new pure + `filterWidthCategory`/`filterInputWidthCh` (`core/filter-width.ts`) resolves + a stable `ch`-unit width from the type's effective base (`Nullable`/ + `LowCardinality` already unwrapped by `parseParamType`), or from `'enum'` + for a dropdown/curated field: boolean/tiny-int 9ch, numeric 13ch, `Date` + 13ch, `DateTime`/relative-time 17ch (narrower than `DateTime` even though + both share the same relative-time combobox control), enum/curated 14ch, + generic string/UUID/unknown 16ch. Applied exactly once per field build via + the shared `applyFieldWidth` (`ui/var-field.ts`, next to the existing + `applyFieldState`) from both `{name:Type}` surfaces — the Workbench + var-strip (`ui/app.ts`) and the Dashboard/detached-view/curated shared + filter bar (`ui/filter-bar.ts`) — through a `--var-input-ch` CSS custom + property, so the width never shifts while typing. The curated Dashboard + Filter field's inline clear-button padding (18px wider than a plain + field's) is compensated in its own `.filter-select .var-input` rule so its + usable text room matches a plain field at the same width category. Long + values still scroll horizontally inside the input; combobox/curated + popups are unaffected and can render wider than their input. - **Dashboard tiles move with a grip drag + live reflow, plain drags select text, and table/log cells open the shared cell-detail drawer** (#332). Native whole-card HTML5 `draggable` is removed. A tile move now starts from the diff --git a/src/core/filter-width.ts b/src/core/filter-width.ts new file mode 100644 index 00000000..436367af --- /dev/null +++ b/src/core/filter-width.ts @@ -0,0 +1,81 @@ +// Pure resolution of a compact, stable `{name:Type}` filter/variable input's +// visible width (#345): one width per field for its whole lifetime, selected +// by declared ClickHouse type + control kind, never derived from the current +// value or recomputed on keystroke. Shared by every `{name:Type}` filter +// surface — the workbench var-strip (`src/ui/app.ts`), the Dashboard/detached- +// view shared filter bar (`src/ui/filter-bar.ts`), and its curated single- +// select branch — so none of them can drift out of sync (CLAUDE.md rule 5). +// +// Keyed off the type's EFFECTIVE base (`parseParamType` already unwraps +// `Nullable`/`LowCardinality`), not the field's rendered control kind alone: +// `Date` and `DateTime` share the same relative-time combobox control +// (`isDateLikeType`, param-pipeline.js) but get different widths here, since a +// `DateTime`/relative-time literal ("-1d", "now") is materially longer than a +// bare `Date` one. + +import { parseParamType } from './param-type.js'; + +/** Bool and the single-byte integers — short enough that a couple of digits + * or "true"/"false" never need to scroll. */ +const BOOL_TINY_INT = /^(Bool|Boolean|U?Int8)$/; +/** Every other numeric base: wider integers, floats, and Decimal (with or + * without a bit-width suffix — `Decimal(10, 2)`'s base is bare `Decimal`). */ +const NUMERIC_BASE = /^(U?Int(16|32|64|128|256)|Float32|Float64|BFloat16|Decimal(32|64|128|256)?)$/; + +/** The width category a `{name:Type}` field resolves to — see the issue's + * sizing contract (#345) for the reasoning behind each band. */ +export type FilterWidthCategory = 'bool' | 'numeric' | 'date' | 'datetime' | 'enum' | 'string'; + +// One stable `ch` width per category — each inside the issue's suggested +// band (bool 8–10, numeric 11–13, date 12–14, date-time 15–18, enum 12–16, +// string/UUID/unknown 14–18). `numeric` sits at the TOP of its band (13, not +// the band's midpoint) — real-browser measurement against the app's actual +// UI font showed a negative Int32 extreme ("-2147483648") already needs every +// bit of that room; going narrower would make the single most common numeric +// `{name:Type}` declaration overflow on its own valid range, not just an +// edge case. Even at 13ch, a full-range `UInt64`/`Int128`/`Int256`/ +// `Decimal128`/`Decimal256` literal (up to 20–40 digits) still needs to +// scroll — no width inside the issue's compact-band contract can fit those +// without abandoning "materially narrower than 150px" for the whole numeric +// family, so that's the accepted tradeoff the issue's own "preserve +// horizontal scrolling for values longer than the visible width" requirement +// exists for, not a gap in this table. +const WIDTH_CH: Record = { + bool: 9, + numeric: 13, + date: 13, + datetime: 17, + enum: 14, + string: 16, +}; + +/** + * Which width category a declared `{name:Type}` field falls into. + * `isEnumLike` is the caller's own decision (an `Enum8`/`Enum16` declaration + * OR a Dashboard curated Filter-source single-select — see + * `fieldControlKind`/`buildFilterBar`'s curated branch) — it always wins over + * the type's own base, since an enum/curated dropdown's width is about the + * option labels, not the underlying scalar. Otherwise falls back to the + * generic `'string'` band for anything not recognized as boolean/numeric/ + * date/date-time (String, UUID, FixedString, Array, an unparsable + * declaration, …) — the same "opaque passthrough" fallback + * `parseParamType` itself uses. Pure. + */ +export function filterWidthCategory(type: string, isEnumLike: boolean = false): FilterWidthCategory { + if (isEnumLike) return 'enum'; + const base = parseParamType(type).base; + if (BOOL_TINY_INT.test(base)) return 'bool'; + if (NUMERIC_BASE.test(base)) return 'numeric'; + if (base === 'Date' || base === 'Date32') return 'date'; + if (base === 'DateTime' || base === 'DateTime64') return 'datetime'; + return 'string'; +} + +/** + * The stable `ch`-unit width a `{name:Type}` field's `.var-input` should use + * for its whole lifetime — `filterWidthCategory`'s resolved band, in `ch`. + * Pure; see `filterWidthCategory` for the category rules. + */ +export function filterInputWidthCh(type: string, isEnumLike: boolean = false): number { + return WIDTH_CH[filterWidthCategory(type, isEnumLike)]; +} diff --git a/src/styles.css b/src/styles.css index 4ec11832..3866a27a 100644 --- a/src/styles.css +++ b/src/styles.css @@ -774,8 +774,14 @@ body { is the same `.var-combo`/`.var-input`/`.var-combo-list` control every other combobox field uses — it only adds an inline clear (×) button for its "All"/ inactive state, so the input reserves room on the right and the button sits - over that gap (the `.var-combo` wrapper is already position:relative). */ -.filter-select .var-input { padding-right: 26px; } + over that gap (the `.var-combo` wrapper is already position:relative). The + extra 18px of padding-right (26px here vs the plain field's 8px) would + otherwise eat straight into the SAME `--var-input-ch` content width every + other field gets (#345 review finding: measured live, an 11-character + curated label left under 3px of margin at the nominal 14ch enum band) — so + this rule widens the box by that same 18px, keeping this field's actual + TEXT room equal to a plain field at the same width category. */ +.filter-select .var-input { padding-right: 26px; width: calc(var(--var-input-ch, 16) * 1ch + 18px); } .var-combo-clear-inline { position: absolute; top: 0; right: 3px; height: 24px; width: 20px; border-radius: 4px; padding: 0; @@ -1409,8 +1415,15 @@ body.detached-tab .graph-overlay-panel { glyph (the input title spells it out either way). */ .var-field.is-optional .var-name { color: var(--fg-mute); } .var-field:not(.is-optional) .var-name { font-weight: 700; } +/* Compact, type-aware width (#345): `--var-input-ch` is set once per field, + at build time, by whichever field-family builder created the input + (`applyFieldWidth`, var-field.ts — filter-width.ts resolves the ch count + from the declared type). Falls back to the generic string band (16ch) for + any input this custom property was never set on (the Filter drawer's + read-only preview cell, filter-preview.ts, which has no declared + ClickHouse type to key off). */ .var-input { - height: 24px; width: 150px; padding: 0 8px; + height: 24px; width: calc(var(--var-input-ch, 16) * 1ch); padding: 0 8px; background: var(--bg-input); color: var(--fg); border: 1px solid var(--border); border-radius: 5px; font-size: 11.5px; font-family: inherit; diff --git a/src/ui/app.ts b/src/ui/app.ts index a005ddf3..05c84868 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -46,7 +46,7 @@ import { openDetailPane } from './schema-detail.js'; import type { NodeDetail, DetailNode } from './schema-detail.js'; import { openDocEntry, openDocDisambiguation, closeDocPane, isDocPaneOpen } from './doc-pane.js'; import { renderSavedHistory } from './saved-history.js'; -import { applyFieldState } from './var-field.js'; +import { applyFieldState, applyFieldWidth } from './var-field.js'; import { buildRelativeTimeField } from './relative-time-field.js'; import type { RelativeTimeField } from './relative-time-field.js'; import { buildRecentField } from './recent-field.js'; @@ -833,6 +833,10 @@ export function createApp(env: CreateAppEnv = {}): App { else if (ctl.kind === 'date') combo = buildRelativeTimeField({ ...fieldOpts, wallNow }); else combo = buildRecentField(fieldOpts); input = combo.input; + // #345: a stable, type-appropriate width — set once per field + // build (never on keystroke), same rule the Dashboard/detached-view + // filter bar uses (filter-bar.js). + applyFieldWidth(input, v.type, ctl.kind === 'enum'); wireComboInput(combo, { onValueInput, onCommit: onCommitHard }); if (conflictNote) input.classList.add('is-conflict'); params.hardenVar(v.name, initialFields[v.name]); diff --git a/src/ui/filter-bar.ts b/src/ui/filter-bar.ts index cdb833ea..348dfeed 100644 --- a/src/ui/filter-bar.ts +++ b/src/ui/filter-bar.ts @@ -13,7 +13,7 @@ import { fieldControlKind } from '../core/param-pipeline.js'; import type { FieldControl, PreparedFieldState, ValidationMode } from '../core/param-pipeline.js'; import { recentOptions } from '../core/recent-values.js'; import type { RecentMap } from '../core/recent-values.js'; -import { applyFieldState } from './var-field.js'; +import { applyFieldState, applyFieldWidth } from './var-field.js'; import { buildRelativeTimeField as _buildRelativeTimeField, } from './relative-time-field.js'; @@ -168,6 +168,9 @@ export function buildFilterBar( onCommit: () => onCommit(p.name), }); field.input.title = baseTitle; + // #345: a curated field is always the 'enum' width band (short option + // labels) regardless of the declared param type behind it. + applyFieldWidth(field.input, p.type, true); if (conflictNote) field.input.classList.add('is-conflict'); // Same shared invalid-field affordance the plain-text branch gets below // (#170/var-field.js) — a curated field's committed value can still be @@ -235,6 +238,10 @@ export function buildFilterBar( else if (ctl.kind === 'date') combo = buildRelativeTimeField({ ...fieldOpts, wallNow: app.wallNow }); else combo = buildRecentField(fieldOpts); input = combo.input; + // #345: a stable, type-appropriate width — set once per field build + // (never on keystroke), keyed off the declared type so Date/DateTime + // (same combobox control, different widths) don't collapse to one band. + applyFieldWidth(input, p.type, ctl.kind === 'enum'); // The shared listener block (review F8): the combobox hooks first, then // D3's own persist-on-type / Enter-blur hard-commit bodies. wireComboInput(combo, { onValueInput, onCommit: onCommitHard }); diff --git a/src/ui/var-field.ts b/src/ui/var-field.ts index 4551e95e..a2cef033 100644 --- a/src/ui/var-field.ts +++ b/src/ui/var-field.ts @@ -26,6 +26,16 @@ // caller already fills it (`updatePreview` in relative-time-field.js) — it // only wires/unwires `aria-describedby` to reflect whether there's currently // something in it to describe. +// +// `applyFieldWidth` (#345) is the same-shape sibling for the field's visible +// WIDTH rather than its validity: both callers build a field's `` +// exactly once per rebuild (a fresh field, not a per-keystroke repaint — the +// signature guards in app.js/filter-bar.js's callers only rebuild when the +// detected `{name:Type}` set changes), so calling it once right after +// construction keeps a compact, stable, type-appropriate width for the +// field's whole lifetime without it ever resizing while the user types. + +import { filterInputWidthCh } from '../core/filter-width.js'; /** The validated per-param rollup shape (param-pipeline's `.fields[name]`) — * `state`'s other values ('ok'/'incomplete'/'missing'/'inactive') all render @@ -48,3 +58,13 @@ export function applyFieldState( else input.removeAttribute('aria-describedby'); } } + +/** + * Sets the compact, type-aware `--var-input-ch` custom property (#345) a + * `.var-input`'s CSS width reads (`styles.css`) — `filterInputWidthCh`'s + * resolved band for `type`/`isEnumLike`. See the header comment for why this + * is a build-time-once call, never a per-keystroke one. + */ +export function applyFieldWidth(input: HTMLElement, type: string, isEnumLike: boolean = false): void { + input.style.setProperty('--var-input-ch', String(filterInputWidthCh(type, isEnumLike))); +} diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 1df76cbc..504457c0 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -935,6 +935,13 @@ describe('query run', () => { expect(app.dom.varStrip!.style.display).toBe('none'); expect(app.dom.runBtn!.disabled).toBe(false); }); + it('query variables (#345): a field gets a compact, type-aware width — Date narrower than DateTime, Enum its own band', () => { + const { app } = appForRun([]); + app.activeTab().sqlDraft = "SELECT {n:UInt8}, {d:Date}, {dt:DateTime}, {k:Enum8('a' = 1, 'b' = 2)}"; + app.renderVarStrip(); + const inputs = qsa(app.dom.varStrip!, '.var-input'); + expect(inputs.map((i) => i.style.getPropertyValue('--var-input-ch'))).toEqual(['9', '13', '17', '14']); + }); it('query variables (#134): typing a value updates the shared store, persists, and re-enables Run', () => { vi.stubGlobal('localStorage', memStore()); const { app } = appForRun([]); diff --git a/tests/unit/filter-bar.test.ts b/tests/unit/filter-bar.test.ts index 27005709..9827afee 100644 --- a/tests/unit/filter-bar.test.ts +++ b/tests/unit/filter-bar.test.ts @@ -125,6 +125,63 @@ describe('buildFilterBar (shared filter row)', () => { expect((input as HTMLInputElement).title).toBe('Bad value'); }); + // #345: compact, type-aware field widths — one stable ch width per field, + // resolved from the declared type (or 'enum' for a dropdown/curated field). + describe('field width (#345)', () => { + it('a plain text field (String) gets the generic string width', () => { + const app = makeApp(); + const bar = buildFilterBar(app, paramsFor('SELECT {name:String}'), () => {}, okField); + const input = bar.el.querySelector('.var-input')!; + expect(input.style.getPropertyValue('--var-input-ch')).toBe('16'); + }); + it('a tiny-integer field (UInt8) gets the bool/tiny-int width', () => { + const app = makeApp(); + const bar = buildFilterBar(app, paramsFor('SELECT {flag:UInt8}'), () => {}, okField); + const input = bar.el.querySelector('.var-input')!; + expect(input.style.getPropertyValue('--var-input-ch')).toBe('9'); + }); + it('a Date field is narrower than a DateTime field, even though both render the date-like combobox', () => { + const app = makeApp(); + const dateBar = buildFilterBar(app, paramsFor('SELECT {d:Date}'), () => {}, okField); + const dtBar = buildFilterBar(app, paramsFor('SELECT {dt:DateTime}'), () => {}, okField); + expect(dateBar.el.querySelector('.var-input')!.style.getPropertyValue('--var-input-ch')).toBe('13'); + expect(dtBar.el.querySelector('.var-input')!.style.getPropertyValue('--var-input-ch')).toBe('17'); + }); + it("a declared Enum8 field gets the enum width", () => { + const app = makeApp(); + const bar = buildFilterBar(app, paramsFor("SELECT {kind:Enum8('a' = 1, 'b' = 2)}"), () => {}, okField); + const input = bar.el.querySelector('.var-input')!; + expect(input.style.getPropertyValue('--var-input-ch')).toBe('14'); + }); + it('a curated field gets the enum width regardless of its declared type', () => { + const app = makeApp(); + const bar = buildFilterBar(app, paramsFor('SELECT {x:UInt64}'), () => {}, okField, { + curatedFields: { x: { options: [{ value: 'a', label: 'Alpha' }] } }, + }); + const input = bar.el.querySelector('.var-input')!; + expect(input.style.getPropertyValue('--var-input-ch')).toBe('14'); + }); + it('a type-conflicted field still gets a width, from its first bound declaration\'s type', () => { + const app = makeApp(); + const params = fieldControls(analyzeParameterizedSources([ + { id: 'A', kind: 'tab', sql: 'SELECT {x:UInt64}', bindPolicy: 'row-returning' }, + { id: 'B', kind: 'tab', sql: 'SELECT {x:String}', bindPolicy: 'row-returning' }, + ])); + const bar = buildFilterBar(app, params, () => {}, okField); + const input = bar.el.querySelector('.var-input')!; + expect(input.style.getPropertyValue('--var-input-ch')).toBe('13'); // UInt64 (first declaration) → numeric + }); + it('never changes while typing — set once at field build, not on every keystroke', () => { + const app = makeApp(); + const bar = buildFilterBar(app, paramsFor('SELECT {name:String}'), () => {}, okField); + const input = bar.el.querySelector('.var-input')!; + const before = input.style.getPropertyValue('--var-input-ch'); + input.value = 'a much longer value than the field is wide'; + input.dispatchEvent(new Event('input', { bubbles: true })); + expect(input.style.getPropertyValue('--var-input-ch')).toBe(before); + }); + }); + 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/filter-width.test.ts b/tests/unit/filter-width.test.ts new file mode 100644 index 00000000..69a56639 --- /dev/null +++ b/tests/unit/filter-width.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect } from 'vitest'; +import { filterWidthCategory, filterInputWidthCh } from '../../src/core/filter-width.js'; + +describe('filterWidthCategory', () => { + it('classifies Bool/Boolean and the single-byte integers as bool', () => { + for (const t of ['Bool', 'Boolean', 'Int8', 'UInt8']) { + expect(filterWidthCategory(t)).toBe('bool'); + } + }); + + it('classifies wider integers, floats, and Decimal as numeric', () => { + for (const t of ['Int16', 'UInt32', 'Int64', 'UInt128', 'Int256', 'Float32', 'Float64', 'BFloat16', 'Decimal', 'Decimal(10, 2)', 'Decimal64']) { + expect(filterWidthCategory(t)).toBe('numeric'); + } + }); + + it('classifies Date/Date32 as date', () => { + expect(filterWidthCategory('Date')).toBe('date'); + expect(filterWidthCategory('Date32')).toBe('date'); + }); + + it('classifies DateTime/DateTime64 as datetime, distinct from Date', () => { + expect(filterWidthCategory('DateTime')).toBe('datetime'); + expect(filterWidthCategory("DateTime64(3)")).toBe('datetime'); + }); + + it('classifies String/UUID/FixedString/unrecognized as the generic string fallback', () => { + for (const t of ['String', 'UUID', 'FixedString(4)', 'IPv4', 'Whatever', 'Array(String)']) { + expect(filterWidthCategory(t)).toBe('string'); + } + }); + + it('unwraps Nullable/LowCardinality to the effective base, same as parseParamType', () => { + expect(filterWidthCategory('Nullable(UInt8)')).toBe('bool'); + expect(filterWidthCategory('LowCardinality(Float64)')).toBe('numeric'); + expect(filterWidthCategory('LowCardinality(Nullable(DateTime))')).toBe('datetime'); + }); + + it('isEnumLike always wins over the type\'s own base, including for a curated non-Enum type', () => { + expect(filterWidthCategory('String', true)).toBe('enum'); + expect(filterWidthCategory("Enum8('a' = 1)", true)).toBe('enum'); + expect(filterWidthCategory('UInt8', true)).toBe('enum'); + }); + + it('defaults isEnumLike to false when omitted', () => { + expect(filterWidthCategory('String')).toBe('string'); + }); +}); + +describe('filterInputWidthCh', () => { + it('maps each category to its stable ch width', () => { + expect(filterInputWidthCh('UInt8')).toBe(9); + expect(filterInputWidthCh('Int32')).toBe(13); + expect(filterInputWidthCh('Date')).toBe(13); + expect(filterInputWidthCh('DateTime64(3)')).toBe(17); + expect(filterInputWidthCh('String', true)).toBe(14); + expect(filterInputWidthCh('String')).toBe(16); + expect(filterInputWidthCh('UUID')).toBe(16); + }); +}); diff --git a/tests/unit/var-field.test.ts b/tests/unit/var-field.test.ts index ab3da528..36637669 100644 --- a/tests/unit/var-field.test.ts +++ b/tests/unit/var-field.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { applyFieldState } from '../../src/ui/var-field.js'; +import { applyFieldState, applyFieldWidth } from '../../src/ui/var-field.js'; function makeInput() { return document.createElement('input'); @@ -92,3 +92,28 @@ describe('applyFieldState', () => { }); }); }); + +// #345: the compact, type-aware `--var-input-ch` custom property a field's +// CSS width reads. +describe('applyFieldWidth', () => { + it('sets the resolved ch width as the --var-input-ch custom property', () => { + const el = makeInput(); + applyFieldWidth(el, 'UInt8'); + expect(el.style.getPropertyValue('--var-input-ch')).toBe('9'); + }); + it('a wider numeric type gets a wider ch width', () => { + const el = makeInput(); + applyFieldWidth(el, 'Float64'); + expect(el.style.getPropertyValue('--var-input-ch')).toBe('13'); + }); + it('isEnumLike overrides the type-derived category', () => { + const el = makeInput(); + applyFieldWidth(el, 'String', true); + expect(el.style.getPropertyValue('--var-input-ch')).toBe('14'); + }); + it('defaults isEnumLike to false when omitted', () => { + const el = makeInput(); + applyFieldWidth(el, 'String'); + expect(el.style.getPropertyValue('--var-input-ch')).toBe('16'); + }); +});