fix(ui): user-test round 4 — five browser-reported control defects

Every one of these was silent at the code level and loud in a real
browser; each fix carries its own tripwire (render pin, unit pin, or
CSP-safe rewrite) so the class cannot quietly return.

1. Theme picker dead since the CSP migration (eb57b720): the Appearance
   panel's theme buttons emitted `@click="selectTheme(opt.Name)"` as
   LITERAL text (templ gotcha #1 — quoted attribute values are never
   interpolated; `opt` is a Go loop var), so Alpine threw "Undefined
   variable: opt" on every click and no PUT /api/auth/theme ever fired.
   The wood-paneling sibling always used the correct expression form,
   which is why the panel half-worked. Fixed to the interpolated form;
   TestThemePickerHandlersAreInterpolated pins that real theme names
   reach the HTML and the raw identifier never does.

2. Filter-builder op dropdown "does nothing": the field ingest
   hardcoded `ops: []` into every field's meta while only mirroring the
   ops into the global label registry — opsFor() returned [] for EVERY
   field, so the op menu rendered zero items: an invisible menu, no
   console error (user tried author/publisher/title/subtitle, all
   equally dead). FieldMeta.ops is now the wire's op objects, opsFor()
   is a standalone exported helper, and opsForField unit tests pin the
   round-trip. Verified live: the field menu opens/closes while the op
   menu stayed shut, both with identical wiring — the difference was
   the data, not the wiring.

3. AND/OR connective chip dead: the chip carried an `if` STATEMENT in
   the Alpine attribute (`@click="if (row.chipClickable) ..."`) which
   the CSP build's evaluator rejects as a statement — the whole
   expression silently never ran. Moved the clickability check into a
   toggleConnective(row) component method; the attribute is now a plain
   method call.

4. Pencil vs card navigation race: navigateToCollection guarded with
   `event.target instanceof HTMLElement`, which is FALSE for the
   pencil glyph's SVGElement — a click landing on the glyph sailed past
   the guard and navigated while the htmx edit-modal request still
   fired (the reported "modal flashes, then the builder page"). The
   guard now uses target.closest("button") (works on SVGElement) and
   takes $event explicitly instead of the deprecated window.event.
   .icon-btn is already a 36px square, so no hit-area change needed.

5. Add Items modal under the sidebar + taller than the window: the
   picker overlay sat at z-50 UNDER the app-sidebar's z-60 (its left
   edge rendered beneath the nav), and the card had no height bound —
   header + tabs + filters + grid + footer overflow short windows with
   the footer unreachable (fixed overlay, no page scroll). The picker
   and create-modal overlays move to z-[65], both confirm dialogs to
   z-[70], and the picker card is now a flex column capped at
   max-h-[85vh] whose grids flex-fill (they grow on tall windows,
   shrink on short ones). The ladder is documented at .app-sidebar in
   input.css: sidebar 60 / overlays 65 / confirm 70 / toasts 9999.

Found by clicking every control the way a user does (panel clicks +
computed-style reads + screenshots) after the previous sweep trusted
DOM-text snapshots, which show hidden menus' text and cannot see
occlusion, stacking, or geometry at all.
This commit is contained in:
John O'Keefe committed 2026-10-31 13:14:05 -04:00
1 parent 8aec80cbb7
commit d5dbaa48ea
15 files changed
+299 -177

No files matched your search

