diff --git a/web/src/lib/mailbox/__tests__/folderOrder.test.ts b/web/src/lib/mailbox/__tests__/folderOrder.test.ts index c21cf0b..faee3bf 100644 --- a/web/src/lib/mailbox/__tests__/folderOrder.test.ts +++ b/web/src/lib/mailbox/__tests__/folderOrder.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { canPlaceFolder, compareFolders, neighbour, placeFolder, siblingsOf } from "../folderOrder"; +import { canPlaceFolder, compareFolders, neighbour, placeFolder, siblingsOf, treeOrder } from "../folderOrder"; import type { Id, Mailbox } from "@/jmap/types"; const RIGHTS = { mayRename: true, mayCreateChild: true } as Mailbox["myRights"]; @@ -119,3 +119,23 @@ describe("neighbour", () => { expect(neighbour(hidden, "alpha", "up", (m) => m.isSubscribed)).toEqual({ targetId: "junk", placement: "before" }); }); }); + +describe("treeOrder", () => { + const ids = (all: Record) => treeOrder(all).map((m) => m.id); + + it("lists the tree the way the sidebar does, each folder followed by its subfolders", () => { + expect(ids(fresh)).toEqual(["inbox", "drafts", "sent", "junk", "trash", "alpha", "work", "clients", "zeta"]); + }); + + it("follows a saved order rather than A–Z", () => { + // #1 on GitLab: the move-to picker kept the old order after the sidebar changed. + 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(ids(ordered)).toEqual(["inbox", "zeta", "sent", "alpha", "drafts", "junk", "trash", "work", "clients"]); + }); + + it("still lists a folder the walk from the top can't reach", () => { + const looped = apply(fresh, { work: { parentId: "clients" } }); + expect(ids(looped)).toHaveLength(Object.keys(looped).length); + expect(ids(looped)).toEqual(expect.arrayContaining(["work", "clients"])); + }); +}); diff --git a/web/src/lib/mailbox/folderOrder.ts b/web/src/lib/mailbox/folderOrder.ts index b70e05a..46b9b3a 100644 --- a/web/src/lib/mailbox/folderOrder.ts +++ b/web/src/lib/mailbox/folderOrder.ts @@ -25,6 +25,37 @@ function roleRank(m: Mailbox): number { return m.role && m.role in ROLE_ORDER ? ROLE_ORDER[m.role]! : Number.MAX_SAFE_INTEGER; } +/** + * Every folder, parents before their children and siblings in + * `compareFolders` order: the sidebar's order with every folder expanded. + * Lists that show all folders at once, like the move-to picker, use this so a + * folder sits where the user dragged it rather than where A–Z would put it. + * + * A folder the walk from the top never reaches (a parent loop the server + * should not allow) is appended rather than dropped, so it can still be + * picked. + */ +export function treeOrder(mailboxes: Record): Mailbox[] { + const byParent = new Map(); + for (const m of Object.values(mailboxes)) { + const p = m.parentId && mailboxes[m.parentId] ? m.parentId : null; + byParent.set(p, [...(byParent.get(p) ?? []), m]); + } + for (const list of byParent.values()) list.sort(compareFolders); + const out: Mailbox[] = []; + const seen = new Set(); + const walk = (parent: Id | null) => { + for (const m of byParent.get(parent) ?? []) { + if (seen.has(m.id)) continue; + seen.add(m.id); + out.push(m); + walk(m.id); + } + }; + walk(null); + return out.concat(Object.values(mailboxes).filter((m) => !seen.has(m.id)).sort(compareFolders)); +} + /** Every folder under `parentId` (null: the top level), in list order. */ export function siblingsOf(mailboxes: Record, parentId: Id | null): Mailbox[] { return Object.values(mailboxes) diff --git a/web/src/views/mail/MailboxPicker.tsx b/web/src/views/mail/MailboxPicker.tsx index 43eece7..41ffeda 100644 --- a/web/src/views/mail/MailboxPicker.tsx +++ b/web/src/views/mail/MailboxPicker.tsx @@ -5,6 +5,7 @@ import { Dialog } from "@/ui/dialog"; import type { Id, Mailbox } from "@/jmap/types"; import { t } from "@/lib/i18n"; import { mailboxDisplayPath } from "@/lib/mailbox/mailboxName"; +import { treeOrder } from "@/lib/mailbox/folderOrder"; /** * @param need which right a folder has to grant to be worth offering. @@ -24,10 +25,11 @@ export function MailboxPicker({ title, onClose, onPick, exclude, need = "mayAddI const [q, setQ] = useState(""); const [active, setActive] = useState(0); const list = useMemo(() => { - const all = Object.values(mailboxes) + // The sidebar's order, not A–Z by path: a folder dragged into place has to + // be found in the same place here. + const all = treeOrder(mailboxes) .filter((m) => !exclude?.includes(m.id) && m.myRights[need] && (!allow || allow(m.id))) - .map((m) => ({ m, path: mailboxDisplayPath(m, mailboxes), pick: () => onPick(m.id) })) - .sort((a, b) => (a.m.role === "inbox" ? -1 : b.m.role === "inbox" ? 1 : a.path.localeCompare(b.path))); + .map((m) => ({ m, path: mailboxDisplayPath(m, mailboxes), pick: () => onPick(m.id) })); const rows: { m: Mailbox | null; path: string; pick: () => void }[] = root ? [{ m: null, path: root.label, pick: root.onPick }, ...all] : all; const ql = q.trim().toLowerCase(); return ql ? rows.filter((x) => x.path.toLowerCase().includes(ql)) : rows; diff --git a/web/src/views/mail/__tests__/move-picker-order.test.tsx b/web/src/views/mail/__tests__/move-picker-order.test.tsx new file mode 100644 index 0000000..6d00d78 --- /dev/null +++ b/web/src/views/mail/__tests__/move-picker-order.test.tsx @@ -0,0 +1,62 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { MailboxPicker } from "../MailboxPicker"; +import { useMail } from "@/store/mail"; +import type { Mailbox, MailboxRole } from "@/jmap/types"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** + * The move-to picker (v) lists folders in the sidebar's order (#1 on GitLab). + * + * It used to sort A–Z by path, so a folder dragged into place in the sidebar + * turned up somewhere else here. The ordering has its own tests in + * lib/mailbox; these check what the dialog actually shows. + */ + +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, sortOrder = 0): Mailbox => ({ + id, name, parentId, role, sortOrder, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, myRights: rights, isSubscribed: true, +}); + +/** Ordered by hand in the sidebar: Zeta dragged to the top, Alpha to the bottom. */ +const MAILBOXES = { + inbox: box("inbox", "Inbox", null, "inbox", 10), + zeta: box("zeta", "Zeta", null, null, 20), + sent: box("sent", "Sent", null, "sent", 30), + work: box("work", "Work", null, null, 40), + clients: box("clients", "Clients", "work"), + trash: box("trash", "Deleted Items", null, "trash", 50), + alpha: box("alpha", "Alpha", null, null, 60), +}; + +describe("the move-to picker", () => { + let host: HTMLDivElement; + let root: Root; + const rows = () => Array.from(document.querySelectorAll('[role="option"]')).map((r) => r.querySelector(".grow")?.textContent); + + function open(props: Partial[0]> = {}) { + act(() => root.render( {}} onPick={() => {}} {...props} />)); + } + + beforeEach(() => { + useMail.setState({ mailboxes: MAILBOXES, mailboxesLoaded: true }); + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + }); + afterEach(() => { act(() => root.unmount()); host.remove(); }); + + it("lists folders in the order they were dragged into, not A–Z", () => { + open(); + expect(rows()).toEqual(["Inbox", "Zeta", "Sent", "Work", "Work / Clients", "Deleted Items", "Alpha"]); + }); + + it("keeps that order for the folders left after excluding one", () => { + open({ exclude: ["work"] }); + expect(rows()).toEqual(["Inbox", "Zeta", "Sent", "Work / Clients", "Deleted Items", "Alpha"]); + }); +});