From 0859936f2755893e92c03a23d5b5750cce6dcfd6 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Tue, 15 Sep 2026 07:00:12 -0700 Subject: [PATCH] Move a folder from its menu, with the same picker as moving mail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A folder could only be moved by dragging it, which is slow in a long list and not offered at all on a touch screen. Its menu now has "Move to…", which opens the searchable folder picker that moving messages already uses, with a "Top level" row above the folders. The picker lists only legal destinations: the same rules as a drop -- not into itself, its own subtree or the parent it already has -- plus the rights a picker has to check up front because it shows every folder at once: mayRename on the folder being moved, which RFC 8621 uses for reparenting, and mayCreateChild on the destination. Both only say no on shared mail. The move itself goes through the same path as a drop, so the toast and the expanded destination are unchanged. "Move “{name}” to…" and "Top level" are in all nine catalogues. Closes #355 --- web/src/lib/__tests__/folderMove.test.ts | 26 +++++++++++++++++++- web/src/lib/folderMove.ts | 15 ++++++++++++ web/src/locales/de.ts | 2 ++ web/src/locales/es.ts | 2 ++ web/src/locales/fr.ts | 2 ++ web/src/locales/ja.ts | 2 ++ web/src/locales/nl.ts | 2 ++ web/src/locales/pt-BR.ts | 2 ++ web/src/locales/ru.ts | 2 ++ web/src/locales/uk.ts | 2 ++ web/src/locales/zh-Hans.ts | 2 ++ web/src/views/mail/MailboxPicker.tsx | 31 ++++++++++++++---------- web/src/views/mail/MailboxTree.tsx | 25 ++++++++++++++++--- 13 files changed, 97 insertions(+), 18 deletions(-) diff --git a/web/src/lib/__tests__/folderMove.test.ts b/web/src/lib/__tests__/folderMove.test.ts index b679688..f4c6103 100644 --- a/web/src/lib/__tests__/folderMove.test.ts +++ b/web/src/lib/__tests__/folderMove.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { canDropFolder, descendantIds, folderColor, movable } from "../folderMove"; +import { canDropFolder, canMoveFolderTo, descendantIds, folderColor, movable } from "../folderMove"; import type { Id, Mailbox } from "@/jmap/types"; const mb = (id: string, name: string, parentId: string | null, role: Mailbox["role"] = null): Mailbox => @@ -68,6 +68,30 @@ describe("canDropFolder", () => { }); }); +describe("canMoveFolderTo", () => { + const rights = (r: Partial) => ({ mayRename: true, mayCreateChild: true, ...r }) as Mailbox["myRights"]; + const owned: Record = Object.fromEntries(Object.values(tree).map((m) => [m.id, { ...m, myRights: rights({}) }])); + + it("agrees with a drop when every right is granted", () => { + expect(canMoveFolderTo(owned, "news", "work")).toBe(true); + expect(canMoveFolderTo(owned, "eu", null)).toBe(true); + expect(canMoveFolderTo(owned, "work", "eu")).toBe(false); + expect(canMoveFolderTo(owned, "news", null)).toBe(false); + }); + + it("refuses a folder the user may not rename, top level included", () => { + const locked = { ...owned, eu: { ...owned.eu!, myRights: rights({ mayRename: false }) } }; + expect(canMoveFolderTo(locked, "eu", "news")).toBe(false); + expect(canMoveFolderTo(locked, "eu", null)).toBe(false); + }); + + it("refuses a destination that may not hold new subfolders", () => { + const closed = { ...owned, work: { ...owned.work!, myRights: rights({ mayCreateChild: false }) } }; + expect(canMoveFolderTo(closed, "news", "work")).toBe(false); + expect(canMoveFolderTo(closed, "eu", null)).toBe(true); + }); +}); + describe("folderColor", () => { it("returns the colour chosen for that folder, and null for the rest", () => { const colors = { work: "#7c3aed" }; diff --git a/web/src/lib/folderMove.ts b/web/src/lib/folderMove.ts index a40a289..99973da 100644 --- a/web/src/lib/folderMove.ts +++ b/web/src/lib/folderMove.ts @@ -51,6 +51,21 @@ export function canDropFolder(mailboxes: Record, draggedId: Id, tar return !descendantIds(mailboxes, draggedId).has(targetId); } +/** + * Whether `id` may be moved under `targetId` (null: the top level) from the + * folder picker. + * + * Stricter than a drop, because the picker lists every folder at once rather + * than letting the server refuse one drag: moving needs `mayRename` on the + * folder itself -- RFC 8621 folds reparenting into that right -- and + * `mayCreateChild` on the destination. Both only ever say no on shared mail. + */ +export function canMoveFolderTo(mailboxes: Record, id: Id, targetId: Id | null): boolean { + if (!mailboxes[id]?.myRights.mayRename) return false; + if (!canDropFolder(mailboxes, id, targetId)) return false; + return targetId === null || Boolean(mailboxes[targetId]?.myRights.mayCreateChild); +} + /** The colour chosen for a folder, if any. Ids are used, so a rename keeps it. */ export function folderColor(colors: Record, id: Id): string | null { return colors[id] ?? null; diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index 84d99ee..717a38e 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -1017,6 +1017,8 @@ export const catalog: Catalog = { "folder": "Ordner", "“{name}” moved into “{parent}”": "„{name}“ wurde nach „{parent}“ verschoben", "“{name}” moved to the top level": "„{name}“ wurde auf die oberste Ebene verschoben", + "Move “{name}” to…": "„{name}“ verschieben nach…", + "Top level": "Oberste Ebene", "Could not move “{name}”: {reason}": "„{name}“ konnte nicht verschoben werden: {reason}", "Delete “{name}”?": "„{name}“ löschen?", "Rename folder": "Ordner umbenennen", diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index 5395ada..7c810ae 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -937,6 +937,8 @@ export const catalog: Catalog = { "folder": "carpeta", "“{name}” moved into “{parent}”": "«{name}» se ha movido a «{parent}»", "“{name}” moved to the top level": "«{name}» se ha movido al nivel superior", + "Move “{name}” to…": "Mover «{name}» a…", + "Top level": "Nivel superior", "Could not move “{name}”: {reason}": "No se ha podido mover «{name}»: {reason}", "Delete “{name}”?": "¿Eliminar «{name}»?", "Rename folder": "Cambiar el nombre de la carpeta", diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index 42f2f10..e1649d3 100644 --- a/web/src/locales/fr.ts +++ b/web/src/locales/fr.ts @@ -942,6 +942,8 @@ export const catalog: Catalog = { "folder": "dossier", "“{name}” moved into “{parent}”": "« {name} » a été déplacé dans « {parent} »", "“{name}” moved to the top level": "« {name} » a été déplacé au niveau supérieur", + "Move “{name}” to…": "Déplacer « {name} » vers…", + "Top level": "Niveau supérieur", "Could not move “{name}”: {reason}": "Impossible de déplacer « {name} » : {reason}", "Delete “{name}”?": "Supprimer « {name} » ?", "Rename folder": "Renommer le dossier", diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index ba0ebac..cc0e599 100644 --- a/web/src/locales/ja.ts +++ b/web/src/locales/ja.ts @@ -998,6 +998,8 @@ export const catalog: Catalog = { "folder": "フォルダー", "“{name}” moved into “{parent}”": "「{name}」を「{parent}」に移動しました", "“{name}” moved to the top level": "「{name}」を最上位に移動しました", + "Move “{name}” to…": "「{name}」の移動先…", + "Top level": "最上位", "Could not move “{name}”: {reason}": "「{name}」を移動できませんでした: {reason}", "Delete “{name}”?": "「{name}」を削除しますか？", "Rename folder": "フォルダー名を変更", diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index 111e7c6..3437f22 100644 --- a/web/src/locales/nl.ts +++ b/web/src/locales/nl.ts @@ -933,6 +933,8 @@ export const catalog: Catalog = { "folder": "map", "“{name}” moved into “{parent}”": "“{name}” is verplaatst naar “{parent}”", "“{name}” moved to the top level": "“{name}” is naar het hoogste niveau verplaatst", + "Move “{name}” to…": "“{name}” verplaatsen naar…", + "Top level": "Hoogste niveau", "Could not move “{name}”: {reason}": "Kon “{name}” niet verplaatsen: {reason}", "Delete “{name}”?": "“{name}” verwijderen?", "Rename folder": "Map hernoemen", diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index d9e2118..934f09e 100644 --- a/web/src/locales/pt-BR.ts +++ b/web/src/locales/pt-BR.ts @@ -940,6 +940,8 @@ export const catalog: Catalog = { "folder": "pasta", "“{name}” moved into “{parent}”": "“{name}” foi movida para “{parent}”", "“{name}” moved to the top level": "“{name}” foi movida para o nível superior", + "Move “{name}” to…": "Mover “{name}” para…", + "Top level": "Nível superior", "Could not move “{name}”: {reason}": "Não foi possível mover “{name}”: {reason}", "Delete “{name}”?": "Excluir “{name}”?", "Rename folder": "Renomear a pasta", diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index 8e8597d..a5915da 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -939,6 +939,8 @@ export const catalog: Catalog = { "folder": "папка", "“{name}” moved into “{parent}”": "«{name}» перемещена в «{parent}»", "“{name}” moved to the top level": "«{name}» перемещена на верхний уровень", + "Move “{name}” to…": "Переместить «{name}» в…", + "Top level": "Верхний уровень", "Could not move “{name}”: {reason}": "Не удалось переместить «{name}»: {reason}", "Delete “{name}”?": "Удалить «{name}»?", "Rename folder": "Переименовать папку", diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index bfd5388..e5669ef 100644 --- a/web/src/locales/uk.ts +++ b/web/src/locales/uk.ts @@ -933,6 +933,8 @@ export const catalog: Catalog = { "folder": "тека", "“{name}” moved into “{parent}”": "«{name}» переміщено до «{parent}»", "“{name}” moved to the top level": "«{name}» переміщено на верхній рівень", + "Move “{name}” to…": "Перемістити «{name}» до…", + "Top level": "Верхній рівень", "Could not move “{name}”: {reason}": "Не вдалося перемістити «{name}»: {reason}", "Delete “{name}”?": "Видалити «{name}»?", "Rename folder": "Перейменувати теку", diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index 5a77111..ec1d87f 100644 --- a/web/src/locales/zh-Hans.ts +++ b/web/src/locales/zh-Hans.ts @@ -997,6 +997,8 @@ export const catalog: Catalog = { "folder": "文件夹", "“{name}” moved into “{parent}”": "「{name}」已移入「{parent}」", "“{name}” moved to the top level": "「{name}」已移至顶层", + "Move “{name}” to…": "将「{name}」移动到…", + "Top level": "顶层", "Could not move “{name}”: {reason}": "无法移动「{name}」：{reason}", "Delete “{name}”?": "删除「{name}」？", "Rename folder": "重命名文件夹", diff --git a/web/src/views/mail/MailboxPicker.tsx b/web/src/views/mail/MailboxPicker.tsx index 2e7385c..dc2b91d 100644 --- a/web/src/views/mail/MailboxPicker.tsx +++ b/web/src/views/mail/MailboxPicker.tsx @@ -1,5 +1,5 @@ import { useMemo, useState } from "react"; -import { Folder, Inbox } from "lucide-react"; +import { Folder, FolderUp, Inbox } from "lucide-react"; import { useMail } from "@/store/mail"; import { Dialog } from "@/ui/dialog"; import type { Id, Mailbox } from "@/jmap/types"; @@ -13,20 +13,25 @@ import { mailboxDisplayPath } from "@/lib/mailboxName"; * folder you may read but not write to is still somewhere you can go. The * distinction only shows up on shared mail, which is exactly where getting * it wrong would be invisible to whoever wrote the code. + * @param allow a further test a folder has to pass, for when a right alone + * does not settle it -- a folder cannot move into its own subtree. + * @param root a "top level" row above the folders, for the one kind of move + * that has somewhere to go which is not a folder. */ -export function MailboxPicker({ title, onClose, onPick, exclude, need = "mayAddItems" }: { title: string; onClose: () => void; onPick: (id: Id) => void; exclude?: Id[]; need?: "mayAddItems" | "mayReadItems" }) { +export function MailboxPicker({ title, onClose, onPick, exclude, need = "mayAddItems", allow, root }: { title: string; onClose: () => void; onPick: (id: Id) => void; exclude?: Id[]; need?: "mayAddItems" | "mayReadItems"; allow?: (id: Id) => boolean; root?: { label: string; onPick: () => void } }) { const mailboxes = useMail((s) => s.mailboxes); const mailboxPath = useMail((s) => s.mailboxPath); const [q, setQ] = useState(""); const [active, setActive] = useState(0); const list = useMemo(() => { const all = Object.values(mailboxes) - .filter((m) => !exclude?.includes(m.id) && m.myRights[need]) - .map((m) => ({ m, path: mailboxDisplayPath(m, 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))); + 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 ? all.filter((x) => x.path.toLowerCase().includes(ql)) : all; - }, [mailboxes, mailboxPath, q, exclude]); + return ql ? rows.filter((x) => x.path.toLowerCase().includes(ql)) : rows; + }, [mailboxes, mailboxPath, q, exclude, need, allow, root, onPick]); return ( @@ -48,14 +53,13 @@ export function MailboxPicker({ title, onClose, onPick, exclude, need = "mayAddI setActive((a) => Math.max(0, a - 1)); } else if (e.key === "Enter") { e.preventDefault(); - const m = list[active]?.m; - if (m) onPick(m.id); + list[active]?.pick(); } }} />
- {list.map(({ m, path }, i) => ( - onPick(m.id)} onHover={() => setActive(i)} /> + {list.map(({ m, path, pick }, i) => ( + setActive(i)} /> ))} {!list.length &&
{t("No matching folders")}
}
@@ -63,12 +67,13 @@ export function MailboxPicker({ title, onClose, onPick, exclude, need = "mayAddI ); } -function PickerRow({ m, path, active, onClick, onHover }: { m: Mailbox; path: string; active: boolean; onClick: () => void; onHover: () => void }) { +/** `m` is null for the top-level row, which has no icon of its own and nothing to count. */ +function PickerRow({ m, path, active, onClick, onHover }: { m: Mailbox | null; path: string; active: boolean; onClick: () => void; onHover: () => void }) { return ( ); } diff --git a/web/src/views/mail/MailboxTree.tsx b/web/src/views/mail/MailboxTree.tsx index 3ba70ca..0598627 100644 --- a/web/src/views/mail/MailboxTree.tsx +++ b/web/src/views/mail/MailboxTree.tsx @@ -1,6 +1,6 @@ import { 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 } from "lucide-react"; +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 { useMail } from "@/store/mail"; import { canEmpty, confirmAndEmpty, emptyLabel } from "@/lib/emptyFolder"; import { labelTree, visibleLabels } from "@/lib/labelTree"; @@ -12,8 +12,9 @@ import { CALENDAR_COLORS, useIsMobile, useIsTouch } from "@/ui/misc"; import { confirmDialog, promptDialog } from "@/ui/dialog"; import { toast } from "@/ui/toast"; import { ShareDialog } from "../settings/ShareDialog"; +import { MailboxPicker } from "./MailboxPicker"; import { loadRaw, saveJson } from "@/lib/storage"; -import { canDropFolder, folderColor, movable } from "@/lib/folderMove"; +import { canDropFolder, canMoveFolderTo, folderColor, movable } from "@/lib/folderMove"; import { haptic, useTouchRow } from "@/lib/touch"; import { plural, t } from "@/lib/i18n"; import { mailboxDisplayName } from "@/lib/mailboxName"; @@ -46,6 +47,8 @@ export function MailboxTree() { const menu = useMenu(); const [menuTarget, setMenuTarget] = useState(null); const [shareTarget, setShareTarget] = useState(null); + /** The folder being moved from its menu -- the way to move one without a drag, and on touch the only way. */ + const [moveTarget, setMoveTarget] = useState(null); /** * The folder being dragged. Held here rather than read from the drag itself: * dataTransfer.getData is blocked during dragover, so a row cannot ask what @@ -255,8 +258,21 @@ export function MailboxTree() { )} - {menuTarget && void createFolder(menuTarget.id)} onShare={() => setShareTarget(menuTarget)} />} + {menuTarget && void createFolder(menuTarget.id)} onShare={() => setShareTarget(menuTarget)} onMove={() => { menu.close(); setMoveTarget(menuTarget); }} />} + {moveTarget && ( + canMoveFolderTo(mailboxes, moveTarget.id, id)} + root={canMoveFolderTo(mailboxes, moveTarget.id, null) ? { label: t("Top level"), onPick: () => { setMoveTarget(null); void moveFolder(moveTarget.id, null); } } : undefined} + onClose={() => setMoveTarget(null)} + onPick={(id) => { + setMoveTarget(null); + void moveFolder(moveTarget.id, id); + }} + /> + )} {shareTarget && setShareTarget(null)} />} ); @@ -406,7 +422,7 @@ function FolderRow({ mailbox: m, label, depth, hasChildren, open, hiddenUnread, ); } -function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare }: { mailbox: Mailbox; onClose: () => void; onCreateChild: () => void; onShare: () => void }) { +function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare, onMove }: { mailbox: Mailbox; onClose: () => void; onCreateChild: () => void; onShare: () => void; onMove: () => void }) { const shared = Object.keys(m.shareWith ?? {}).length > 0; const [, navigate] = useLocation(); const colors = useSettings((s) => s.settings.folderColors); @@ -471,6 +487,7 @@ function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare }: { mailbox: )} } 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} /> : } 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