+11 -7
View File
@@ -54,13 +54,17 @@ function setupHTMXAuth(): void {
});
}
function navigateToCollection(element: HTMLElement): void {
const event = window.event as Event;
if (event && event.target instanceof HTMLElement) {
const target = event.target as HTMLElement;
if (target.tagName === "BUTTON" || target.closest("button") !== null) {
return;
}
function navigateToCollection(element: HTMLElement, event?: Event): void {
// ANY ancestor-or-self button swallows the click. The guard used to be
// `event.target instanceof HTMLElement` — false for SVG elements — so
// a click landing exactly on the pencil glyph's <svg>/<path> sailed
// past it and navigated while the htmx edit-modal request still fired
// (user report 2026-10-31: modal "flashes", then the builder page).
// Element.closest works on SVGElement too; window.event is also gone
// (deprecated) in favor of the passed $event.
const target = event?.target as Element | null;
if (target && typeof target.closest === "function" && target.closest("button") !== null) {
return;
}
const href = element.getAttribute("data-href");
+27 -3
View File
@@ -4,6 +4,7 @@ import {
ingestFilterFields,
isFlatable,
isRule,
opsForField,
pruneIncomplete,
ruleToFlatParam,
type FilterGroupState,
@@ -49,9 +50,7 @@ const rule = (field: string, op: string, values: string[] = []): FilterRuleState
field,
op,
values,
});
const group = (
});const group = (
mode: "all" | "any",
children: (FilterRuleState | FilterGroupState)[],
implicit = false,
@@ -201,3 +200,28 @@ describe("isRule", () => {
expect(isRule(group("all", []))).toBe(false);
});
});
// The op menu renders straight from opsForField — the ingest hardcoded
// `ops: []` into every field's meta (2026-10-31 user report: the op
// dropdown "did nothing" — a zero-item menu renders invisible, no
// console error), so the wire round-trip is pinned here.
describe("opsForField", () => {
it("returns the field's ingested op objects", () => {
const ops = opsForField("published_year");
expect(ops.map((o) => o.key)).toEqual(["between", "at_least", "at_most"]);
expect(ops[0]).toMatchObject({ label: "between", arity: "two", negated: false });
});
it("normalizes missing negated/arity", () => {
expect(opsForField("cover")[0]).toMatchObject({
key: "has",
label: "has",
negated: false,
arity: "none",
});
});
it("returns [] for unknown fields", () => {
expect(opsForField("nonexistent_field")).toEqual([]);
});
});
+38 -4
View File
@@ -43,6 +43,16 @@ let wireOps: Record<string, OpMeta> = {};
let wireFieldGroups: { group: string; fields: FieldMeta[] }[] = [];
const FIELD_BY_KEY = new Map<string, FieldMeta>();
// opsForField — the op menu's item list for a field: the ingested wire
// op objects ({key, label, negated, arity}). Standalone + exported so
// the invisible-menu regression has a unit pin: the ingest hardcoded
// `ops: []` into every field (2026-10-31 user report: the op dropdown
// "did nothing" — a zero-item menu renders at zero height, no console
// error), so the op list must always round-trip from the wire.
export function opsForField(fieldKey: string): OpMeta[] {
return FIELD_BY_KEY.get(fieldKey)?.ops ?? [];
}
export function ingestFilterFields(raw: unknown): void {
if (!Array.isArray(raw)) return;
const fields = raw as WireField[];
@@ -56,7 +66,16 @@ export function ingestFilterFields(raw: unknown): void {
label: f.label,
group: f.group,
valueKind: f.valueKind,
ops: [],
// Copy the wire op objects verbatim (normalized) — opsFor() feeds
// the op menu's x-for directly; an empty array here renders an
// invisible zero-item menu (the 2026-10-31 "op dropdown does
// nothing" bug).
ops: (f.ops ?? []).map((o) => ({
key: o.key,
label: o.label,
negated: o.negated === true,
arity: (o.arity ?? "many") as OpMeta["arity"],
})),
defaultOp: f.defaultOp,
suggest: f.suggest === true,
options: f.options,
@@ -108,7 +127,12 @@ interface FieldMeta {
valueKind: "text" | "number" | "days" | "id" | "fixed" | "none";
suggest: boolean;
options?: string[];
ops: string[];
// The field's supported operators as the wire op objects
// ({key, label, negated, arity}) — the op menu renders straight from
// this list. WAS typed string[] and hardcoded [] at ingest, which
// made every op menu render zero items (invisible menu, no console
// error — user report 2026-10-31).
ops: OpMeta[];
defaultOp: string;
}
@@ -444,7 +468,7 @@ function filterBuilderData() {
return FIELD_BY_KEY.get(key)?.label ?? "Field";
},
opsFor(fieldKey: string): OpMeta[] {
return (FIELD_BY_KEY.get(fieldKey)?.ops ?? []) as unknown as OpMeta[];
return opsForField(fieldKey);
},
opMeta(opKey: string): OpMeta | undefined {
return wireOps[opKey];
@@ -652,6 +676,16 @@ function filterBuilderData() {
}
},
// toggleConnective — CSP-safe wrapper for the connective chip: the
// CSP build's evaluator rejects `if` STATEMENTS in attribute
// expressions, so the chip's `@click="if (row.chipClickable)
// toggleRuleConnective(row.path)"` silently never ran (user report
// 2026-10-31). The clickability check lives here now.
toggleConnective(row: BuilderRow): void {
if (!row.chipClickable) return;
this.toggleRuleConnective(row.path);
},
// visibleFieldGroups — the field menu's groups, filtered by the
// menu's filter input (label or key substring, case-insensitive);
// groups with no matches hide.
@@ -890,7 +924,7 @@ function filterBuilderData() {
const input = form.querySelector(`input[name="${wire}"]`) as HTMLInputElement | null;
if (!input || !input.value) continue;
const meta = FIELD_BY_KEY.get(fieldKey);
const op = meta && meta.ops.includes("contains") ? "contains" : "is";
const op = meta && meta.ops.some((o) => o.key === "contains") ? "contains" : "is";
rules.push({ field: fieldKey, op, values: [input.value] });
}
const cover = form.querySelector('input[name="has_cover"]') as HTMLInputElement | null;