From 0c334a113a0f35a65a162300686d7d28381cff54 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Tue, 25 Aug 2026 07:56:01 -0700 Subject: [PATCH] Give a hand-typed header its own box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking "Other header…" in the filter dialog took the comparator away. The condition row has three columns — field, comparator, value — and the box for the header name was rendered into the comparator's, so the comparator disappeared along with any way to change it. Whatever it had been when you switched, contains, was what the rule got: matching a header exactly, or on a regex, could not be expressed at all. The header name now has a column of its own and the comparator keeps its, on a row that widens to hold both. Fixes #23 --- web/src/styles/app.css | 2 + web/src/views/settings/RuleDialog.tsx | 74 ++++++++++--------- .../__tests__/rule-dialog-header.test.tsx | 67 +++++++++++++++++ 3 files changed, 109 insertions(+), 34 deletions(-) create mode 100644 web/src/views/settings/__tests__/rule-dialog-header.test.tsx diff --git a/web/src/styles/app.css b/web/src/styles/app.css index 5c8bdbb..d5a9c32 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -607,6 +607,8 @@ img { max-width: 100%; } .rule-card.disabled { opacity: .6; } .rule-row { display: grid; grid-template-columns: 1fr 1fr 1fr auto; gap: 8px; align-items: center; margin-bottom: 8px; } .rule-row.actions { grid-template-columns: 1fr 2fr auto; } +/* A header typed by hand needs a box of its own, alongside the comparator. */ +.rule-row.named-header { grid-template-columns: 1fr 1fr 1fr 1fr auto; } .code { font-family: var(--font-mono); font-size: 12.5px; line-height: 1.5; white-space: pre; overflow: auto; background: var(--bg-sunken); border: 1px solid var(--border); border-radius: var(--radius-sm); padding: 12px; min-height: 240px; width: 100%; resize: vertical; tab-size: 2; } .shortcut-grid { display: grid; grid-template-columns: repeat(auto-fill, minmax(300px, 1fr)); gap: 16px 32px; } .shortcut-grid h3 { margin: 0 0 6px; font-size: .9em; text-transform: uppercase; letter-spacing: .05em; color: var(--fg-faint); } diff --git a/web/src/views/settings/RuleDialog.tsx b/web/src/views/settings/RuleDialog.tsx index 93c3341..d23d682 100644 --- a/web/src/views/settings/RuleDialog.tsx +++ b/web/src/views/settings/RuleDialog.tsx @@ -43,41 +43,47 @@ export function RuleDialog({ rule, onClose, onSave, applyMailbox, title, saveLab - {r.tests.map((t, i) => ( -
- - {t.type === "header" && !HEADER_CHOICES.some((h) => h.value === t.header && h.value !== "__custom__") ? ( - setTest(i, { ...t, header: e.target.value })} /> - ) : t.type === "size" ? ( - - ) : t.type === "body" ? ( - - ) : t.type === "true" ? : ( - h.value === t.header) ? t.header : "__custom__"} onChange={(e) => { + const v = e.target.value; + if (v === "size") setTest(i, { type: "size", op: "over", value: 1024 * 1024 }); + else if (v === "body") setTest(i, { type: "body", op: "contains", value: "" }); + else if (v === "true") setTest(i, { type: "true" }); + else if (v === "address") setTest(i, { type: "address", header: "from", part: "domain", op: "is", value: "" }); + else setTest(i, { type: "header", header: v === "__custom__" ? "" : v, op: "contains", value: "" }); + }}> + {HEADER_CHOICES.map((h) => )} + + + + - )} - {t.type === "size" ? ( -
setTest(i, { ...t, value: Number(e.target.value) * 1024 })} />KB
- ) : t.type === "true" ? : t.type === "header" && (t.op === "exists" || t.op === "notexists") ? : ( - setTest(i, { ...t, value: e.target.value } as SieveTest)} /> - )} - -
- ))} + {customHeader && t.type === "header" && ( + setTest(i, { ...t, header: e.target.value })} /> + )} + {t.type === "size" ? ( + + ) : t.type === "body" ? ( + + ) : t.type === "true" ? : ( + + )} + {t.type === "size" ? ( +
setTest(i, { ...t, value: Number(e.target.value) * 1024 })} />KB
+ ) : t.type === "true" ? : t.type === "header" && (t.op === "exists" || t.op === "notexists") ? : ( + setTest(i, { ...t, value: e.target.value } as SieveTest)} /> + )} + + + ); + })}
Then
diff --git a/web/src/views/settings/__tests__/rule-dialog-header.test.tsx b/web/src/views/settings/__tests__/rule-dialog-header.test.tsx new file mode 100644 index 0000000..8022f4b --- /dev/null +++ b/web/src/views/settings/__tests__/rule-dialog-header.test.tsx @@ -0,0 +1,67 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { RuleDialog } from "../RuleDialog"; +import { newRule } from "@/lib/sieve"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** + * Choosing "Other header…" used to put the header-name box in the column the + * comparator lived in, so the comparator vanished: whatever it happened to be + * (contains) was what you were stuck with. Both belong in the row. + */ +describe("RuleDialog custom headers", () => { + let host: HTMLDivElement; + let root: Root; + + /** The condition row's own selects: [field, comparator]. */ + const selects = () => Array.from(document.querySelectorAll(".rule-row:not(.actions) select")); + const find = (sel: string) => document.querySelector(sel); + const pick = (el: HTMLSelectElement, value: string) => act(() => { + el.value = value; + el.dispatchEvent(new Event("change", { bubbles: true })); + }); + + beforeEach(() => { + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + }); + afterEach(() => { + act(() => root.unmount()); + host.remove(); + }); + + const render = () => act(() => { + root.render( undefined} onSave={() => undefined} />); + }); + + it("keeps the comparator when a header is typed by hand", () => { + render(); + // [field, comparator] — the rule starts on "from contains". + expect(selects()).toHaveLength(2); + pick(selects()[0]!, "__custom__"); + + const header = find('input[aria-label="Header name"]') as HTMLInputElement | null; + expect(header).not.toBeNull(); + expect(header!.value).toBe(""); + const ops = selects()[1]!; + expect(ops.value).toBe("contains"); + expect(Array.from(ops.options).map((o) => o.value)).toContain("matches"); + + pick(ops, "matches"); + expect(selects()[1]!.value).toBe("matches"); + // The header box is still there, and still has a column of its own. + expect(find('input[aria-label="Header name"]')).not.toBeNull(); + expect(find(".rule-row.named-header")).not.toBeNull(); + }); + + it("leaves a listed header alone", () => { + render(); + expect(find('input[aria-label="Header name"]')).toBeNull(); + expect(find(".rule-row.named-header")).toBeNull(); + pick(selects()[1]!, "is"); + expect(selects()[1]!.value).toBe("is"); + }); +});