From 8a7ff5d42f56b67a9b5dc95bd828c3f11303d2ff Mon Sep 17 00:00:00 2001 From: jcoffey <51408202+jcoffey-dev@users.noreply.github.com> Date: Sat, 19 Sep 2026 14:06:21 -0700 Subject: [PATCH] Reorder folders by dragging, with special folders first (#402) (#405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The folder tree ignored sortOrder: Inbox came first, then everything A–Z, so Sent ended up among ordinary folders. The tree now lists Inbox, then any order the user has chosen, then the other special folders (Drafts, Sent, Archive, Junk, Trash), then the rest A–Z. Stalwart gives every folder sortOrder 0 until someone orders it, so an existing sidebar changes once, to that default. Dropping a folder on the top or bottom quarter of a row puts it above or below that row, with a line to show where it will land. Dropping on the middle still nests it. Special folders can now be dragged, to be reordered but never nested; on those, the whole row reorders by the nearer half. The folder menu gains Move up and Move down, for the keyboard and touch. Inbox stays first. A reorder numbers the level 10 apart and writes only the folders whose number changes, in one Mailbox/set. The order is saved on the server, so it follows the account to every device and to other JMAP clients. No new strings: Move up and Move down were already translated. Fixes #402 Co-authored-by: Claude Opus 5 (1M context) --- FEATURES.md | 13 +- .../lib/mailbox/__tests__/folderOrder.test.ts | 121 ++++++++++++++++++ web/src/lib/mailbox/folderOrder.ts | 101 +++++++++++++++ web/src/store/mail/index.ts | 17 ++- web/src/store/mail/types.ts | 2 + web/src/styles/app.css | 3 + web/src/views/mail/MailboxTree.tsx | 96 ++++++++++---- .../mail/__tests__/folder-reorder.test.tsx | 112 ++++++++++++++++ 8 files changed, 439 insertions(+), 26 deletions(-) create mode 100644 web/src/lib/mailbox/__tests__/folderOrder.test.ts create mode 100644 web/src/lib/mailbox/folderOrder.ts create mode 100644 web/src/views/mail/__tests__/folder-reorder.test.tsx diff --git a/FEATURES.md b/FEATURES.md index 3f1c2a1..35002f2 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -239,9 +239,16 @@ for that one, and the dialog says so. Real JMAP mailboxes, with the server's roles honored. - Create, rename, create a subfolder, delete (with or without its mail). -- **Drag a folder onto another** to reparent it. Folders with a server role - (Inbox, Sent, Drafts, Trash, Junk, Archive) are structural and are not - offered the drag, because the server refuses to move them anyway. +- **Order:** Inbox first, then the other special folders (Drafts, Sent, + Archive, Junk, Trash), then everything else A–Z, at every level. +- **Drag a folder between two others** to put it there. The line shows where + it will land. The order is saved on the server as the folders' JMAP + `sortOrder`, so it follows you to every device, and other clients that + honor `sortOrder` show it too. *Move up* and *Move down* in the folder menu + do the same from the keyboard or on touch. Inbox always stays first. +- **Drag a folder onto the middle of another** to reparent it. Folders with a + server role (Sent, Drafts, Trash, Junk, Archive) can be reordered but not + nested, because the server refuses to move them to another parent. - **Subscribe / unsubscribe** — *Show in list* / *Hide from list*. An unsubscribed folder still exists and still receives; it is just out of the way. Inbox cannot be hidden. diff --git a/web/src/lib/mailbox/__tests__/folderOrder.test.ts b/web/src/lib/mailbox/__tests__/folderOrder.test.ts new file mode 100644 index 0000000..c21cf0b --- /dev/null +++ b/web/src/lib/mailbox/__tests__/folderOrder.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "vitest"; +import { canPlaceFolder, compareFolders, neighbour, placeFolder, siblingsOf } from "../folderOrder"; +import type { Id, Mailbox } from "@/jmap/types"; + +const RIGHTS = { mayRename: true, mayCreateChild: true } as Mailbox["myRights"]; + +const mb = (id: string, name: string, parentId: string | null, role: Mailbox["role"] = null, sortOrder = 0, over: Partial = {}): Mailbox => + ({ id, name, parentId, role, sortOrder, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, isSubscribed: true, myRights: RIGHTS, ...over }); + +const tree = (...list: Mailbox[]): Record => Object.fromEntries(list.map((m) => [m.id, m])); + +/** As Stalwart hands it over before anybody orders anything: every sortOrder 0. */ +const fresh = tree( + mb("zeta", "Zeta", null), + mb("trash", "Deleted Items", null, "trash"), + mb("sent", "Sent Items", null, "sent"), + mb("inbox", "Inbox", null, "inbox"), + mb("alpha", "Alpha", null), + mb("junk", "Junk Mail", null, "junk"), + mb("drafts", "Drafts", null, "drafts"), + mb("work", "Work", null), + mb("clients", "Clients", "work"), +); + +const names = (all: Record, parentId: Id | null = null) => siblingsOf(all, parentId).map((m) => m.id); + +/** Apply what `placeFolder` asks for, as the server would. */ +function apply(all: Record, updates: Record> | null): Record { + const next = { ...all }; + for (const [id, patch] of Object.entries(updates ?? {})) next[id] = { ...next[id]!, ...patch }; + return next; +} + +describe("compareFolders", () => { + it("lists Inbox, then the special folders in mail-client order, then the rest A–Z, when nothing is ordered yet", () => { + // #402: Sent landed fourth from the bottom among the reporter's 88 folders. + expect(names(fresh)).toEqual(["inbox", "drafts", "sent", "junk", "trash", "alpha", "work", "zeta"]); + }); + + it("puts a saved order ahead of the special-folder default", () => { + const ordered = apply(fresh, { zeta: { sortOrder: 10 }, sent: { sortOrder: 20 }, alpha: { sortOrder: 30 }, drafts: { sortOrder: 40 }, junk: { sortOrder: 50 }, trash: { sortOrder: 60 }, work: { sortOrder: 70 } }); + expect(names(ordered)).toEqual(["inbox", "zeta", "sent", "alpha", "drafts", "junk", "trash", "work"]); + }); + + it("keeps Inbox first whatever its sortOrder says", () => { + const a = mb("inbox", "Inbox", null, "inbox", 99); + const b = mb("alpha", "Alpha", null, null, 1); + expect(compareFolders(a, b)).toBeLessThan(0); + }); + + it("sorts names numerically, not by character", () => { + const all = tree(mb("f10", "Folder 10", null), mb("f9", "Folder 9", null)); + expect(names(all)).toEqual(["f9", "f10"]); + }); +}); + +describe("placeFolder", () => { + it("numbers the whole level 10 apart, with the folder where it was dropped", () => { + const next = apply(fresh, placeFolder(fresh, "zeta", "drafts", "before")); + expect(names(next)).toEqual(["inbox", "zeta", "drafts", "sent", "junk", "trash", "alpha", "work"]); + expect(siblingsOf(next, null).map((m) => m.sortOrder)).toEqual([10, 20, 30, 40, 50, 60, 70, 80]); + }); + + it("writes only the folders whose number changes", () => { + const once = apply(fresh, placeFolder(fresh, "zeta", "drafts", "before")); + // Swapping the last two leaves everything above them where it was. + expect(Object.keys(placeFolder(once, "work", "alpha", "before")!).sort()).toEqual(["alpha", "work"]); + }); + + it("asks for nothing when the folder is dropped where it already is", () => { + expect(placeFolder(fresh, "sent", "drafts", "after")).toBeNull(); + expect(placeFolder(fresh, "sent", "junk", "before")).toBeNull(); + }); + + it("moves a folder to another level, and gives it a place there", () => { + const updates = placeFolder(fresh, "alpha", "clients", "before")!; + expect(updates.alpha).toEqual({ sortOrder: 10, parentId: "work" }); + expect(names(apply(fresh, updates), "work")).toEqual(["alpha", "clients"]); + }); +}); + +describe("canPlaceFolder", () => { + it("lets a special folder be reordered among its siblings", () => { + expect(canPlaceFolder(fresh, "sent", "alpha", "after")).toBe(true); + }); + + it("does not let a special folder move to another level", () => { + expect(canPlaceFolder(fresh, "sent", "clients", "before")).toBe(false); + }); + + it("puts nothing above Inbox", () => { + expect(canPlaceFolder(fresh, "sent", "inbox", "before")).toBe(false); + expect(canPlaceFolder(fresh, "sent", "inbox", "after")).toBe(true); + }); + + it("does not put a folder inside its own subtree", () => { + expect(canPlaceFolder(fresh, "work", "clients", "before")).toBe(false); + }); + + it("needs the right to rename, which RFC 8621 folds moving into", () => { + const locked = apply(fresh, { alpha: { myRights: { ...RIGHTS, mayRename: false } } }); + expect(canPlaceFolder(locked, "alpha", "zeta", "after")).toBe(false); + }); +}); + +describe("neighbour", () => { + it("steps past the folder above or below", () => { + expect(neighbour(fresh, "alpha", "up")).toEqual({ targetId: "trash", placement: "before" }); + expect(neighbour(fresh, "alpha", "down")).toEqual({ targetId: "work", placement: "after" }); + }); + + it("has nowhere to go past either end, or above Inbox", () => { + expect(neighbour(fresh, "zeta", "down")).toBeNull(); + expect(neighbour(fresh, "drafts", "up")).toBeNull(); + }); + + it("skips folders that aren't on screen, so every step visibly moves", () => { + const hidden = apply(fresh, { trash: { isSubscribed: false } }); + expect(neighbour(hidden, "alpha", "up", (m) => m.isSubscribed)).toEqual({ targetId: "junk", placement: "before" }); + }); +}); diff --git a/web/src/lib/mailbox/folderOrder.ts b/web/src/lib/mailbox/folderOrder.ts new file mode 100644 index 0000000..b70e05a --- /dev/null +++ b/web/src/lib/mailbox/folderOrder.ts @@ -0,0 +1,101 @@ +import type { Id, Mailbox } from "@/jmap/types"; +import { ROLE_ORDER } from "@/store/mail/mailboxes"; +import { canDropFolder, descendantIds } from "./folderMove"; + +/** + * The order folders are listed in, at every level of the tree (#402). + * + * Inbox always comes first. After that the folder's own `sortOrder` decides, + * which is where a folder dragged into place keeps its position, and where + * any other JMAP client that orders folders keeps its choice too. Stalwart + * gives every folder 0 until somebody orders it, so for everyone who never + * has, the tie-breaks decide: special folders first, in the usual mail-client + * order (Drafts, Sent, Archive, Junk, Trash), then the rest A–Z. + */ +export function compareFolders(a: Mailbox, b: Mailbox): number { + if ((a.role === "inbox") !== (b.role === "inbox")) return a.role === "inbox" ? -1 : 1; + if (a.sortOrder !== b.sortOrder) return a.sortOrder - b.sortOrder; + const ra = roleRank(a); + const rb = roleRank(b); + if (ra !== rb) return ra - rb; + return a.name.localeCompare(b.name, undefined, { sensitivity: "base", numeric: true }); +} + +function roleRank(m: Mailbox): number { + return m.role && m.role in ROLE_ORDER ? ROLE_ORDER[m.role]! : Number.MAX_SAFE_INTEGER; +} + +/** Every folder under `parentId` (null: the top level), in list order. */ +export function siblingsOf(mailboxes: Record, parentId: Id | null): Mailbox[] { + return Object.values(mailboxes) + .filter((m) => (m.parentId && mailboxes[m.parentId] ? m.parentId : null) === parentId) + .sort(compareFolders); +} + +export type Placement = "before" | "after"; + +/** + * Whether `draggedId` may be put just above or below `targetId`. + * + * Special folders can be reordered but not reparented, so they may only land + * among their own siblings. Nothing goes above Inbox, which stays first. + */ +export function canPlaceFolder(mailboxes: Record, draggedId: Id, targetId: Id, placement: Placement): boolean { + const dragged = mailboxes[draggedId]; + const target = mailboxes[targetId]; + if (!dragged || !target || draggedId === targetId) return false; + if (!dragged.myRights.mayRename) return false; + if (target.role === "inbox" && placement === "before") return false; + if (descendantIds(mailboxes, draggedId).has(targetId)) return false; + const from = parentOf(mailboxes, dragged); + const to = parentOf(mailboxes, target); + return from === to || canDropFolder(mailboxes, draggedId, to); +} + +/** + * The updates that put `draggedId` just above or below `targetId`, or null when + * it is already there. + * + * The new level is numbered afresh, 10 apart, so that another client can put + * a folder between two of them without renumbering. Only folders whose number + * actually changes are written. + */ +export function placeFolder(mailboxes: Record, draggedId: Id, targetId: Id, placement: Placement): Record> | null { + const dragged = mailboxes[draggedId]!; + const parentId = parentOf(mailboxes, mailboxes[targetId]!); + const reparent = parentOf(mailboxes, dragged) !== parentId; + const current = siblingsOf(mailboxes, parentId); + const order = current.filter((m) => m.id !== draggedId); + const at = order.findIndex((m) => m.id === targetId) + (placement === "after" ? 1 : 0); + order.splice(at, 0, dragged); + // Dropped where it already was. Renumbering would change nothing anyone sees. + if (!reparent && order.every((m, i) => m.id === current[i]!.id)) return null; + + const updates: Record> = {}; + order.forEach((m, i) => { + const sortOrder = (i + 1) * 10; + if (m.sortOrder !== sortOrder) updates[m.id] = { sortOrder }; + }); + if (reparent) updates[draggedId] = { ...updates[draggedId], parentId }; + return updates; +} + +/** + * The neighbour to place a folder against for "Move up" / "Move down", if it + * has one. Only folders on screen count (`shown`), so each step visibly moves + * the folder rather than passing a hidden one. + */ +export function neighbour(mailboxes: Record, id: Id, direction: "up" | "down", shown: (m: Mailbox) => boolean = () => true): { targetId: Id; placement: Placement } | null { + const m = mailboxes[id]; + if (!m) return null; + const level = siblingsOf(mailboxes, parentOf(mailboxes, m)).filter((x) => x.id === id || shown(x)); + const i = level.findIndex((x) => x.id === id); + const other = level[direction === "up" ? i - 1 : i + 1]; + if (!other) return null; + const placement = direction === "up" ? "before" : "after"; + return canPlaceFolder(mailboxes, id, other.id, placement) ? { targetId: other.id, placement } : null; +} + +function parentOf(mailboxes: Record, m: Mailbox): Id | null { + return m.parentId && mailboxes[m.parentId] ? m.parentId : null; +} diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index 78ace03..e51048d 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -29,6 +29,7 @@ import { plural, t } from "@/lib/i18n"; import { withBase } from "@/lib/basePath"; import { isDeviceTrusted, loadRaw, saveJson } from "@/lib/storage"; import { MAILBOX_PROPS, LIST_PROPS, FULL_PROPS, BODY_PROPS } from "./props"; +import { compareFolders } from "@/lib/mailbox/folderOrder"; import { type ListQuery, type MailState } from "./types"; import { playNewMailSound, showNotification } from "@/lib/notify/notify"; import { pushEnabledHere } from "@/lib/notify/webpush"; @@ -161,7 +162,7 @@ export const useMail = create((set, get) => ({ childrenOf(parentId) { return Object.values(get().mailboxes) .filter((m) => (m.parentId ?? null) === parentId) - .sort((a, b) => a.sortOrder - b.sortOrder || a.name.localeCompare(b.name)); + .sort(compareFolders); }, async query(q, opts = {}) { @@ -762,6 +763,20 @@ export const useMail = create((set, get) => ({ if (before.length) await followFolders(before); }, + async arrangeMailboxes(updates) { + const accountId = get().accountId!; + const moved = Object.keys(updates).filter((id) => updates[id]!.parentId !== undefined); + const before = moved.flatMap((id) => folderRefs(get(), id)); + // One request for the whole level rather than one per folder. JMAP applies + // each update on its own, so a refusal can leave the level part-numbered; + // reloading shows whatever order the server actually kept. + const res = await client.call("Mailbox/set", { accountId, update: updates }); + const failed = Object.values(res.notUpdated ?? {})[0]; + await get().loadMailboxes(); + if (failed) throw new Error(setErrorMessage(failed)); + if (before.length) await followFolders(before); + }, + async destroyMailbox(id, removeEmails = true) { const accountId = get().accountId!; const before = folderRefs(get(), id); diff --git a/web/src/store/mail/types.ts b/web/src/store/mail/types.ts index 1121d57..751a677 100644 --- a/web/src/store/mail/types.ts +++ b/web/src/store/mail/types.ts @@ -99,6 +99,8 @@ export interface MailState { /** Give something the Archive role -- adopting a folder already named for it, or making one. */ ensureArchiveFolder(): Promise; updateMailbox(id: Id, patch: Partial): Promise; + /** Several folders' `sortOrder` (and at most a new parent) in one request: a reorder from the tree. */ + arrangeMailboxes(updates: Record>): Promise; destroyMailbox(id: Id, removeEmails?: boolean): Promise; loadIdentities(): Promise; diff --git a/web/src/styles/app.css b/web/src/styles/app.css index c6db6e4..10d6c48 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -1310,6 +1310,9 @@ a.menu-item:hover { color: var(--fg); } .nav-item.active.unread .nav-label, .nav-item.active.unread .nav-count { color: inherit; } .nav-item.drop-target { background: var(--accent-soft); outline: 2px dashed var(--accent); outline-offset: -2px; } .nav-item.folder-row.dragging { opacity: .45; } +/* A folder dragged between two others: a line where it will land. */ +.nav-item.folder-row.drop-before { box-shadow: inset 0 2px 0 var(--accent); } +.nav-item.folder-row.drop-after { box-shadow: inset 0 -2px 0 var(--accent); } /* A folder color tints its icon; the label keeps the sidebar's contrast. */ .folder-row .folder-icon { display: inline-flex; align-items: center; } /* .nav-item svg sets color on the svg itself, so inheriting from the span is diff --git a/web/src/views/mail/MailboxTree.tsx b/web/src/views/mail/MailboxTree.tsx index 4e4b1ce..858532e 100644 --- a/web/src/views/mail/MailboxTree.tsx +++ b/web/src/views/mail/MailboxTree.tsx @@ -1,6 +1,6 @@ import { lazy, Suspense, useEffect, useMemo, useState, type DragEvent, type ReactNode } from "react"; import { Link, useLocation } from "wouter"; -import { AlertOctagon, Archive, ChevronDown, ChevronLeft, Clock, ChevronRight, File, Folder, FolderPlus, Inbox, Mail, MoreVertical, Palette, Send, Star, Tag, Trash2, Plus, Pencil, Eye, EyeOff, CheckCheck, Eraser, Share2, X, FolderInput } from "lucide-react"; +import { AlertOctagon, Archive, ArrowDown, ArrowUp, ChevronDown, ChevronLeft, Clock, ChevronRight, File, Folder, FolderPlus, Inbox, Mail, MoreVertical, Palette, Send, Star, Tag, Trash2, Plus, Pencil, Eye, EyeOff, CheckCheck, Eraser, Share2, X, FolderInput } from "lucide-react"; import { useMail } from "@/store/mail"; import { canEmpty, confirmAndEmpty, emptyLabel } from "@/lib/mailbox/emptyFolder"; import { labelTree, visibleLabels } from "@/lib/mailbox/labelTree"; @@ -14,6 +14,7 @@ import { toast } from "@/ui/toast"; import { MailboxPicker } from "./MailboxPicker"; import { loadRaw, saveJson } from "@/lib/storage"; import { canDropFolder, canMoveFolderTo, folderColor, movable } from "@/lib/mailbox/folderMove"; +import { canPlaceFolder, compareFolders, neighbour, placeFolder, type Placement } from "@/lib/mailbox/folderOrder"; import { haptic, useTouchRow } from "@/lib/input/touch"; import { plural, t } from "@/lib/i18n"; import { mailboxDisplayName } from "@/lib/mailbox/mailboxName"; @@ -78,8 +79,32 @@ export function MailboxTree() { } }; - // Tree: A–Z at every level (Inbox pinned to the top of the root), subfolders nested and - // collapsed by default. Expansion state is remembered per folder. + /** Whether the folder in flight may go just above or below this folder. */ + const canPlace = (targetId: Id, placement: Placement): boolean => Boolean(draggingId) && canPlaceFolder(mailboxes, draggingId!, targetId, placement); + + /** Put a folder just above or below another: a drag between rows, or Move up / Move down. */ + const placeFolderAt = async (id: Id, targetId: Id, placement: Placement) => { + setDraggingId(null); + const updates = placeFolder(mailboxes, id, targetId, placement); + if (!updates) return; + try { + await useMail.getState().arrangeMailboxes(updates); + const parentId = updates[id]?.parentId; + if (parentId) { + const next = { ...expanded, [parentId]: true }; + setExpanded(next); + saveJson("mbx-expanded", next); + } + } catch (err) { + toast.error(t("Could not move “{name}”: {reason}", { name: mailboxDisplayName(mailboxes[id]!), reason: (err as Error).message })); + } + }; + const shown = (m: Mailbox) => showHidden || m.isSubscribed || m.role === "inbox"; + + // Tree: in `compareFolders` order at every level (Inbox, then any order the + // user has dragged into place, then the special folders, then A–Z), + // subfolders nested and collapsed by default. Expansion state is remembered + // per folder. const [expanded, setExpanded] = useState>(() => loadRaw("mbx-expanded", {})); const toggle = (id: Id) => { const next = { ...expanded, [id]: !expanded[id] }; @@ -87,17 +112,13 @@ export function MailboxTree() { saveJson("mbx-expanded", next); }; const { rows, childrenOf, subtreeUnread } = useMemo(() => { - const all = Object.values(mailboxes).filter((m) => showHidden || m.isSubscribed || m.role === "inbox"); + const all = Object.values(mailboxes).filter(shown); const byParent = new Map(); for (const m of all) { const p = m.parentId && mailboxes[m.parentId] ? m.parentId : null; byParent.set(p, [...(byParent.get(p) ?? []), m]); } - const cmp = (a: Mailbox, b: Mailbox) => { - if ((a.role === "inbox") !== (b.role === "inbox")) return a.role === "inbox" ? -1 : 1; - return a.name.localeCompare(b.name, undefined, { sensitivity: "base", numeric: true }); - }; - for (const list of byParent.values()) list.sort(cmp); + for (const list of byParent.values()) list.sort(compareFolders); const out: Array<{ m: Mailbox; depth: number; hasChildren: boolean; open: boolean; hiddenUnread: number; childUnread: number }> = []; const unreadBelow = (id: Id): number => (byParent.get(id) ?? []).reduce((n, c) => n + c.unreadEmails + unreadBelow(c.id), 0); const walk = (parent: Id | null, depth: number) => { @@ -207,6 +228,8 @@ export function MailboxTree() { onFolderDragStart={() => {}} onFolderDragEnd={() => {}} onFolderDrop={() => {}} + canPlace={() => false} + onFolderPlace={() => {}} /> )} @@ -229,6 +252,8 @@ export function MailboxTree() { onFolderDragStart={() => setDraggingId(m.id)} onFolderDragEnd={() => { setDraggingId(null); setRootDrop(false); }} onFolderDrop={(id) => void moveFolder(id, m.id)} + canPlace={(placement) => canPlace(m.id, placement) && !(placement === "after" && open && hasChildren)} + onFolderPlace={(id, placement) => void placeFolderAt(id, m.id, placement)} /> ))} {/* Labels are a flat list that belongs to the mailbox, not to whichever @@ -260,7 +285,11 @@ export function MailboxTree() { )} - {menuTarget && void createFolder(menuTarget.id)} onShare={() => setShareTarget(menuTarget)} onMove={() => { menu.close(); setMoveTarget(menuTarget); }} />} + {menuTarget && void createFolder(menuTarget.id)} onShare={() => setShareTarget(menuTarget)} onMove={() => { menu.close(); setMoveTarget(menuTarget); }} onStep={(direction) => { + menu.close(); + const to = neighbour(mailboxes, menuTarget.id, direction, shown); + if (to) void placeFolderAt(menuTarget.id, to.targetId, to.placement); + }} canStep={(direction) => Boolean(neighbour(mailboxes, menuTarget.id, direction, shown))} />} {moveTarget && ( void; onDrillIn?: () => void; currentId?: string; onMenu: (m: Mailbox, e: { currentTarget: Element }) => void; dragging: boolean; acceptsFolder: boolean; onFolderDragStart: () => void; onFolderDragEnd: () => void; onFolderDrop: (id: Id) => void }) { - const [dropping, setDropping] = useState(false); +function FolderRow({ mailbox: m, label, depth, hasChildren, open, hiddenUnread, childUnread, onToggle, onDrillIn, currentId, onMenu, dragging, acceptsFolder, onFolderDragStart, onFolderDragEnd, onFolderDrop, canPlace, onFolderPlace }: { mailbox: Mailbox; label: string; depth: number; hasChildren: boolean; open: boolean; hiddenUnread: number; childUnread: number; onToggle: () => void; onDrillIn?: () => void; currentId?: string; onMenu: (m: Mailbox, e: { currentTarget: Element }) => void; dragging: boolean; acceptsFolder: boolean; onFolderDragStart: () => void; onFolderDragEnd: () => void; onFolderDrop: (id: Id) => void; canPlace: (placement: Placement) => boolean; onFolderPlace: (id: Id, placement: Placement) => void }) { + /** Where a drop here would land: in this folder, or just above or below it. */ + const [drop, setDrop] = useState<"into" | Placement | null>(null); /** Expanding in place and drilling in are the same relationship; only one shows. */ const twisty = hasChildren && !onDrillIn; // Scheduled counts like Drafts: everything in it is already read, so the @@ -297,19 +327,37 @@ function FolderRow({ mailbox: m, label, depth, hasChildren, open, hiddenUnread, // Subscribed, not read once: picking a color has to repaint the row. const tint = useSettings((s) => folderColor(s.settings.folderColors, m.id)); + /* + * A folder dropped on the top or bottom quarter of a row goes above or below + * it; anywhere else, into it. Where "into" isn't allowed -- a special folder, + * which can be reordered but never nested -- the whole row reorders, by + * whichever half the pointer is in. + */ + const folderZone = (e: DragEvent): "into" | Placement | null => { + const r = e.currentTarget.getBoundingClientRect(); + const y = e.clientY - r.top; + const edge = r.height / 4; + const zone = y < edge ? "before" : y > r.height - edge ? "after" : "into"; + if (zone !== "into" && canPlace(zone)) return zone; + if (acceptsFolder) return "into"; + const half = y < r.height / 2 ? "before" : "after"; + return canPlace(half) ? half : null; + }; const onDragOver = (e: DragEvent) => { - const folder = e.dataTransfer.types.includes(FOLDER_MIME); - if (folder ? !acceptsFolder : !e.dataTransfer.types.includes("application/x-ihasmail-emails")) return; + const zone = e.dataTransfer.types.includes(FOLDER_MIME) ? folderZone(e) : e.dataTransfer.types.includes("application/x-ihasmail-emails") ? "into" : null; + if (!zone) return; e.preventDefault(); e.dataTransfer.dropEffect = "move"; - if (!dropping) setDropping(true); + if (drop !== zone) setDrop(zone); }; const onDrop = (e: DragEvent) => { e.preventDefault(); - setDropping(false); + setDrop(null); const folderId = e.dataTransfer.getData(FOLDER_MIME); if (folderId) { - if (acceptsFolder) onFolderDrop(folderId); + const zone = folderZone(e); + if (zone === "into") onFolderDrop(folderId); + else if (zone) onFolderPlace(folderId, zone); return; } const raw = e.dataTransfer.getData("application/x-ihasmail-emails"); @@ -348,16 +396,17 @@ function FolderRow({ mailbox: m, label, depth, hasChildren, open, hiddenUnread, return ( setDropping(false)} + onDragLeave={() => setDrop(null)} onDrop={onDrop} onContextMenu={(e) => { e.preventDefault(); @@ -424,7 +473,7 @@ function FolderRow({ mailbox: m, label, depth, hasChildren, open, hiddenUnread, ); } -function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare, onMove }: { mailbox: Mailbox; onClose: () => void; onCreateChild: () => void; onShare: () => void; onMove: () => void }) { +function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare, onMove, onStep, canStep }: { mailbox: Mailbox; onClose: () => void; onCreateChild: () => void; onShare: () => void; onMove: () => void; onStep: (direction: "up" | "down") => void; canStep: (direction: "up" | "down") => boolean }) { const shared = Object.keys(m.shareWith ?? {}).length > 0; const [, navigate] = useLocation(); const colors = useSettings((s) => s.settings.folderColors); @@ -490,6 +539,9 @@ function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare, onMove }: { } label={t("New subfolder")} onClick={onCreateChild} disabled={!m.myRights.mayCreateChild} /> } label={t("Rename")} onClick={() => void rename()} disabled={isSpecial || !m.myRights.mayRename} /> } label={t("Move to…")} onClick={onMove} disabled={!movable(m) || !m.myRights.mayRename} /> + {/* The way to reorder without a drag: from the keyboard, and on touch. */} + } label={t("Move up")} onClick={() => onStep("up")} disabled={!canStep("up")} /> + } label={t("Move down")} onClick={() => onStep("down")} disabled={!canStep("down")} /> : } label={m.isSubscribed ? t("Hide from list") : t("Show in list")} onClick={() => void useMail.getState().updateMailbox(m.id, { isSubscribed: !m.isSubscribed })} disabled={m.role === "inbox"} /> {/* Sharing a mail folder is withdrawn, not removed: Stalwart accepts and stores the share, and it never reaches the other account -- its own diff --git a/web/src/views/mail/__tests__/folder-reorder.test.tsx b/web/src/views/mail/__tests__/folder-reorder.test.tsx new file mode 100644 index 0000000..01ad5b9 --- /dev/null +++ b/web/src/views/mail/__tests__/folder-reorder.test.tsx @@ -0,0 +1,112 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { MailboxTree } from "../MailboxTree"; +import { useMail } from "@/store/mail"; +import { useSettings } from "@/store/settings"; +import type { Mailbox, MailboxRole } from "@/jmap/types"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** + * Reordering folders in the sidebar (#402), driven through the real tree. + * + * The placement arithmetic has its own tests in lib/mailbox; these are about + * the row deciding where a drop lands from where the pointer is, which only + * the component knows. + */ + +window.matchMedia = ((q: string) => ({ matches: false, media: q, addEventListener() {}, removeEventListener() {} })) as unknown as typeof window.matchMedia; + +const rights = { mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true }; +const box = (id: string, name: string, parentId: string | null, role: MailboxRole = null): Mailbox => ({ + id, name, parentId, role, sortOrder: 0, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, myRights: rights, isSubscribed: true, +}); + +const MAILBOXES = { + zeta: box("zeta", "Zeta", null), + trash: box("trash", "Deleted Items", null, "trash"), + sent: box("sent", "Sent", null, "sent"), + inbox: box("inbox", "Inbox", null, "inbox"), + alpha: box("alpha", "Alpha", null), + drafts: box("drafts", "Drafts", null, "drafts"), +}; + +/** jsdom has no DataTransfer; this is the part of one the tree touches. */ +function transfer() { + const data: Record = {}; + return { + get types() { return Object.keys(data); }, + setData: (k: string, v: string) => { data[k] = v; }, + getData: (k: string) => data[k] ?? "", + effectAllowed: "", dropEffect: "", + }; +} + +function fire(el: Element, type: string, dataTransfer: ReturnType, clientY = 0) { + const e = new Event(type, { bubbles: true, cancelable: true }); + Object.assign(e, { dataTransfer, clientY }); + act(() => { el.dispatchEvent(e); }); +} + +describe("reordering folders in the tree", () => { + let host: HTMLDivElement; + let root: Root; + const arrange = vi.fn(async (_updates: Record>) => {}); + const updateMailbox = vi.fn(async (_id: string, _patch: Partial) => {}); + const rows = () => Array.from(document.querySelectorAll(".nav-item.folder-row")).map((r) => r.querySelector(".nav-label")?.textContent); + const rowFor = (name: string) => Array.from(document.querySelectorAll(".nav-item.folder-row")).find((r) => r.querySelector(".nav-label")?.textContent === name)!; + + /** Drag `from` over `to` at a fraction of its height, drop, and say what the row showed. */ + function drag(from: string, to: string, frac: number) { + const dt = transfer(); + const target = rowFor(to); + // Every row is 36px tall, from 100px down the page. + target.getBoundingClientRect = () => ({ top: 100, height: 36, bottom: 136, left: 0, right: 200, width: 200, x: 0, y: 100, toJSON() {} }); + fire(rowFor(from), "dragstart", dt); + fire(target, "dragover", dt, 100 + 36 * frac); + const shown = /drop-(before|after|target)/.exec(target.className)?.[1] ?? null; + fire(target, "drop", dt, 100 + 36 * frac); + return shown; + } + + beforeEach(() => { + arrange.mockClear(); + updateMailbox.mockClear(); + window.history.replaceState({}, "", "/mail/inbox"); + useMail.setState({ mailboxes: MAILBOXES, mailboxesLoaded: true, arrangeMailboxes: arrange, updateMailbox }); + useSettings.setState((s) => ({ settings: { ...s.settings, showHiddenFolders: false, labelsSidebar: false } })); + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + act(() => root.render()); + }); + afterEach(() => { act(() => root.unmount()); host.remove(); }); + + it("lists special folders under Inbox before the rest, until something is dragged", () => { + expect(rows()).toEqual(["Inbox", "Drafts", "Sent", "Deleted Items", "Alpha", "Zeta"]); + }); + + it("puts a folder above the row when it's dropped on the row's top edge", () => { + expect(drag("Zeta", "Drafts", 0.1)).toBe("before"); + expect(arrange).toHaveBeenCalledWith({ + zeta: { sortOrder: 20 }, drafts: { sortOrder: 30 }, sent: { sortOrder: 40 }, trash: { sortOrder: 50 }, alpha: { sortOrder: 60 }, inbox: { sortOrder: 10 }, + }); + }); + + it("nests a folder dropped on the middle of an ordinary folder, as before", () => { + expect(drag("Zeta", "Alpha", 0.5)).toBe("target"); + expect(updateMailbox).toHaveBeenCalledWith("zeta", { parentId: "alpha" }); + expect(arrange).not.toHaveBeenCalled(); + }); + + it("reorders a special folder by the nearer half, since it can't be nested", () => { + expect(drag("Deleted Items", "Drafts", 0.4)).toBe("before"); + expect(Object.keys(arrange.mock.calls[0]![0])).toContain("trash"); + }); + + it("drops nothing above Inbox", () => { + expect(drag("Sent", "Inbox", 0.1)).toBeNull(); + expect(arrange).not.toHaveBeenCalled(); + }); +});