Merge pull request #357 from Coffey-Labs/feat/move-folder-picker

Move a folder from its menu, with the same picker as moving mail

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<Mailbox["myRights"]>) => ({ mayRename: true, mayCreateChild: true, ...r }) as Mailbox["myRights"];
+  const owned: Record<Id, Mailbox> = 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" };

web/src/lib/folderMove.ts

@@ -51,6 +51,21 @@ export function canDropFolder(mailboxes: Record<Id, Mailbox>, 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, Mailbox>, 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<string, string>, id: Id): string | null {
   return colors[id] ?? null;

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",

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",

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",

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": "フォルダー名を変更",

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",

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",

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": "Переименовать папку",

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": "Перейменувати теку",

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": "重命名文件夹",

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])
+      .filter((m) => !exclude?.includes(m.id) && m.myRights[need] && (!allow || allow(m.id)))
-      .map((m) => ({ m, path: mailboxDisplayPath(m, mailboxes) }))
+      .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;
+    return ql ? rows.filter((x) => x.path.toLowerCase().includes(ql)) : rows;
-  }, [mailboxes, mailboxPath, q, exclude]);
+  }, [mailboxes, mailboxPath, q, exclude, need, allow, root, onPick]);
 
   return (
     <Dialog open onClose={onClose} title={title} size="sm">
@@ -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;
+            list[active]?.pick();
-            if (m) onPick(m.id);
           }
         }}
       />
       <div style={{ maxHeight: 360, overflowY: "auto", marginTop: 8 }} role="listbox">
-        {list.map(({ m, path }, i) => (
+        {list.map(({ m, path, pick }, i) => (
-          <PickerRow key={m.id} m={m} path={path} active={i === active} onClick={() => onPick(m.id)} onHover={() => setActive(i)} />
+          <PickerRow key={m?.id ?? ""} m={m} path={path} active={i === active} onClick={pick} onHover={() => setActive(i)} />
         ))}
         {!list.length && <div className="empty" style={{ padding: 24 }}>{t("No matching folders")}</div>}
       </div>
@@ -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 (
     <button className={`menu-item ${active ? "active" : ""}`} onClick={onClick} onMouseEnter={onHover} role="option" aria-selected={active}>
-      {m.role === "inbox" ? <Inbox size={16} /> : <Folder size={16} />}
+      {!m ? <FolderUp size={16} /> : m.role === "inbox" ? <Inbox size={16} /> : <Folder size={16} />}
       <span className="grow truncate">{path}</span>
-      <span className="menu-kbd">{m.totalEmails}</span>
+      {m && <span className="menu-kbd">{m.totalEmails}</span>}
     </button>
   );
 }

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<Mailbox | null>(null);
   const [shareTarget, setShareTarget] = useState<Mailbox | null>(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<Mailbox | null>(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() {
         )}
       </nav>
       <Popover anchor={menu.anchor} onClose={menu.close} width={300}>
-        {menuTarget && <MailboxMenu mailbox={menuTarget} onClose={menu.close} onCreateChild={() => void createFolder(menuTarget.id)} onShare={() => setShareTarget(menuTarget)} />}
+        {menuTarget && <MailboxMenu mailbox={menuTarget} onClose={menu.close} onCreateChild={() => void createFolder(menuTarget.id)} onShare={() => setShareTarget(menuTarget)} onMove={() => { menu.close(); setMoveTarget(menuTarget); }} />}
       </Popover>
+      {moveTarget && (
+        <MailboxPicker
+          title={t("Move “{name}” to…", { name: mailboxDisplayName(moveTarget) })}
+          need="mayReadItems"
+          allow={(id) => 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 && <ShareDialog kind="Mailbox" id={shareTarget.id} name={shareTarget.name} shareWith={shareTarget.shareWith ?? null} onClose={() => 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:
       )}
       <MenuItem icon={<FolderPlus size={16} />} label={t("New subfolder")} onClick={onCreateChild} disabled={!m.myRights.mayCreateChild} />
       <MenuItem icon={<Pencil size={16} />} label={t("Rename")} onClick={() => void rename()} disabled={isSpecial || !m.myRights.mayRename} />
+      <MenuItem icon={<FolderInput size={16} />} label={t("Move to…")} onClick={onMove} disabled={!movable(m) || !m.myRights.mayRename} />
       <MenuItem icon={m.isSubscribed ? <EyeOff size={16} /> : <Eye size={16} />} 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