From 178b1f30c5ed2456650105a3459efef5fc7f8585 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sun, 1 Nov 2026 00:30:41 -0400 Subject: [PATCH] fix(filter-builder): menu opens to the whole list again; arity-aware cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression from the id-label resolution (09f8f087): opening an id-kind value menu carried the STORED ids as the typed filter, and the new id-exact-match narrowed the menu to exactly the already-picked rows — nothing else was visible, so "can't add more than one" (and clicking the lone visible row unpicked it). The ids now ride as the typed lookup ONLY in quiet mode (label resolution); a real menu open browses (blank) or filters by the typed NAME draft. The multi-value affordance is now ARITY-AWARE, per user request: the op decides how many values the cell takes, and both the hint and the pick behavior follow it — "in" says "pick one or more" and toggles ids into the comma list; "is" says "pick one" and REPLACES the value. Changing the operator updates the hint in place. Pins: menu-open after a pick must not carry the stored ids (the regression), one-arity pick replaces instead of toggling, and the placeholder follows the op's arity. 38 filter-builder vitest cases; web suite 183/183; container filter + episode-tree slices green. --- web/src/filter_builder.test.ts | 49 +++++++++++++++++++++++++++++++++- web/src/filter_builder.ts | 16 ++++++----- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/web/src/filter_builder.test.ts b/web/src/filter_builder.test.ts index 0c6ed3d1..b46747bb 100644 --- a/web/src/filter_builder.test.ts +++ b/web/src/filter_builder.test.ts @@ -242,7 +242,10 @@ const builderFields = [ valueKind: "id" as const, defaultOp: "in", suggest: true, - ops: [{ key: "in", label: "in", arity: "many" }], + ops: [ + { key: "in", label: "in", arity: "many" }, + { key: "is", label: "is", arity: "one" }, + ], }, { key: "author", @@ -436,6 +439,50 @@ describe("id-kind value cells", () => { vi.unstubAllGlobals(); }); + it("opening the menu after a pick browses the WHOLE list (regression)", async () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [{ field: "library", op: "in", values: ["lib-1"] }], + }; + c.idLabels["lib-1"] = "Books"; + let url = ""; + vi.stubGlobal( + "fetch", + vi.fn(async (u: string | URL) => { + url = String(u); + return { ok: true, json: async () => ({ results: [] }) }; + }), + ); + // the user opens the cell to ADD another — the stored ids must not + // filter the menu down to the picked row (2026-11-01 report: "can't + // add more than one"). + await c.fetchSuggestions("0", [0], c.filterTree.children[0] as FilterRuleState); + expect(url).not.toContain("lib-1"); + expect(url).toContain("library=+"); + vi.unstubAllGlobals(); + }); + + it("a one-arity op (is) picks ONE value — replace, not toggle", () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [{ field: "library", op: "is", values: ["lib-1"] }], + }; + c.pickSuggestion("0", [0], { value: "lib-2", display: "Comics", count: 2 }); + expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lib-2"]); + expect(c.valueCellText([0])).toBe("Comics"); + }); + + it("the placeholder follows the op's arity", () => { + const c = filterBuilderData(); + expect(c.valuePlaceholderFor({ field: "library", op: "in", values: [] })).toBe( + "pick one or more", + ); + expect(c.valuePlaceholderFor({ field: "library", op: "is", values: [] })).toBe("pick one"); + expect(c.valuePlaceholderFor({ field: "author", op: "contains", values: [] })).toBe("value"); + }); + it("text cells keep writing straight through", () => { const c = filterBuilderData(); c.filterTree = { diff --git a/web/src/filter_builder.ts b/web/src/filter_builder.ts index 26fb0376..8fa0ab30 100644 --- a/web/src/filter_builder.ts +++ b/web/src/filter_builder.ts @@ -727,12 +727,14 @@ export function filterBuilderData() { this.filterTree.children.forEach((child, idx) => visit(child, [idx])); }, - // valuePlaceholderFor — the value cell's hint. Id-kind cells are - // the few MULTI-VALUE boxes (pick several; the menu keeps toggling - // them in) — say so, because nothing else on the page does. + // valuePlaceholderFor — the value cell's hint, ARITY-AWARE: the op + // decides how many values the cell takes ("in" = several, "is" = + // one), and the hint follows the op as it changes. valuePlaceholderFor(rule: FilterRuleState): string { if (this.fieldIsDays(rule.field)) return "days"; - if (FIELD_BY_KEY.get(rule.field)?.valueKind === "id") return "pick one or more"; + if (FIELD_BY_KEY.get(rule.field)?.valueKind === "id") { + return this.valueArity(rule) === "many" ? "pick one or more" : "pick one"; + } return "value"; }, @@ -795,8 +797,8 @@ export function filterBuilderData() { const meta = FIELD_BY_KEY.get(node.field); this.idLabels[s.value] = s.display ?? s.value; delete this.idDrafts[path.join("-")]; // the cell shows labels again - if (meta?.valueKind === "id") { - // id-valued: toggle the id in the comma list. + if (meta?.valueKind === "id" && this.valueArity(node) === "many") { + // id-valued MULTI: toggle the id in the comma list. const ids = node.values.join(",").split(",").filter(Boolean); const at = ids.indexOf(s.value); if (at >= 0) { @@ -878,7 +880,7 @@ export function filterBuilderData() { if (meta.valueKind === "fixed") return; const typed = ( this.isIdValueRow(path) - ? (this.idDrafts[path.join("-")] ?? rule.values.join(",")) + ? (this.idDrafts[path.join("-")] ?? (quiet ? rule.values.join(",") : "")) : (rule.values[0] ?? "") ).trim(); // 1 char is too little to filter on; EMPTY means BROWSE — the