From 26aef327860cf672d33ade68ce8a944e7b7b22c2 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 28 Oct 2026 04:57:25 +0100 Subject: [PATCH] fix: show the same actions in the same order in every menu of a contact, group or folder --- .../__tests__/contacts-menus.test.tsx | 52 +++++++ components/contacts/contact-context-menu.tsx | 56 ++++--- components/contacts/contact-detail.tsx | 138 +++++------------- components/contacts/contact-group-detail.tsx | 28 ++-- components/contacts/contact-list.tsx | 3 + components/contacts/contacts-app.tsx | 1 + .../file-browser-shared-folders.test.tsx | 13 ++ components/files/file-browser.tsx | 22 +-- components/files/files-sidebar.tsx | 11 +- 9 files changed, 172 insertions(+), 152 deletions(-) create mode 100644 components/contacts/__tests__/contacts-menus.test.tsx diff --git a/components/contacts/__tests__/contacts-menus.test.tsx b/components/contacts/__tests__/contacts-menus.test.tsx new file mode 100644 index 000000000..bd7158564 --- /dev/null +++ b/components/contacts/__tests__/contacts-menus.test.tsx @@ -0,0 +1,52 @@ +import { fireEvent, render, screen, within } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { ContactDetail } from '../contact-detail'; +import { ContactGroupDetail } from '../contact-group-detail'; +import { ContactList } from '../contact-list'; +import type { ContactCard } from '@/lib/jmap/types'; + +vi.mock('@/components/contacts/contact-activity', () => ({ ContactActivity: () => null })); + +const alice: ContactCard = { + id: '1', + addressBookIds: {}, + name: { components: [{ kind: 'given', value: 'Alice' }], isOrdered: true }, + emails: { e0: { address: 'alice@example.com' } }, +}; + +const menuLabels = () => within(screen.getByRole('menu')).getAllByRole('menuitem').map((i) => i.textContent); + +describe('one action list per contact (R6)', () => { + it('offers Duplicate in the right-click menu as in the detail ⋮', () => { + const noop = vi.fn(); + render( + , + ); + fireEvent.contextMenu(screen.getByText('Alice')); + const rightClick = menuLabels(); + expect(rightClick).toContain('context_menu.duplicate'); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Escape' }); + + render(); + fireEvent.click(screen.getByRole('button', { name: 'detail.more_actions' })); + const detail = menuLabels(); + // The detail's ⋮ is the same list, without what the page has as buttons. + expect(detail).toEqual(rightClick.filter((l) => !['context_menu.open', 'context_menu.send_email', 'context_menu.edit'].includes(l ?? ''))); + }); + + it('says "Edit group" on a group page and keeps Delete in a ⋮ like a contact page', () => { + const group = { id: 'g', kind: 'group', addressBookIds: {}, name: { full: 'Team' } } as ContactCard; + const onDelete = vi.fn(); + render(); + expect(screen.getByRole('button', { name: /groups.edit/ })).toBeInTheDocument(); + expect(screen.queryByText('form.edit_title')).toBeNull(); + fireEvent.click(screen.getByRole('button', { name: 'detail.more_actions' })); + fireEvent.click(screen.getByText('groups.delete')); + expect(onDelete).toHaveBeenCalled(); + }); +}); diff --git a/components/contacts/contact-context-menu.tsx b/components/contacts/contact-context-menu.tsx index db1f2058e..3cc1707d6 100644 --- a/components/contacts/contact-context-menu.tsx +++ b/components/contacts/contact-context-menu.tsx @@ -45,12 +45,16 @@ interface ContactContextMenuProps { menuRef: React.RefObject; isMultiSelect?: boolean; selectedCount?: number; - onOpen: () => void; + /** Left out where the contact is already open (the detail's ⋮). */ + onOpen?: () => void; /** Touch: start selecting with this contact (there is no Ctrl+click). */ onSelect?: () => void; - onEdit: () => void; + /** Left out where Edit is a button next to the menu. */ + onEdit?: () => void; onDelete: () => void; - onAddToGroup: () => void; + onAddToGroup?: () => void; + /** Leave out what the detail shows as buttons of its own. */ + inDetail?: boolean; /** Writes to the contact in the app's composer. */ onCompose?: () => void; onDuplicate?: () => void; @@ -79,6 +83,7 @@ export function ContactContextMenu({ onDuplicate, onMoveTo, onBatchMoveTo, + inDetail = false, onBatchExport, onBatchAddToGroup, onBatchDelete, @@ -155,24 +160,43 @@ export function ContactContextMenu({ return ( - + {/* R6 order: Open, write, Edit, then the rest; the detail's ⋮ leaves + out what it has as buttons (Edit, Send email, Call). */} + {onOpen && } {onSelect && } - - {(email || phone) && } - {email && onCompose && ( + {email && onCompose && !inDetail && ( )} - {phone && ( + {phone && !inDetail && ( )} + {onEdit && } + {!inDetail && } + {onAddToGroup && ( + + )} + {onDuplicate && ( + + )} + {onMoveTo && ( + + )} {email && ( handleCopy(phone))} /> )} - - - {onDuplicate && ( - - )} - {onMoveTo && ( - - )} ; - label: string; - onClick: () => void; - destructive?: boolean; - disabled?: boolean; - separator?: false; - } - | { separator: true }; - interface ContactDetailProps { contact: ContactCard | null; onEdit: () => void; @@ -85,12 +74,13 @@ function getCompletedYears(dateInput: AnniversaryDate): number | null { export function ContactDetail({ contact, onEdit, onDelete, onAddToGroup, onDuplicate, onMoveTo, onCompose, isMobile, className }: ContactDetailProps) { const t = useTranslations("contacts"); const locale = useLocale(); + // The ⋮ shows the contact's menu, the one right-click shows in the list (R6). + const moreMenu = useContextMenu(); const contextLabel = (key: string) => (CONTEXT_LABEL_KEYS[key] ? t(CONTEXT_LABEL_KEYS[key]) : key); const featureLabel = (key: string) => (PHONE_FEATURE_LABEL_KEYS[key] ? t(PHONE_FEATURE_LABEL_KEYS[key]) : key); const cryptoKeys = contact?.cryptoKeys ? Object.values(contact.cryptoKeys) : []; const hasCryptoKeysSlotOffers = usePluginSlotOffers('contact-cryptokeys').length > 0; - const exporting = useContactExportBusy(); const pluginContactProps = useMemo(() => (contact ? { contact: contactToView(contact) } : null), [contact]); // Tell plugins once per contact shown here, not on every edit of that card. @@ -117,28 +107,6 @@ export function ContactDetail({ contact, onEdit, onDelete, onAddToGroup, onDupli const photo = getContactPhotoRef(contact) ?? null; const phone = contact.phones ? Object.values(contact.phones)[0]?.number : undefined; - const handleExport = async () => { - if (await exportContact(contact)) toast.success(t("export.success", { count: 1 })); - }; - - const handlePrint = () => { - void printContact(contact, printLabels(t, locale), name); - }; - - const moreItems: MoreItem[] = []; - if (onAddToGroup) { - moreItems.push({ icon: Users, label: t("context_menu.add_to_group"), onClick: onAddToGroup }); - } - if (onDuplicate) { - moreItems.push({ icon: Copy, label: t("context_menu.duplicate"), onClick: onDuplicate }); - } - if (onMoveTo) { - moreItems.push({ icon: FolderInput, label: t("address_books.move_to"), onClick: onMoveTo }); - } - moreItems.push({ icon: Download, label: t("context_menu.export_vcard"), onClick: handleExport, disabled: exporting }); - moreItems.push({ icon: Printer, label: t("context_menu.print"), onClick: handlePrint }); - moreItems.push({ separator: true }); - moreItems.push({ icon: Trash2, label: t("context_menu.delete"), onClick: onDelete, destructive: true }); const emails = contact.emails ? Object.values(contact.emails) : []; const phones = contact.phones ? Object.values(contact.phones) : []; const orgs = contact.organizations ? Object.values(contact.organizations) : []; @@ -206,7 +174,32 @@ export function ContactDetail({ contact, onEdit, onDelete, onAddToGroup, onDupli {t("form.edit_title")} - + moreMenu.openContextMenuFrom(e.currentTarget, contact, "end")} + className="touch-manipulation" + > + + + {moreMenu.contextMenu.data && ( + + )} @@ -529,71 +522,6 @@ function RowActions({ children }: { children: React.ReactNode }) { ); } -function MoreActionsMenu({ items, label }: { items: MoreItem[]; label: string }) { - const [open, setOpen] = useState(false); - const ref = useRef(null); - - useEffect(() => { - if (!open) return; - const onDocMouseDown = (e: MouseEvent) => { - if (!ref.current?.contains(e.target as Node)) setOpen(false); - }; - const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape") setOpen(false); - }; - document.addEventListener("mousedown", onDocMouseDown); - document.addEventListener("keydown", onKey); - return () => { - document.removeEventListener("mousedown", onDocMouseDown); - document.removeEventListener("keydown", onKey); - }; - }, [open]); - - if (items.length === 0) return null; - - return ( -
- - {open && ( -
- {items.map((item, i) => { - if (item.separator) { - return ; - } - return ( - { - item.onClick(); - setOpen(false); - }} - /> - ); - })} -
- )} -
- ); -} - function CopyButton({ value, label, successMsg, failMsg, className }: { value: string; label: string; successMsg: string; failMsg: string; className?: string }) { return ( - + + + + { menu.closeContextMenu(); onEdit(); }} /> + + { menu.closeContextMenu(); onDelete(); }} /> + {onComposeGroup && hasEmailMembers && ( diff --git a/components/contacts/contact-list.tsx b/components/contacts/contact-list.tsx index e8d10c767..a090edd81 100644 --- a/components/contacts/contact-list.tsx +++ b/components/contacts/contact-list.tsx @@ -117,6 +117,7 @@ interface ContactListProps { onAddContactToGroup: (id: string) => void; /** Opens the app's composer to the contact. */ onComposeContact?: (contact: ContactCard) => void; + onDuplicateContact?: (contact: ContactCard) => void; /** Move contacts to another address book; undefined where there is none. */ onMoveContacts?: (ids: string[]) => void; /** Show a burger button at the start that opens the (overlay) categories sidebar. */ @@ -145,6 +146,7 @@ export function ContactList({ onDeleteContact, onAddContactToGroup, onComposeContact, + onDuplicateContact, onMoveContacts, onMenuClick, isLoading = false, @@ -750,6 +752,7 @@ export function ContactList({ onDelete={() => onDeleteContact(contextMenu.data!)} onAddToGroup={() => onAddContactToGroup(contextMenu.data!.id)} onCompose={onComposeContact ? () => onComposeContact(contextMenu.data!) : undefined} + onDuplicate={onDuplicateContact ? () => onDuplicateContact(contextMenu.data!) : undefined} onMoveTo={onMoveContacts ? () => onMoveContacts([contextMenu.data!.id]) : undefined} onBatchMoveTo={onMoveContacts ? () => onMoveContacts([...selectedContactIds]) : undefined} onBatchExport={onBulkExport} diff --git a/components/contacts/contacts-app.tsx b/components/contacts/contacts-app.tsx index 07229f257..25e1de47a 100644 --- a/components/contacts/contacts-app.tsx +++ b/components/contacts/contacts-app.tsx @@ -1290,6 +1290,7 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps = onDeleteContact={handleDeleteContact} onAddContactToGroup={guard(handleAddContactToGroup)} onComposeContact={handleComposeContact} + onDuplicateContact={(contact) => void handleDuplicateContact(contact)} onMoveContacts={canMoveContacts ? handleMoveContacts : undefined} onMenuClick={isNarrow ? () => setNarrowSidebarOpen(true) : undefined} isLoading={supportsSync && !contactsLoaded} diff --git a/components/files/__tests__/file-browser-shared-folders.test.tsx b/components/files/__tests__/file-browser-shared-folders.test.tsx index 64cd1ee8a..e9502f9f6 100644 --- a/components/files/__tests__/file-browser-shared-folders.test.tsx +++ b/components/files/__tests__/file-browser-shared-folders.test.tsx @@ -449,4 +449,17 @@ describe('FileBrowser', () => { fireEvent.keyDown(document.body, { key: '/' }); expect(document.activeElement).toHaveAttribute('role', 'searchbox'); }); + + it("lists a folder's actions in the same order in the list and in the tree", () => { + renderBrowser({ props: { onDownloadZip: vi.fn(async () => {}), onMoveNode: vi.fn(async () => {}) } }); + const labels = () => within(screen.getByRole('menu')).getAllByRole('menuitem').map((i) => (i.textContent ?? '').replace(/(F2|Del|Ctrl\+[A-Z]|⌘[A-Z])$/, '')); + fireEvent.contextMenu(within(list()).getByText('Documents')); + const inList = labels(); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Escape' }); + fireEvent.contextMenu(within(tree()).getByText('Documents')); + const inTree = labels(); + const shared = inTree.filter((l) => inList.includes(l)); + expect(shared).toEqual(inList.filter((l) => inTree.includes(l))); + expect(shared).toEqual(expect.arrayContaining(['open', 'star', 'rename', 'move_to', 'download_zip', 'delete'])); + }); }); diff --git a/components/files/file-browser.tsx b/components/files/file-browser.tsx index 7389694c1..6117bd7e5 100644 --- a/components/files/file-browser.tsx +++ b/components/files/file-browser.tsx @@ -876,22 +876,18 @@ export function FileBrowser({ const starred = starredPaths.has(path); return ( <> + {/* R6 order, the same as the folder tree's menu: Open, Edit, Share, + Star, Rename, Duplicate, Move to…, Cut, Copy, Download, Details, Delete. */} {resource.isDirectory ? ( - <> - { closeMenu(); openResource(resource); }} /> - {onDownloadZip && ( - { closeMenu(); downloadNames([resource.name]); }} /> - )} - + { closeMenu(); openResource(resource); }} /> ) : resource.isSymlink ? null : ( <> - {isOfficeEditable?.(resource.name) && onEditFile && ( - { closeMenu(); onEditFile(resource.name); }} /> - )} {isPreviewable(resource.name) && ( { closeMenu(); previewResource(resource); }} /> )} - { closeMenu(); void onDownload(resource.name); }} /> + {isOfficeEditable?.(resource.name) && onEditFile && ( + { closeMenu(); onEditFile(resource.name); }} /> + )} )} {(canShare(resource) || (resource.isDirectory && !resource.isShared)) && } @@ -917,6 +913,12 @@ export function FileBrowser({ {!resource.isSymlink && ( { closeMenu(); onCopy([resource.name]); }} /> )} + {resource.isDirectory && onDownloadZip && ( + { closeMenu(); downloadNames([resource.name]); }} /> + )} + {!resource.isDirectory && !resource.isSymlink && ( + { closeMenu(); void onDownload(resource.name); }} /> + )} { closeMenu(); toggleDetails(true); }} /> { closeMenu(); deleteNames([resource.name]); }} /> diff --git a/components/files/files-sidebar.tsx b/components/files/files-sidebar.tsx index afb61f00a..18deac2d1 100644 --- a/components/files/files-sidebar.tsx +++ b/components/files/files-sidebar.tsx @@ -464,7 +464,12 @@ export function FilesSidebar({ <> run(() => onNavigate(path, folder.id))} /> {writable && createItems(path, folder.id)} + {/* The list's folder menu in the same order; Cut, Copy and + Details act on the list and are left out here (R6). */} {(!folder.isShared || canShare(folder)) && } + {canShare(folder) && ( + run(() => onShare(folder))} /> + )} {!folder.isShared && ( run(() => onToggleFavorite(path))} /> )} - {canShare(folder) && ( - run(() => onShare(folder))} /> - )} - {(mayRename || mayDelete) && } + {mayRename && ( run(() => onRenameFolder(folder))} /> )} @@ -485,6 +487,7 @@ export function FilesSidebar({ {onDownloadFolder && ( run(() => onDownloadFolder(folder))} /> )} + {mayDelete && } {mayDelete && ( run(() => onDeleteFolder(folder))} /> )}