Give a hand-typed header its own box
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
This commit is contained in:
@@ -607,6 +607,8 @@ img { max-width: 100%; }
|
|||||||
.rule-card.disabled { opacity: .6; }
|
.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 { 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; }
|
.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; }
|
.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 { 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); }
|
.shortcut-grid h3 { margin: 0 0 6px; font-size: .9em; text-transform: uppercase; letter-spacing: .05em; color: var(--fg-faint); }
|
||||||
|
|||||||
@@ -43,41 +43,47 @@ export function RuleDialog({ rule, onClose, onSave, applyMailbox, title, saveLab
|
|||||||
<option value="anyof">any of the following match</option>
|
<option value="anyof">any of the following match</option>
|
||||||
</select>
|
</select>
|
||||||
</div>
|
</div>
|
||||||
{r.tests.map((t, i) => (
|
{r.tests.map((t, i) => {
|
||||||
<div key={i} className="rule-row">
|
// A header the dropdown doesn't list needs a box to type its name in,
|
||||||
<select className="select" value={t.type === "true" ? "true" : t.type === "size" ? "size" : t.type === "body" ? "body" : t.type === "address" ? "address" : HEADER_CHOICES.some((h) => h.value === t.header) ? t.header : "__custom__"} onChange={(e) => {
|
// which takes a column of its own — the comparator keeps its.
|
||||||
const v = e.target.value;
|
const customHeader = t.type === "header" && !HEADER_CHOICES.some((h) => h.value === t.header && h.value !== "__custom__");
|
||||||
if (v === "size") setTest(i, { type: "size", op: "over", value: 1024 * 1024 });
|
return (
|
||||||
else if (v === "body") setTest(i, { type: "body", op: "contains", value: "" });
|
<div key={i} className={`rule-row${customHeader ? " named-header" : ""}`}>
|
||||||
else if (v === "true") setTest(i, { type: "true" });
|
<select className="select" value={t.type === "true" ? "true" : t.type === "size" ? "size" : t.type === "body" ? "body" : t.type === "address" ? "address" : HEADER_CHOICES.some((h) => h.value === t.header) ? t.header : "__custom__"} onChange={(e) => {
|
||||||
else if (v === "address") setTest(i, { type: "address", header: "from", part: "domain", op: "is", value: "" });
|
const v = e.target.value;
|
||||||
else setTest(i, { type: "header", header: v === "__custom__" ? "" : v, op: "contains", value: "" });
|
if (v === "size") setTest(i, { type: "size", op: "over", value: 1024 * 1024 });
|
||||||
}}>
|
else if (v === "body") setTest(i, { type: "body", op: "contains", value: "" });
|
||||||
{HEADER_CHOICES.map((h) => <option key={h.value} value={h.value}>{h.label}</option>)}
|
else if (v === "true") setTest(i, { type: "true" });
|
||||||
<option value="address">Sender domain</option>
|
else if (v === "address") setTest(i, { type: "address", header: "from", part: "domain", op: "is", value: "" });
|
||||||
<option value="size">Message size</option>
|
else setTest(i, { type: "header", header: v === "__custom__" ? "" : v, op: "contains", value: "" });
|
||||||
<option value="body">Body text</option>
|
}}>
|
||||||
<option value="true">Always (all messages)</option>
|
{HEADER_CHOICES.map((h) => <option key={h.value} value={h.value}>{h.label}</option>)}
|
||||||
</select>
|
<option value="address">Sender domain</option>
|
||||||
{t.type === "header" && !HEADER_CHOICES.some((h) => h.value === t.header && h.value !== "__custom__") ? (
|
<option value="size">Message size</option>
|
||||||
<input className="input" placeholder="Header name" value={t.header} onChange={(e) => setTest(i, { ...t, header: e.target.value })} />
|
<option value="body">Body text</option>
|
||||||
) : t.type === "size" ? (
|
<option value="true">Always (all messages)</option>
|
||||||
<select className="select" value={t.op} onChange={(e) => setTest(i, { ...t, op: e.target.value as "over" | "under" })}><option value="over">is larger than</option><option value="under">is smaller than</option></select>
|
|
||||||
) : t.type === "body" ? (
|
|
||||||
<select className="select" value={t.op} onChange={(e) => setTest(i, { ...t, op: e.target.value as "contains" | "notcontains" })}><option value="contains">contains</option><option value="notcontains">does not contain</option></select>
|
|
||||||
) : t.type === "true" ? <span /> : (
|
|
||||||
<select className="select" value={t.op} onChange={(e) => setTest(i, { ...t, op: e.target.value as SieveTest extends { op: infer O } ? O : never })}>
|
|
||||||
{HEADER_OPS.map((o) => <option key={o.value} value={o.value}>{o.label}</option>)}
|
|
||||||
</select>
|
</select>
|
||||||
)}
|
{customHeader && t.type === "header" && (
|
||||||
{t.type === "size" ? (
|
<input className="input" placeholder="Header name" aria-label="Header name" value={t.header} onChange={(e) => setTest(i, { ...t, header: e.target.value })} />
|
||||||
<div className="row"><input className="input" type="number" min={1} value={Math.round(t.value / 1024)} onChange={(e) => setTest(i, { ...t, value: Number(e.target.value) * 1024 })} /><span className="muted">KB</span></div>
|
)}
|
||||||
) : t.type === "true" ? <span /> : t.type === "header" && (t.op === "exists" || t.op === "notexists") ? <span /> : (
|
{t.type === "size" ? (
|
||||||
<input className="input" placeholder={t.type === "address" ? "example.com" : "value"} value={(t as { value: string }).value} onChange={(e) => setTest(i, { ...t, value: e.target.value } as SieveTest)} />
|
<select className="select" value={t.op} onChange={(e) => setTest(i, { ...t, op: e.target.value as "over" | "under" })}><option value="over">is larger than</option><option value="under">is smaller than</option></select>
|
||||||
)}
|
) : t.type === "body" ? (
|
||||||
<button className="icon-btn sm danger" aria-label="Remove condition" onClick={() => setR({ ...r, tests: r.tests.filter((_, j) => j !== i) })} disabled={r.tests.length <= 1}><Trash2 size={16} /></button>
|
<select className="select" value={t.op} onChange={(e) => setTest(i, { ...t, op: e.target.value as "contains" | "notcontains" })}><option value="contains">contains</option><option value="notcontains">does not contain</option></select>
|
||||||
</div>
|
) : t.type === "true" ? <span /> : (
|
||||||
))}
|
<select className="select" value={t.op} onChange={(e) => setTest(i, { ...t, op: e.target.value as SieveTest extends { op: infer O } ? O : never })}>
|
||||||
|
{HEADER_OPS.map((o) => <option key={o.value} value={o.value}>{o.label}</option>)}
|
||||||
|
</select>
|
||||||
|
)}
|
||||||
|
{t.type === "size" ? (
|
||||||
|
<div className="row"><input className="input" type="number" min={1} value={Math.round(t.value / 1024)} onChange={(e) => setTest(i, { ...t, value: Number(e.target.value) * 1024 })} /><span className="muted">KB</span></div>
|
||||||
|
) : t.type === "true" ? <span /> : t.type === "header" && (t.op === "exists" || t.op === "notexists") ? <span /> : (
|
||||||
|
<input className="input" placeholder={t.type === "address" ? "example.com" : "value"} value={(t as { value: string }).value} onChange={(e) => setTest(i, { ...t, value: e.target.value } as SieveTest)} />
|
||||||
|
)}
|
||||||
|
<button className="icon-btn sm danger" aria-label="Remove condition" onClick={() => setR({ ...r, tests: r.tests.filter((_, j) => j !== i) })} disabled={r.tests.length <= 1}><Trash2 size={16} /></button>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
})}
|
||||||
<button className="btn btn-ghost btn-sm" onClick={() => setR({ ...r, tests: [...r.tests, { type: "header", header: "subject", op: "contains", value: "" }] })}><Plus size={14} /> Add condition</button>
|
<button className="btn btn-ghost btn-sm" onClick={() => setR({ ...r, tests: [...r.tests, { type: "header", header: "subject", op: "contains", value: "" }] })}><Plus size={14} /> Add condition</button>
|
||||||
|
|
||||||
<div className="row" style={{ margin: "16px 0 8px" }}><span className="label">Then</span></div>
|
<div className="row" style={{ margin: "16px 0 8px" }}><span className="label">Then</span></div>
|
||||||
|
|||||||
@@ -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<HTMLSelectElement>(".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(<RuleDialog rule={newRule({ id: "r1" })} onClose={() => 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");
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user