fix: group the message menus and keep app-level items out of the reader's more menu

The reader's more menu no longer offers Star while the star sits by the
subject, so the tests that expected Star in the menu and the phone panel
now expect it gone. Import mail moved to the command palette (it stays in
the folder menu and Settings); Keyboard shortcuts stay behind ?, the rail
and Help.
This commit is contained in:
Linus Rath
2026-11-04 05:19:08 +01:00
parent 3fa82849a8
commit 2525c27a7d
6 changed files with 92 additions and 32 deletions
@@ -1,4 +1,4 @@
import { render, screen, fireEvent, act, within } from '@testing-library/react';
import { render, renderHook, screen, fireEvent, act, within } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type { Email, Mailbox } from '@/lib/jmap/types';
@@ -28,6 +28,7 @@ const { EmailViewer } = await import('../email-viewer');
const { EmailContextMenu } = await import('../email-context-menu');
const { usePrintRequestStore, requestPrint } = await import('@/hooks/use-message-file-actions');
const { emailKeyFor } = await import('@/lib/thread-utils');
const { useMessageActionList } = await import('../message-actions');
const email = {
id: 'e1',
@@ -112,17 +113,53 @@ describe("the reader's ⋮ on a desktop", () => {
renderViewer({ onTogglePinned });
fireEvent.click(screen.getByRole('button', { name: 'more_actions' }));
const menu = screen.getByRole('menu', { name: 'more_actions' });
for (const id of ['ctx-star', 'ctx-move-to', 'ctx-tag', 'ctx-export', 'ctx-print', 'ctx-view-source']) {
for (const id of ['ctx-move-to', 'ctx-tag', 'ctx-export', 'ctx-print', 'ctx-view-source']) {
expect(within(menu).getByTestId(id)).toBeTruthy();
}
// Delete is on the toolbar already.
// Delete is on the toolbar already, and the star by the subject.
expect(within(menu).queryByTestId('ctx-delete')).toBeNull();
expect(within(menu).queryByTestId('ctx-star')).toBeNull();
fireEvent.click(within(menu).getByRole('menuitem', { name: 'pin' }));
expect(onTogglePinned).toHaveBeenCalledTimes(1);
expect(screen.queryByRole('menu', { name: 'more_actions' })).toBeNull();
});
});
describe("the reader's ⋮ in groups", () => {
it("has the message's actions only, in groups", () => {
renderViewer({ onTogglePinned: vi.fn(), onShowShortcuts: vi.fn() } as never);
fireEvent.click(screen.getByRole('button', { name: 'more_actions' }));
const menu = screen.getByRole('menu', { name: 'more_actions' });
const rows = Array.from(menu.querySelectorAll<HTMLElement>('[role="menuitem"], [role="separator"]'))
.map((row) => (row.getAttribute('role') === 'separator' ? '|' : row.querySelector('[data-menu-label]')?.textContent ?? ''));
// No app-level entries: Import is in the folder's menu, Settings and the
// palette, the shortcuts behind ?. Answer; file away; mark; spam; link;
// how it shows; save.
expect(rows).toEqual([
'reply', 'reply_all', 'forward', 'forward_as_attachment', '|',
'archive', '|',
'move_to', '|',
'mark_unread', 'pin', 'tag', '|',
'mark_as_spam', '|',
'copy_message', 'copy_conversation', '|',
'reader_view', '|',
'export_email', 'print', 'view_source',
]);
});
it('puts how the reader shows the message in a group before Export, Print and View source', () => {
const { result } = renderHook(() => useMessageActionList({
email,
onExport: () => {},
onPrint: () => {},
views: [{ kind: 'item', key: 'extra:reader_view', icon: () => null, label: 'reader_view', run: () => {} }],
extras: [{ kind: 'item', key: 'extra:other', icon: () => null, label: 'other', run: () => {} }],
}));
const labels = result.current.map((entry) => (entry.kind === 'separator' ? '|' : entry.label));
expect(labels.slice(-7)).toEqual(['|', 'reader_view', '|', 'export_email', 'print', '|', 'other']);
});
});
describe("the reader toolbar's Move and Tag menus", () => {
it('are the ContextMenu: in a portal, with its rows and keys', () => {
renderViewer();
@@ -149,7 +186,9 @@ describe("the phone's More panel", () => {
const panel = screen.getByRole('menu', { name: 'more_actions' });
const pin = within(panel).getByTestId('panel-pin');
expect(pin.className).toContain('min-h-[44px]');
for (const id of ['panel-star', 'panel-export', 'panel-print', 'panel-view_source', 'panel-copy_message_link']) {
// The star is by the subject, on the phone too.
expect(within(panel).queryByTestId('panel-star')).toBeNull();
for (const id of ['panel-export', 'panel-print', 'panel-view_source', 'panel-copy_message_link']) {
expect(within(panel).getByTestId(id)).toBeTruthy();
}
fireEvent.click(pin);
+5 -1
View File
@@ -131,7 +131,9 @@ interface EmailContextMenuProps {
onViewSource?: () => void;
/** Entries the host already shows elsewhere (the reader's toolbar). */
omit?: ReadonlySet<MessageActionKey>;
/** The host's own entries after the message's (the reader's view toggles). */
/** How the host shows the message (the reader's view toggles), before Export, Print and View source. */
views?: MessageActionEntry[];
/** The host's own entries after the message's. */
extras?: MessageActionEntry[];
/** The row's plugin entries (message-row-actions). Default true. */
rowPlugins?: boolean;
@@ -192,6 +194,7 @@ export function EmailContextMenu({
onPrint,
onViewSource,
omit,
views,
extras,
rowPlugins = true,
extraEntries,
@@ -319,6 +322,7 @@ export function EmailContextMenu({
onPrint,
onViewSource,
omit,
views,
extras,
});
+8 -18
View File
@@ -3,7 +3,6 @@
import { Fragment, useState, useEffect, useLayoutEffect, useMemo, useRef, useCallback, useId } from "react";
import { Email, ContactCard, Mailbox } from "@/lib/jmap/types";
import { attachmentDownloadFilename, attachmentsBundleFilename, DEFAULT_ATTACHMENT_TEMPLATE } from "@/lib/download-filename";
import { openMailImport } from "@/stores/mail-transfer-store";
import { TRANSPARENT_BLOCKED_PIXEL, applyNewTabToAnchor, emailIframeCsp, escapeHtml, isExternalResourceUrl, isOpenableLinkHref, plainTextToSafeHtml, sanitizeEmailBodyForIframe, sanitizeEmailHtml, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization";
import { getRenderableHtmlBody } from "@/lib/email-body-selection";
import { mailLanguage } from "@/lib/mail-language";
@@ -106,7 +105,6 @@ import {
Send,
FolderInput,
Sun,
Upload,
Moon,
PlayCircle,
PenSquare,
@@ -278,7 +276,6 @@ interface EmailViewerProps {
onReplyAllConversationEmail?: (email: Email) => void;
/** Mountain View: a reply being written in place, shown under the conversation instead of the reply buttons. */
mvInlineComposer?: React.ReactNode;
onShowShortcuts?: () => void;
onEditDraft?: () => void;
onCancelScheduledForEdit?: () => void;
/** Stop a scheduled message from going out; it goes back to Drafts. */
@@ -1157,6 +1154,7 @@ function ReaderMoreMenu({ menu, options, label, mailboxes, selectedMailbox, onMo
onPrint={options.onPrint}
onViewSource={options.onViewSource}
omit={options.omit}
views={options.views}
extras={options.extras}
rowPlugins={false}
extraEntries={pluginEntries}
@@ -1341,7 +1339,6 @@ export function EmailViewer({
onReplyConversationEmail,
onReplyAllConversationEmail,
mvInlineComposer,
onShowShortcuts,
onEditDraft,
onCancelScheduledForEdit,
onCancelScheduledSend,
@@ -1364,7 +1361,6 @@ export function EmailViewer({
const tWindows = useTranslations('pwaWindows');
const tComposer = useTranslations('email_composer');
const tNotifications = useTranslations('notifications');
const tImport = useTranslations('mail_import');
const tCommon = useTranslations('common');
const tFiles = useTranslations('files');
const tDemoWelcome = useTranslations('demo_welcome');
@@ -3768,11 +3764,6 @@ export function EmailViewer({
await exportEmail(email, { onArchive, onDelete, archiveEnabled: archiveActionEnabled }, blobSource ?? { client, accountId: undefined });
};
// Import mail into the open folder: the import dialog (lib/mail-import) does the rest.
const handleImportEmail = () => {
openMailImport({ mailboxId: useEmailStore.getState().selectedMailbox || undefined });
};
// What a printout of the message shows, for the Print action's window and
// for the page's print copy (Ctrl+P). Text fields are unescaped.
const messagePrintFields = (): MessagePrintFields | null => {
@@ -4228,6 +4219,8 @@ ${printWatermark ? printWatermark.html : ''}
// the bottom bar has the replies.
const toolbarShownActions = (): ReadonlySet<MessageActionKey> => {
const shown = new Set<MessageActionKey>(['delete', 'edit_draft']);
// The star sits by the subject, or on the sender's line in Mountain View.
if (toolbarPosition === 'top' || mvViewer) shown.add('star');
toolbarRef.current?.querySelectorAll<HTMLElement>('[data-overflow-item]').forEach((item) => {
const key = TOOLBAR_PRIORITY_ACTIONS[item.dataset.overflowPriority ?? ''];
if (key && item.getClientRects().length > 0) shown.add(key);
@@ -4248,9 +4241,10 @@ ${printWatermark ? printWatermark.html : ''}
return hidden;
};
const toggleViewMode = () => setEmailViewDarkOverride(prev => prev === null ? !(resolvedTheme === 'dark') : !prev);
// The reader's own entries after the message's: the view toggle, Import
// (into the open folder) and, with a keyboard, the shortcuts.
const readerExtras: MessageActionEntry[] = [
// How the reader shows the message, a group of the ⋮ menu of its own. Import
// and the keyboard shortcuts are no message's: Import is in the folder's menu,
// Settings and the command palette, the shortcuts behind ?, the rail and Help.
const readerViews: MessageActionEntry[] = [
...(readAloud.supported && !isBodyLoading
? [{ kind: 'item' as const, key: 'extra:read_aloud' as const, icon: Volume2, label: t('read_aloud'), run: readAloud.start }]
: []),
@@ -4263,10 +4257,6 @@ ${printWatermark ? printWatermark.html : ''}
...(effectiveEmailContent.isHtml
? [{ kind: 'item' as const, key: 'extra:view_mode' as const, icon: isDark ? Sun : Moon, label: isDark ? t('view_in_light_mode') : t('view_in_dark_mode'), run: toggleViewMode }]
: []),
{ kind: 'item', key: 'extra:import', icon: Upload, label: tImport('menu_import'), run: handleImportEmail },
...(onShowShortcuts && !isMobile
? [{ kind: 'item' as const, key: 'extra:shortcuts' as const, icon: Keyboard, label: t('keyboard_shortcuts'), run: onShowShortcuts }]
: []),
];
const readerActionOptions: MessageActionOptions = {
email,
@@ -4291,7 +4281,7 @@ ${printWatermark ? printWatermark.html : ''}
onPrint: devices.noPrint ? undefined : () => handlePrint(),
onViewSource: devices.noExport ? undefined : () => setShowSourceModal(true),
omit: readerMenuOmit,
extras: readerExtras.filter((entry) => entry.kind !== 'item' || !readerMenuOmit.has(entry.key)),
views: readerViews,
};
// Shared toolbar items used by both 'top' and 'below-subject' positions
+17 -7
View File
@@ -125,6 +125,8 @@ export interface MessageActionOptions {
onViewSource?: () => void;
/** Entries the host already shows elsewhere (the reader's toolbar). */
omit?: ReadonlySet<MessageActionKey>;
/** How the host shows the message (the reader's view toggles): a group before Export, Print and View source. */
views?: MessageActionEntry[];
/** The host's own entries, after the message's own and a separator. */
extras?: MessageActionEntry[];
}
@@ -230,7 +232,6 @@ export function useMessageActionList(o: MessageActionOptions): MessageActionEntr
void copyLink(buildMailPath({ mailboxId: null, emailId: null, threadId: email.threadId!, ...emailLinkSource(email) }));
})]
: []),
SEPARATOR,
];
const archive = archiveEnabled ? [item("archive", Archive, t("archive"), o.onArchive)] : [];
const del = item("delete", Trash2, t("delete"), o.onDelete, { destructive: !mountainView, testId: "ctx-delete" });
@@ -267,25 +268,34 @@ export function useMessageActionList(o: MessageActionOptions): MessageActionEntr
const find = o.onFindFromSender && sender?.email
? [SEPARATOR, item("find_from_sender", Search, t("find_emails_from", { name: sender.name || sender.email }), o.onFindFromSender, { testId: "ctx-find-from" })]
: [];
// Download/Export, Print, View source (R6: after Copy, before the end).
const openWindow = o.onOpenInWindow
? [item("open_window", ExternalLink, tWindows("open_in_window"), o.onOpenInWindow, { testId: "ctx-open-window" })]
: [];
// How the host shows it, then Download/Export, Print, View source, at the end.
const views = o.views ?? [];
const file = [
...(o.onOpenInWindow ? [item("open_window", ExternalLink, tWindows("open_in_window"), o.onOpenInWindow, { testId: "ctx-open-window" })] : []),
...(o.onExport ? [item("export", Download, tViewer("export_email"), o.onExport, { disabled: !email.blobId, testId: "ctx-export" })] : []),
...(o.onPrint ? [item("print", Printer, tViewer("print"), o.onPrint, { testId: "ctx-print" })] : []),
...(o.onViewSource ? [item("view_source", Code, tViewer("view_source"), o.onViewSource, { disabled: !email.blobId, testId: "ctx-view-source" })] : []),
];
// In groups, as mail clients have them: answer; file away; mark; spam;
// open and link; how it shows; save, print, source.
list = mountainView
? [
...draft, ...replies, ...archive, del, read, SEPARATOR,
...move, tag, ...rules, ...copy, SEPARATOR,
star, ...pin, ...spam, ...reportPhishing, ...find, SEPARATOR,
...links, ...file,
...links, SEPARATOR, ...views, SEPARATOR, ...openWindow, ...file,
]
: [
...draft, ...replies, ...links, ...archive, del, SEPARATOR,
...move, ...copy, ...rules, star, ...pin, tag, ...spam, ...reportPhishing, SEPARATOR,
read, SEPARATOR, ...file,
...draft, ...replies, ...archive, del, SEPARATOR,
...move, ...copy, ...rules, SEPARATOR,
read, star, ...pin, tag, SEPARATOR,
...spam, ...reportPhishing, SEPARATOR,
...openWindow, ...links, SEPARATOR,
...views, SEPARATOR,
...file,
];
}
@@ -8,6 +8,7 @@ import { useSettingsStore } from '@/stores/settings-store';
import { usePluginStore } from '@/stores/plugin-store';
import { useThemeStore } from '@/stores/theme-store';
import { useToastStore } from '@/stores/toast-store';
import { useMailTransferStore } from '@/stores/mail-transfer-store';
import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils';
import { clearAllHooks } from '@/lib/plugin-hooks';
import type { CommandGroup } from '@/lib/command-registry';
@@ -108,6 +109,18 @@ describe('Mail in the command palette', () => {
expect(ids).not.toContain('mark-read');
});
it("offers Import mail into the open folder, which left the reader's ⋮ menu", async () => {
let groups: CommandGroup[] = [];
render(<><MailApp /><Palette onGroups={(g) => { groups = g; }} /></>);
await screen.findByText('viewer', {}, { timeout: 5000 });
await waitFor(() => expect(groups.find((g) => g.label === 'sidebar.mail')).toBeDefined());
const importMail = groups.find((g) => g.label === 'sidebar.mail')!.commands.find((c) => c.id === 'import');
expect(importMail?.label).toBe('mail_import.menu_import');
act(() => importMail!.run());
expect(useMailTransferStore.getState().request).toMatchObject({ kind: 'import', mailboxId: 'inbox' });
act(() => useMailTransferStore.setState({ request: null, visible: false }));
});
it("says whose a shared folder is and where a subfolder is, so two Inboxes don't look the same", async () => {
const shared = { ...folder('support:inbox', 'inbox'), isShared: true, accountName: 'Support', accountId: 'support', originalId: 'inbox' } as Mailbox;
const archive = folder('archive', 'archive', 'Archive');
+6 -2
View File
@@ -136,7 +136,7 @@ import { computeReplyThreadingHeaders } from "@/lib/email-threading";
import { openMailImport } from "@/stores/mail-transfer-store";
import { deliveryAddresses, resolveComposeAccountEmail } from "@/lib/reply-identity";
import { useProMultiAccountIdentities } from "@/hooks/use-pro-multi-account-identities";
import { Filter, X, Mail, MailOpen, PenSquare, CheckSquare, Square, AlertTriangle, ArrowLeft, RefreshCw, Reply, ReplyAll, Forward, Archive, Trash2, FolderInput, ShieldAlert, ShieldCheck, Folder } from "@/components/icons";
import { Filter, X, Mail, MailOpen, PenSquare, CheckSquare, Square, AlertTriangle, ArrowLeft, RefreshCw, Reply, ReplyAll, Forward, Archive, Trash2, FolderInput, ShieldAlert, ShieldCheck, Folder, Upload } from "@/components/icons";
import { useRegisterCommands } from "@/lib/command-registry";
import { MoveToDialog } from "@/components/email/move-to-dialog";
import { ResizeHandle } from "@/components/layout/resize-handle";
@@ -4819,6 +4819,11 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
when: () => !!paletteEmail && conversationSpamApplicable,
run: () => void (conversationInJunk ? handleUndoSpam(paletteEmail) : handleMarkAsSpam(paletteEmail)),
},
// Into the open folder, as the folder's own menu does it.
{
id: 'import', label: t('mail_import.menu_import'), icon: Upload,
run: () => openMailImport({ mailboxId: selectedMailbox || undefined }),
},
...mailboxes
.filter((mb) => !mb.id.startsWith('shared-') && mb.id !== selectedMailbox)
.map((mb) => {
@@ -6113,7 +6118,6 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
onToggleFullscreen={!isEmbedded && !isMobile ? () => setFullscreenReading((v) => !v) : undefined}
onOpenInWindow={!isEmbedded && !isMobile && openEmailInWindow && selectedEmail ? () => openEmailInWindow(selectedEmail) : undefined}
isFullscreen={fullscreenReading}
onShowShortcuts={() => setShowShortcutsModal(true)}
onEditDraft={handleEditDraft}
onCancelScheduledForEdit={async () => {
if (!client || !selectedEmail) return;