From 696b3713ed270094d257f4a63463af6193a77756 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Tue, 25 Aug 2026 08:50:43 -0700 Subject: [PATCH] Let a filter rule be dragged into place Twenty-five rules and two buttons that move one place at a time meant a rule pushed to the wrong end cost ten clicks to bring back. It can now be dragged. A grip on the left of each card arms the drag, so the switch, the name and the buttons still take a plain click, and the up and down buttons stay for the keyboard. The card being dragged fades; the one under the pointer draws a line on the edge the rule would land on, top half or bottom. The guard against dropping a rule onto itself reads a ref rather than state: dragstart and the first dragover can arrive in the same frame, and a stale read there drew a drop line on the card being dragged. Found by driving the real thing in a browser, and covered by a test that fires the two events back to back. --- web/src/lib/__tests__/sieve.test.ts | 20 +++- web/src/lib/sieve.ts | 14 +++ web/src/styles/app.css | 6 ++ web/src/views/settings/FiltersSettings.tsx | 51 +++++++++- .../__tests__/rule-drag-reorder.test.tsx | 98 +++++++++++++++++++ 5 files changed, 184 insertions(+), 5 deletions(-) create mode 100644 web/src/views/settings/__tests__/rule-drag-reorder.test.tsx diff --git a/web/src/lib/__tests__/sieve.test.ts b/web/src/lib/__tests__/sieve.test.ts index 36fb50b..58f2235 100644 --- a/web/src/lib/__tests__/sieve.test.ts +++ b/web/src/lib/__tests__/sieve.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { newRule, rulesToSieve, sieveToRules, testToSieve, sieveString, upsertRule } from "../sieve"; +import { newRule, reorderRules, rulesToSieve, sieveToRules, testToSieve, sieveString, upsertRule, type SieveRule } from "../sieve"; describe("sieve codec", () => { it("escapes strings", () => { @@ -37,3 +37,21 @@ describe("sieve codec", () => { expect(sieveToRules("")).toEqual([]); }); }); + +describe("reordering rules", () => { + const ids = (rs: SieveRule[]) => rs.map((r) => r.id); + const list = ["a", "b", "c", "d"].map((id) => newRule({ id })); + + it("drops a rule above or below the card it was dropped on", () => { + expect(ids(reorderRules(list, "a", "c", false))).toEqual(["b", "a", "c", "d"]); + expect(ids(reorderRules(list, "a", "c", true))).toEqual(["b", "c", "a", "d"]); + expect(ids(reorderRules(list, "d", "a", false))).toEqual(["d", "a", "b", "c"]); + expect(ids(reorderRules(list, "b", "d", true))).toEqual(["a", "c", "d", "b"]); + }); + it("leaves the list alone when the drop goes nowhere", () => { + expect(reorderRules(list, "a", "a", true)).toBe(list); + expect(reorderRules(list, "a", "zz", true)).toBe(list); + expect(reorderRules(list, "zz", "a", true)).toBe(list); + expect(ids(list)).toEqual(["a", "b", "c", "d"]); + }); +}); diff --git a/web/src/lib/sieve.ts b/web/src/lib/sieve.ts index 153edfd..5c0417f 100644 --- a/web/src/lib/sieve.ts +++ b/web/src/lib/sieve.ts @@ -210,6 +210,20 @@ export function upsertRule(rules: SieveRule[], rule: SieveRule): SieveRule[] { return rules.some((x) => x.id === rule.id) ? rules.map((x) => (x.id === rule.id ? rule : x)) : [...rules, rule]; } +/** + * Moves the rule `fromId` to sit either side of `toId`. `below` says which, + * decided by which half of the target card the pointer was over. + */ +export function reorderRules(rules: SieveRule[], fromId: string, toId: string, below: boolean): SieveRule[] { + if (fromId === toId) return rules; + const moved = rules.find((r) => r.id === fromId); + const rest = rules.filter((r) => r.id !== fromId); + const target = rest.findIndex((r) => r.id === toId); + if (!moved || target < 0) return rules; + const at = below ? target + 1 : target; + return [...rest.slice(0, at), moved, ...rest.slice(at)]; +} + export function describeRule(r: SieveRule): string { const tests = r.tests .map((t) => { diff --git a/web/src/styles/app.css b/web/src/styles/app.css index d5a9c32..8b8edd7 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -605,6 +605,12 @@ img { max-width: 100%; } .swatch.active { border-color: var(--fg); } .rule-card { border: 1px solid var(--border); border-radius: var(--radius); padding: 12px 14px; margin-bottom: 10px; background: var(--bg-elev); } .rule-card.disabled { opacity: .6; } +.rule-card.dragging { opacity: .5; } +/* The line shows which side of this card the dragged rule would land on. */ +.rule-card.drop-above { box-shadow: inset 0 3px 0 0 var(--accent); } +.rule-card.drop-below { box-shadow: inset 0 -3px 0 0 var(--accent); } +.rule-card .drag-handle { display: flex; align-items: center; padding: 2px; margin-left: -4px; color: var(--fg-faint); cursor: grab; touch-action: none; } +.rule-card .drag-handle:active { cursor: grabbing; } .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. */ diff --git a/web/src/views/settings/FiltersSettings.tsx b/web/src/views/settings/FiltersSettings.tsx index 88f76bd..e5ff16f 100644 --- a/web/src/views/settings/FiltersSettings.tsx +++ b/web/src/views/settings/FiltersSettings.tsx @@ -1,8 +1,8 @@ -import { useEffect, useState } from "react"; -import { ArrowDown, ArrowUp, Code, Plus, Trash2, Wand2, Play, AlertTriangle, Power } from "lucide-react"; +import { useEffect, useRef, useState } from "react"; +import { ArrowDown, ArrowUp, Code, GripVertical, Plus, Trash2, Wand2, Play, AlertTriangle, Power } from "lucide-react"; import { useSieve } from "@/store/sieve"; import { useMail } from "@/store/mail"; -import { describeRule, newRule, rulesToSieve, upsertRule, type SieveRule } from "@/lib/sieve"; +import { describeRule, newRule, reorderRules, rulesToSieve, upsertRule, type SieveRule } from "@/lib/sieve"; import { RuleDialog } from "./RuleDialog"; import { saveAndApply } from "../mail/FilterFromMessage"; import { confirmDialog, promptDialog } from "@/ui/dialog"; @@ -40,6 +40,9 @@ export function FiltersSettings() { ); } +/** Private drag type, so a rule can only be dropped on the rule list. */ +const RULE_MIME = "application/x-ihasmail-sieve-rule"; + function RulesEditor() { const sieve = useSieve(); const { script, rules, content } = sieve.rules(); @@ -49,6 +52,19 @@ function RulesEditor() { const list = local ?? rules ?? []; const dirty = local !== null; const inbox = useMail((s) => { const id = s.roleId("inbox"); return id ? s.mailboxes[id] : undefined; }); + /** The rule being dragged, the one armed to be, and where a drop would land. */ + const [dragId, setDragId] = useState(null); + const [armed, setArmed] = useState(null); + const [over, setOver] = useState<{ id: string; below: boolean } | null>(null); + /** + * The same id as `dragId`, kept synchronously: the first dragover can arrive + * before React has re-rendered with the state, and a stale read there draws a + * drop line on the card being dragged. + */ + const dragging = useRef(null); + const endDrag = () => { dragging.current = null; setDragId(null); setArmed(null); setOver(null); }; + /** Which half of the card the pointer is over decides which side of it the rule lands. */ + const isBelow = (el: HTMLElement, y: number) => { const b = el.getBoundingClientRect(); return y > b.top + b.height / 2; }; const activeIsOther = script && script.name !== "ihasmail" && script.isActive; const save = async (next: SieveRule[]) => { @@ -79,8 +95,35 @@ function RulesEditor() { {activeIsOther &&
Another script (“{script?.name}”) is active. Saving rules here will activate the “ihasmail” script instead.
} {list.length === 0 &&

No filters yet

Create a rule to move newsletters to a folder, flag important senders, or forward mail.

} {list.map((r, i) => ( -
+
{ e.dataTransfer.setData(RULE_MIME, r.id); e.dataTransfer.effectAllowed = "move"; dragging.current = r.id; setDragId(r.id); }} + onDragEnd={endDrag} + onDragOver={(e) => { + if (!e.dataTransfer.types.includes(RULE_MIME) || dragging.current === r.id) return; + e.preventDefault(); + e.dataTransfer.dropEffect = "move"; + const below = isBelow(e.currentTarget, e.clientY); + if (over?.id !== r.id || over.below !== below) setOver({ id: r.id, below }); + }} + onDragLeave={() => setOver((o) => (o?.id === r.id ? null : o))} + onDrop={(e) => { + e.preventDefault(); + const from = e.dataTransfer.getData(RULE_MIME); + if (from) setLocal(reorderRules(list, from, r.id, isBelow(e.currentTarget, e.clientY))); + endDrag(); + }} + >
+ setLocal(list.map((x) => (x.id === r.id ? { ...x, enabled: v } : x)))} />
setEditing(r)}>
{r.name}
diff --git a/web/src/views/settings/__tests__/rule-drag-reorder.test.tsx b/web/src/views/settings/__tests__/rule-drag-reorder.test.tsx new file mode 100644 index 0000000..11bb8c4 --- /dev/null +++ b/web/src/views/settings/__tests__/rule-drag-reorder.test.tsx @@ -0,0 +1,98 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { FiltersSettings } from "../FiltersSettings"; +import { useSieve } from "@/store/sieve"; +import { newRule, rulesToSieve } from "@/lib/sieve"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** jsdom has no DataTransfer, and no layout — both are faked just enough. */ +function dataTransfer() { + const data: Record = {}; + return { + types: [] as string[], + effectAllowed: "", + dropEffect: "", + setData(k: string, v: string) { data[k] = v; this.types.push(k); }, + getData(k: string) { return data[k] ?? ""; }, + }; +} +function fire(el: Element, type: string, dt: ReturnType, clientY = 0) { + const ev = new Event(type, { bubbles: true, cancelable: true }); + Object.defineProperty(ev, "dataTransfer", { value: dt }); + Object.defineProperty(ev, "clientY", { value: clientY }); + act(() => { el.dispatchEvent(ev); }); +} +/** Cards have no size in jsdom, so any positive clientY counts as the lower half. */ +const LOWER = 1; +const UPPER = 0; + +describe("reordering rules by dragging", () => { + let host: HTMLDivElement; + let root: Root; + const cards = () => Array.from(document.querySelectorAll(".rule-card")); + const names = () => cards().map((c) => c.querySelector('div[style*="font-weight"]')?.textContent); + const press = (i: number) => act(() => { cards()[i]!.querySelector(".drag-handle")!.dispatchEvent(new Event("pointerdown", { bubbles: true })); }); + + beforeEach(() => { + const rules = ["Newsletters", "From the boss", "Receipts"].map((name, i) => newRule({ id: `r${i}`, name })); + useSieve.setState({ + accountId: "a", available: true, loading: false, error: null, + scripts: [{ id: "s1", name: "ihasmail", blobId: "b1", isActive: true }], + contents: { s1: rulesToSieve(rules) }, + }); + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + act(() => root.render()); + }); + afterEach(() => { act(() => root.unmount()); host.remove(); }); + + it("arms dragging only from the handle of the rule pressed", () => { + expect(cards()).toHaveLength(3); + expect(cards().map((c) => c.getAttribute("draggable"))).toEqual(["false", "false", "false"]); + press(1); + expect(cards().map((c) => c.getAttribute("draggable"))).toEqual(["false", "true", "false"]); + }); + + it("drops a rule below the card it was dragged onto", () => { + press(0); + const dt = dataTransfer(); + fire(cards()[0]!, "dragstart", dt); + expect(cards()[0]!.className).toContain("dragging"); + fire(cards()[2]!, "dragover", dt, LOWER); + expect(cards()[2]!.className).toContain("drop-below"); + fire(cards()[2]!, "drop", dt, LOWER); + expect(names()).toEqual(["From the boss", "Receipts", "Newsletters"]); + expect(cards().every((c) => !/dragging|drop-(above|below)/.test(c.className))).toBe(true); + }); + + it("drops a rule above the card when the pointer is in its top half", () => { + press(2); + const dt = dataTransfer(); + fire(cards()[2]!, "dragstart", dt); + fire(cards()[0]!, "dragover", dt, UPPER); + expect(cards()[0]!.className).toContain("drop-above"); + fire(cards()[0]!, "drop", dt, UPPER); + expect(names()).toEqual(["Receipts", "Newsletters", "From the boss"]); + }); + + it("draws no drop line on the rule being dragged, even before React re-renders", () => { + press(1); + const dt = dataTransfer(); + // dragstart and dragover back to back: the guard cannot wait for a render. + fire(cards()[1]!, "dragstart", dt); + fire(cards()[1]!, "dragover", dt, LOWER); + expect(cards().some((c) => /drop-(above|below)/.test(c.className))).toBe(false); + fire(cards()[1]!, "drop", dt, LOWER); + expect(names()).toEqual(["Newsletters", "From the boss", "Receipts"]); + }); + + it("ignores a drag that is not a rule", () => { + const dt = dataTransfer(); + dt.setData("text/plain", "hello"); + fire(cards()[1]!, "dragover", dt, LOWER); + expect(cards().some((c) => /drop-(above|below)/.test(c.className))).toBe(false); + }); +});