From e9931e28f8b5b13bd8057d86f5daf18237a038eb Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:34:28 +0100 Subject: [PATCH] fix: drop the reader toolbar's labels before its buttons and keep Reply, Forward, Archive, Delete and Move The toolbar measure test counted only item width reads; a pass that fits now reads no item widths, so it counts the toolbar's own scrollWidth reads too. --- app/globals.css | 5 + .../reader-toolbar-labels-first.test.tsx | 131 ++++++++++++++++++ .../__tests__/viewer-toolbar-measure.test.tsx | 6 +- components/email/email-viewer.tsx | 118 ++++++++++------ components/plugins/plugin-actions.tsx | 2 +- lib/toolbar-overflow.ts | 20 +++ 6 files changed, 236 insertions(+), 46 deletions(-) create mode 100644 components/email/__tests__/reader-toolbar-labels-first.test.tsx diff --git a/app/globals.css b/app/globals.css index b9aee76aa..a7e851e20 100644 --- a/app/globals.css +++ b/app/globals.css @@ -566,6 +566,11 @@ optgroup { animation: toast-exit 0.28s cubic-bezier(0.06, 0.71, 0.55, 1) forwards; } +/* The reader toolbar with no room for its labels: icons only, the names stay in the tooltips. */ +[data-toolbar-compact] [data-toolbar-label] { + display: none !important; +} + /* Mobile Responsive Utilities */ /* diff --git a/components/email/__tests__/reader-toolbar-labels-first.test.tsx b/components/email/__tests__/reader-toolbar-labels-first.test.tsx new file mode 100644 index 000000000..ea51036a2 --- /dev/null +++ b/components/email/__tests__/reader-toolbar-labels-first.test.tsx @@ -0,0 +1,131 @@ +import { render, act, fireEvent, screen } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import type { Email, Mailbox } from '@/lib/jmap/types'; + +// The reader toolbar hid whole buttons (Forward, Archive, Move, at 1024 px +// even Reply) while their labels stayed. Now the labels go first, and Reply, +// Reply all, Forward, Archive and Move stay while anything else can give way. + +vi.mock('@/hooks/use-media-query', () => ({ + useDeviceDetection: () => ({ isMobile: false, isTablet: false, isDesktop: true }), + useMediaQuery: () => false, +})); +vi.mock('@/components/tour/tour-provider', () => ({ + useTour: () => ({ activeTour: null, startTour: vi.fn(), endTour: vi.fn(), registerStep: vi.fn() }), +})); +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/hooks/use-plugin-slot-offers', () => ({ usePluginSlotOffers: () => [] })); + +class NoopResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} +vi.stubGlobal('ResizeObserver', NoopResizeObserver); + +const { EmailViewer } = await import('../email-viewer'); + +const mailboxes = [ + { id: 'inbox', name: 'Inbox', role: 'inbox', parentId: null, myRights: { mayAddItems: true } }, + { id: 'clients', name: 'Clients', role: null, parentId: null, myRights: { mayAddItems: true } }, +] as unknown as Mailbox[]; +const email = { + id: 'e1', blobId: 'b1', threadId: 't1', mailboxIds: { inbox: true }, keywords: { $seen: true }, + from: [{ name: 'Alice', email: 'alice@example.com' }], to: [], subject: 'Hello', receivedAt: '2026-01-01T00:00:00Z', + preview: '', hasAttachment: false, size: 1, textBody: [], htmlBody: [], attachments: [], bodyValues: {}, +} as unknown as Email; + +const REQUIRED = ['1', '2', '3', '4', '5']; + +/** + * A toolbar 600 px wide whose two groups need `labelled` px each with labels + * and `compact` px each without; every item is 40 px. + */ +function layout(labelled: number, compact: number) { + const spies = [ + vi.spyOn(HTMLElement.prototype, 'scrollWidth', 'get').mockImplementation(function (this: HTMLElement) { + return this.closest('[data-toolbar-compact]') ? compact : labelled; + }), + vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(600), + vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockReturnValue(40), + ]; + return () => spies.forEach((spy) => spy.mockRestore()); +} + +let restore: (() => void) | null = null; +afterEach(() => { restore?.(); restore = null; }); + +const viewer = (props: Partial> = {}) => render( + , +); +const toolbar = (container: HTMLElement) => container.querySelector('[class*="@container/reader-toolbar"]')!; +const hiddenPriorities = (container: HTMLElement) => + Array.from(container.querySelectorAll('[data-overflow-item]')) + .filter((item) => item.style.display === 'none') + .map((item) => item.dataset.overflowPriority!); + +describe('reader toolbar overflow', () => { + it('keeps its labels when everything fits', () => { + restore = layout(250, 150); + const { container } = viewer(); + expect(toolbar(container).hasAttribute('data-toolbar-compact')).toBe(false); + expect(hiddenPriorities(container)).toEqual([]); + }); + + it('drops the labels before any button', () => { + restore = layout(400, 250); + const { container } = viewer(); + expect(toolbar(container).hasAttribute('data-toolbar-compact')).toBe(true); + expect(hiddenPriorities(container)).toEqual([]); + // Every label is marked, so the compact toolbar shows icons only. + const labels = Array.from(container.querySelectorAll('[data-toolbar-label]')).map((label) => label.textContent); + expect(labels).toEqual(expect.arrayContaining(['reply', 'reply_all', 'forward', 'archive', 'delete', 'move'])); + }); + + it('then hides the optional buttons, never Reply, Reply all, Forward, Archive or Move', () => { + restore = layout(500, 400); + const { container } = viewer(); + const hidden = hiddenPriorities(container); + expect(hidden.length).toBeGreaterThan(0); + expect(hidden.filter((p) => REQUIRED.includes(p))).toEqual([]); + // The least important first: the dark-mode toggle before the read state. + expect(hidden).toContain('11'); + }); + + it('hides the required ones only in a reader too narrow for their icons, Reply last', () => { + restore = layout(2000, 1000); + const { container } = viewer(); + const hidden = hiddenPriorities(container); + expect(hidden).toEqual(expect.arrayContaining(REQUIRED)); + restore(); + // A little more room keeps Reply. + restore = layout(2000, 470); + const { container: roomier } = viewer(); + const kept = hiddenPriorities(roomier); + expect(kept).not.toContain('1'); + }); + + it('offers Open in new window and Fullscreen in the more menu when they have no room', async () => { + restore = layout(500, 400); + viewer({ onOpenInWindow: vi.fn(), onToggleFullscreen: vi.fn() }); + expect(screen.getByTestId('viewer-open-in-window').hasAttribute('data-overflow-item')).toBe(true); + // jsdom lays nothing out, so every toolbar button reads as hidden here. + await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'more_actions' })); }); + expect(await screen.findByRole('menuitem', { name: 'open_in_window' })).toBeTruthy(); + expect(screen.getByRole('menuitem', { name: 'fullscreen' })).toBeTruthy(); + }); +}); diff --git a/components/email/__tests__/viewer-toolbar-measure.test.tsx b/components/email/__tests__/viewer-toolbar-measure.test.tsx index f5b2ec13d..0f1d8c11b 100644 --- a/components/email/__tests__/viewer-toolbar-measure.test.tsx +++ b/components/email/__tests__/viewer-toolbar-measure.test.tsx @@ -50,7 +50,11 @@ let widthReads = 0; beforeEach(() => { clearAllHooks(); widthReads = 0; - vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(() => { widthReads++; return 40; }); + // A width read in the toolbar: its groups' (every measuring pass) and its + // items' (only a pass that has to hide some). + const inToolbar = (el: HTMLElement) => !!el.closest('[class*="@container/reader-toolbar"]'); + vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function (this: HTMLElement) { if (inToolbar(this)) widthReads++; return 40; }); + vi.spyOn(HTMLElement.prototype, 'scrollWidth', 'get').mockImplementation(function (this: HTMLElement) { if (inToolbar(this)) widthReads++; return 0; }); }); afterEach(() => vi.restoreAllMocks()); diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 0b1b5fac7..2727ea72d 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -49,7 +49,7 @@ import { QuickReplyPluginActions } from "./quick-reply-plugin-actions"; import { quickReplyEnvelope } from "@/lib/outgoing/quick-reply-envelope"; import { formatFileSize, cn, formatDate, formatDateTime, formatRelativeAgo, generateUUID, cleanPreview, type MailboxNode } from "@/lib/utils"; import { getEffectiveTimeZone, getWallClock } from "@/lib/timezone"; -import { itemsToHide } from "@/lib/toolbar-overflow"; +import { itemsToHide, toolbarHideOrder } from "@/lib/toolbar-overflow"; import { formatScheduledSendAt } from "@/lib/scheduled-send-label"; import { buildMoveTargets, resolveMoveOwnerAccountId } from "@/lib/move-targets"; import { emailDisplayDate } from "@/lib/email-date"; @@ -210,8 +210,19 @@ const TOOLBAR_PRIORITY_ACTIONS: Record = { '7': 'spam', '8': 'read', '9': 'print', + '10': 'open_window', '11': 'extra:view_mode', + '12': 'extra:fullscreen', }; +/** + * The reader toolbar's buttons that stay: Reply (1), Reply all (2), Forward + * (3), Archive (4, or what stands in its place in Spam and Trash) and Move + * (5), with Delete, which never gives way. Only a reader too narrow for their + * icons hides them, in this order. + */ +const REQUIRED_TOOLBAR_PRIORITIES = [5, 3, 2, 4, 1] as const; +/** Set on the toolbar while it shows icons without their labels. */ +const TOOLBAR_COMPACT_ATTRIBUTE = 'data-toolbar-compact'; const NO_ACTION_KEYS: ReadonlySet = new Set(); const NO_PLUGIN_KEYS: ReadonlySet = new Set(); @@ -1156,6 +1167,7 @@ function ReaderMoreMenu({ menu, options, label, mailboxes, selectedMailbox, onMo onExport={options.onExport} onPrint={options.onPrint} onViewSource={options.onViewSource} + onOpenInWindow={options.onOpenInWindow} omit={options.omit} extras={options.extras} rowPlugins={false} @@ -1802,7 +1814,7 @@ export function EmailViewer({ // looked like (see layoutKey), and the width it last had: an open that // brings back a toolbar already measured at this width needs no measuring. // A loading message and a loaded one can differ, so a few are kept. - const measuredToolbarRef = useRef(new Map>()); + const measuredToolbarRef = useRef(new Map; compact: boolean }>()); const toolbarWidthRef = useRef(undefined); const currentTagIds = getEmailTagIds(email?.keywords); const sortedTagIds = sortTagIds(currentTagIds); @@ -2060,17 +2072,14 @@ export function EmailViewer({ const key = layoutKey(items); const known = toolbarWidthRef.current !== undefined ? measuredToolbarRef.current.get(key) : undefined; if (known) { - for (const item of items) item.style.display = known.has(Number(item.dataset.overflowPriority)) ? 'none' : ''; - applyHidden(known); + el.toggleAttribute(TOOLBAR_COMPACT_ATTRIBUTE, known.compact); + for (const item of items) item.style.display = known.hidden.has(Number(item.dataset.overflowPriority)) ? 'none' : ''; + applyHidden(known.hidden); return; } - // Sort descending by priority so highest number (least important) is hidden first - items.sort((a, b) => - Number(b.dataset.overflowPriority || 0) - Number(a.dataset.overflowPriority || 0) - ); - // Show all items to measure their natural widths + // Show all items, with their labels, to measure their natural widths items.forEach(item => { item.style.display = ''; }); - const containerWidth = el.clientWidth; + el.removeAttribute(TOOLBAR_COMPACT_ATTRIBUTE); const leftGroup = el.firstElementChild as HTMLElement; const rightGroup = el.lastElementChild as HTMLElement; const mainGap = parseFloat(getComputedStyle(el).gap) || 0; @@ -2078,25 +2087,37 @@ export function EmailViewer({ leftGroup.style.flexShrink = '0'; rightGroup.style.flexShrink = '0'; el.style.overflow = 'hidden'; - // Measure every width in one pass and decide from the numbers (see - // itemsToHide) rather than hiding items one by one and re-reading. - const groupIds = new Map([[leftGroup, 'left'], [rightGroup, 'right']]); - const groupOf = (item: HTMLElement) => { - const parent = item.parentElement; - if (!parent) return ''; - if (!groupIds.has(parent)) groupIds.set(parent, String(groupIds.size)); - return groupIds.get(parent)!; - }; - const measured = items.map((item) => ({ width: item.offsetWidth, group: groupOf(item) })); - const overflow = leftGroup.scrollWidth + rightGroup.scrollWidth + mainGap - (containerWidth + 1); - const gapOf = { - left: parseFloat(getComputedStyle(leftGroup).gap) || 0, - right: parseFloat(getComputedStyle(rightGroup).gap) || 0, - }; + const containerWidth = el.clientWidth; + const overflowNow = () => leftGroup.scrollWidth + rightGroup.scrollWidth + mainGap - (containerWidth + 1); + let overflow = overflowNow(); + // The labels go before any button does (icons keep their tooltips). + const compact = overflow > 0; + if (compact) { + el.setAttribute(TOOLBAR_COMPACT_ATTRIBUTE, ''); + overflow = overflowNow(); + } const hidden = new Set(); - for (const i of itemsToHide(measured, overflow, gapOf)) { - items[i].style.display = 'none'; - hidden.add(Number(items[i].dataset.overflowPriority)); + if (overflow > 0) { + // Measure every width in one pass and decide from the numbers (see + // itemsToHide) rather than hiding items one by one and re-reading. + const order = toolbarHideOrder(items.map((item) => Number(item.dataset.overflowPriority || 0)), REQUIRED_TOOLBAR_PRIORITIES); + const ordered = order.map((i) => items[i]); + const groupIds = new Map([[leftGroup, 'left'], [rightGroup, 'right']]); + const groupOf = (item: HTMLElement) => { + const parent = item.parentElement; + if (!parent) return ''; + if (!groupIds.has(parent)) groupIds.set(parent, String(groupIds.size)); + return groupIds.get(parent)!; + }; + const measured = ordered.map((item) => ({ width: item.offsetWidth, group: groupOf(item) })); + const gapOf = { + left: parseFloat(getComputedStyle(leftGroup).gap) || 0, + right: parseFloat(getComputedStyle(rightGroup).gap) || 0, + }; + for (const i of itemsToHide(measured, overflow, gapOf)) { + ordered[i].style.display = 'none'; + hidden.add(Number(ordered[i].dataset.overflowPriority)); + } } // Restore layout leftGroup.style.flexShrink = ''; @@ -2104,7 +2125,7 @@ export function EmailViewer({ el.style.overflow = ''; const answers = measuredToolbarRef.current; if (answers.size >= 8) answers.delete(answers.keys().next().value!); - answers.set(key, hidden); + answers.set(key, { hidden, compact }); applyHidden(hidden); }; @@ -4267,6 +4288,10 @@ ${printWatermark ? printWatermark.html : ''} ...(onShowShortcuts && !isMobile ? [{ kind: 'item' as const, key: 'extra:shortcuts' as const, icon: Keyboard, label: t('keyboard_shortcuts'), run: onShowShortcuts }] : []), + // Only while the toolbar has no room for its button (see toolbarShownActions). + ...(onToggleFullscreen && !isFullscreen && !mvViewer && !isMobile + ? [{ kind: 'item' as const, key: 'extra:fullscreen' as const, icon: Maximize2, label: t('fullscreen'), run: onToggleFullscreen }] + : []), ]; const readerActionOptions: MessageActionOptions = { email, @@ -4290,6 +4315,8 @@ ${printWatermark ? printWatermark.html : ''} onExport: devices.noExport ? undefined : () => void handleExportEmail(), onPrint: devices.noPrint ? undefined : () => handlePrint(), onViewSource: devices.noExport ? undefined : () => setShowSourceModal(true), + // Only while the toolbar has no room for its button (see toolbarShownActions). + onOpenInWindow: onOpenInWindow && !mvViewer && !isMobile ? onOpenInWindow : undefined, omit: readerMenuOmit, extras: readerExtras.filter((entry) => entry.kind !== 'item' || !readerMenuOmit.has(entry.key)), }; @@ -4358,16 +4385,16 @@ ${printWatermark ? printWatermark.html : ''} title={t('reschedule_send')} > - {showToolbarLabels && {t('reschedule_send')}} + {showToolbarLabels && {t('reschedule_send')}} {onCancelScheduledSend && !email.isSmimeScheduled && ( )} @@ -4396,7 +4423,7 @@ ${printWatermark ? printWatermark.html : ''} title={t('tooltips.reply')} > - {showToolbarLabels && {t('reply')}} + {showToolbarLabels && {t('reply')}} )} @@ -4452,7 +4479,7 @@ ${printWatermark ? printWatermark.html : ''} title={t('tooltips.archive')} > - {showToolbarLabels && {t('archive')}} + {showToolbarLabels && {t('archive')}} )} {/* Delete */} @@ -4464,7 +4491,7 @@ ${printWatermark ? printWatermark.html : ''} title={t('tooltips.delete')} > - {showToolbarLabels && {t('delete')}} + {showToolbarLabels && {t('delete')}} {/* Move to folder */} {moveTree.length > 0 && onMoveToMailbox && ( @@ -4484,7 +4511,7 @@ ${printWatermark ? printWatermark.html : ''} aria-expanded={moveMenuOpen} > - {showToolbarLabels && {t('move')}} + {showToolbarLabels && {t('move')}}
@@ -4516,7 +4543,7 @@ ${printWatermark ? printWatermark.html : ''} aria-expanded={tagMenuOpen} > - {showToolbarLabels && {t('tag')}} + {showToolbarLabels && {t('tag')}}
@@ -4551,7 +4578,7 @@ ${printWatermark ? printWatermark.html : ''} ) : ( )} - {showToolbarLabels && {isInJunkFolder ? t('spam.not_spam_title') : t('spam.button_title')}} + {showToolbarLabels && {isInJunkFolder ? t('spam.not_spam_title') : t('spam.button_title')}} )} @@ -4573,7 +4600,7 @@ ${printWatermark ? printWatermark.html : ''} > {isUnread ? : } {showToolbarLabels && ( - + {t('read')} {t('unread')} @@ -4593,7 +4620,7 @@ ${printWatermark ? printWatermark.html : ''} title={t('print')} > - {showToolbarLabels && {t('print')}} + {showToolbarLabels && {t('print')}} )} @@ -4620,6 +4647,8 @@ ${printWatermark ? printWatermark.html : ''} variant="ghost" size="sm" onClick={onOpenInWindow} + data-overflow-item + data-overflow-priority="10" className="hidden sm:inline-flex h-8 gap-1.5" title={tWindows('open_in_window')} aria-label={tWindows('open_in_window')} @@ -4630,13 +4659,14 @@ ${printWatermark ? printWatermark.html : ''} )} {/* Fullscreen toggle - hidden on mobile (already fullscreen there). - Never overflows into the More menu: in fullscreen this button is - the way back out, so it must stay visible. */} + The first to give way to the More menu, but never in fullscreen: + there this button is the way back out, so it must stay visible. */} {onToggleFullscreen && !mvViewer && (