From c63d04aa98aa04ffe19f2fb00cf1fb3494f0dde3 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:37:33 +0100 Subject: [PATCH] fix: keep the folded folder column tidy with many folders: one rule between sections, shared accounts as items, counters beside the icons, and the drawer opening at the hovered folder --- app/globals.css | 12 + .../sidebar-folded-rail-scroll.test.tsx | 224 +++++++++++++++ components/layout/sidebar.tsx | 255 +++++++++++++++--- 3 files changed, 451 insertions(+), 40 deletions(-) create mode 100644 components/layout/__tests__/sidebar-folded-rail-scroll.test.tsx diff --git a/app/globals.css b/app/globals.css index 4e41210eb..8ffd7cd9c 100644 --- a/app/globals.css +++ b/app/globals.css @@ -313,6 +313,18 @@ optgroup { background-color: var(--color-muted-foreground); } +/* The folded folder column (components/layout/sidebar.tsx) leaves its 48px + to the icons: a 4px scrollbar, in a gutter kept on both sides. */ +:root:not([data-native-scrollbars="on"]) [data-folder-rail]::-webkit-scrollbar { + width: 4px; +} + +@supports not selector(::-webkit-scrollbar) { + :root:not([data-native-scrollbars="on"]) [data-folder-rail] { + scrollbar-width: thin; + } +} + /* Enhanced Email Content Styling */ .email-content-wrapper { diff --git a/components/layout/__tests__/sidebar-folded-rail-scroll.test.tsx b/components/layout/__tests__/sidebar-folded-rail-scroll.test.tsx new file mode 100644 index 000000000..484b666a7 --- /dev/null +++ b/components/layout/__tests__/sidebar-folded-rail-scroll.test.tsx @@ -0,0 +1,224 @@ +import { act, render, screen, fireEvent, cleanup } from '@testing-library/react'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import type { Mailbox } from '@/lib/jmap/types'; + +// The folded folder column (600 to 1024 px) of a mailbox with many folders, +// scrolled: shared accounts left only their headings' rules behind, stacked +// into a ladder of lines; the unread counter sat on the icon; one folder of +// one's own was a letter in a box; and the drawer a hover opened kept the +// column's scroll offset, so the hovered folder was somewhere else. + +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/components/tour/tour-provider', () => ({ + useTour: () => ({ startTour: vi.fn(), resetTourCompletion: vi.fn(), activeTour: null, endTour: vi.fn(), registerStep: vi.fn() }), +})); +vi.mock('@/contexts/drag-drop-context', () => ({ + useDragDropContext: () => ({ isDragging: false }), +})); +vi.mock('@/hooks/use-mailbox-drop', () => ({ + useMailboxDrop: () => ({ dropHandlers: {}, isValidDropTarget: false, isInvalidDropTarget: false }), +})); +vi.mock('@/hooks/use-tag-drop', () => ({ + useTagDrop: () => ({ dropHandlers: {}, isValidDropTarget: false }), +})); +vi.mock('../account-switcher', () => ({ AccountSwitcher: () => null })); + +const { Sidebar } = await import('../sidebar'); +const { useAuthStore } = await import('@/stores/auth-store'); +const { useSettingsStore } = await import('@/stores/settings-store'); +const { useUIStore } = await import('@/stores/ui-store'); + +const box = (id: string, name: string, role: string | null, extra: Partial = {}) => ({ + id, name, role, parentId: null, totalEmails: 0, unreadEmails: 0, myRights: { mayAddItems: true }, ...extra, +}) as unknown as Mailbox; + +const shared = (id: string, name: string, role: string | null, extra: Partial = {}) => + box(`g1:${id}`, name, role, { originalId: id, accountId: 'g1', accountName: 'Sam Ortega', isShared: true, ...extra }); + +const own = [ + box('inbox', 'Inbox', 'inbox', { totalEmails: 120, unreadEmails: 9 }), + box('clients', 'Clients', null, { parentId: 'inbox', unreadEmails: 132 }), + box('trash', 'Trash', 'trash', { unreadEmails: 3 }), + box('news', 'Newsletters', null, { unreadEmails: 150 }), +]; +// A shared account without an inbox: two folders someone shared. +const sharedNoInbox = [shared('team', 'Team', null, { unreadEmails: 2 }), shared('board', 'Board', null, { unreadEmails: 1 })]; + +const nav = () => document.querySelector('[data-folder-nav]')!; +const drawer = () => document.querySelector('[data-folder-drawer]'); +const row = (key: string) => document.querySelector(`[data-rail-key="${key}"]`)!; +const railButtons = () => Array.from(nav().querySelectorAll('[data-testid="folder-row"] > button')); + +describe('the folded folder column with many folders', () => { + beforeEach(() => { + localStorage.clear(); + useAuthStore.setState({ activeAccountId: 'login-a', client: { getAccountId: () => 'me', getCapabilities: () => ({}) } } as never); + useSettingsStore.setState({ + hideAccountSwitcher: true, interfaceLayout: 'default', openOnHover: true, unreadCountStyle: 'count', + showSharedMailAccounts: true, hiddenSharedMailAccounts: {}, compactSharedMailboxes: false, + } as never); + useUIStore.setState({ sidebarCollapsed: true, compactSidebarExpanded: false } as never); + }); + afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); + useUIStore.setState({ sidebarCollapsed: false } as never); + }); + + it('draws one rule between sections, never two in a row', () => { + render(); + // Folders, Your folders, Shared (with its account inside), Tags. + const rules = nav().querySelectorAll('[data-rail-separator]'); + expect(rules).toHaveLength(2 + 1); + // Walk the column in order: something to click between any two rules. + const order = Array.from(nav().querySelectorAll('[data-rail-separator], [data-testid="folder-row"]')); + order.forEach((el, i) => { + if (i === 0) return; + const both = el.hasAttribute('data-rail-separator') && order[i - 1].hasAttribute('data-rail-separator'); + expect(both, `two rules in a row at ${i}`).toBe(false); + }); + }); + + it('lists a shared account as one item, which opens the drawer with it unfolded when it has no inbox', () => { + render(); + const item = row('shared-account-g1'); + expect(item.getAttribute('data-testid')).toBe('folder-row'); + const button = item.querySelector('button')!; + expect(button.getAttribute('title')).toBe('name_unread'); + expect(item.querySelector('[data-testid="folded-unread"]')?.textContent).toBe('3'); + fireEvent.click(button); + expect(drawer()).not.toBeNull(); + expect(screen.getByText('Team')).toBeTruthy(); + }); + + it('opens a shared account\'s inbox from its item, and marks it while one of its folders is open', () => { + const onMailboxSelect = vi.fn(); + const mailboxes = [...own, shared('in', 'Inbox', 'inbox', { unreadEmails: 4 }), ...sharedNoInbox]; + const { rerender } = render(); + fireEvent.click(row('shared-account-g1').querySelector('button')!); + expect(onMailboxSelect).toHaveBeenCalledWith('g1:in'); + rerender(); + expect(row('shared-account-g1').querySelector('[data-testid="folded-selected"]')).not.toBeNull(); + }); + + it('puts the counter at the icon\'s corner, beside the glyph, and says 99+ for more', () => { + render(); + const badge = row('trash').querySelector('[data-testid="folded-unread"]')!; + expect(badge.textContent).toBe('3'); + // On the 36px item, above the 16px icon's box, not inside it over the glyph. + expect(badge.parentElement!.tagName).toBe('BUTTON'); + expect(badge.parentElement!.className).toContain('h-9'); + expect(badge.className.split(/\s+/)).toEqual(expect.arrayContaining(['absolute', 'top-0', 'end-0', 'h-3'])); + expect(badge.className).not.toContain('ring-2'); + expect(row('news').querySelector('[data-testid="folded-unread"]')!.textContent).toBe('99+'); + }); + + it('gives every item a tooltip with its name and count', () => { + render(); + for (const button of railButtons()) { + const title = button.getAttribute('title'); + expect(title, button.outerHTML.slice(0, 120)).toBeTruthy(); + expect(title).toBe(button.getAttribute('aria-label')); + } + // With unread mail the name carries the count. + expect(row('inbox').querySelector('button')!.getAttribute('title')).toBe('name_unread'); + }); + + it('draws one folder of one\'s own as a plain folder, several with their first letter inside the folder', () => { + render(); + expect(row('news').querySelector('[data-testid="folder-initial"]')).toBeNull(); + expect(row('news').querySelector('svg')).not.toBeNull(); + cleanup(); + render(); + const letter = row('receipts').querySelector('[data-testid="folder-initial"]')!; + expect(letter.textContent).toBe('R'); + // Inside the folder glyph, an icon like the others, not a box of its own. + expect(letter.closest('svg')?.querySelector('path')).not.toBeNull(); + expect(letter.closest('svg')!.getAttribute('class')).toContain('w-4'); + }); + + it('marks the open folder with a bar at the edge and a filled item', () => { + render(); + // Over the filled item, not under it. + expect(row('trash').querySelector('[data-testid="folded-selected"]')!.className).toContain('z-10'); + expect(row('trash').querySelector('button')!.className).toContain('bg-accent'); + expect(row('inbox').querySelector('[data-testid="folded-selected"]')).toBeNull(); + expect(row('inbox').querySelector('button')!.className).not.toContain('bg-accent'); + }); + + it('scrolls on its own, never sideways, with a thin scrollbar', () => { + render(); + expect(nav().hasAttribute('data-folder-rail')).toBe(true); + expect(nav().className.split(/\s+/)).toEqual(expect.arrayContaining(['overflow-y-auto', 'overflow-x-hidden', 'min-h-0'])); + }); + + it('moves through the items with the arrows, Home and End', () => { + render(); + const buttons = railButtons().filter((b) => !(b as HTMLButtonElement).disabled); + buttons[0].focus(); + fireEvent.keyDown(buttons[0], { key: 'ArrowDown' }); + expect(document.activeElement).toBe(buttons[1]); + fireEvent.keyDown(buttons[1], { key: 'ArrowUp' }); + expect(document.activeElement).toBe(buttons[0]); + fireEvent.keyDown(buttons[0], { key: 'End' }); + expect(document.activeElement).toBe(buttons[buttons.length - 1]); + fireEvent.keyDown(buttons[buttons.length - 1], { key: 'Home' }); + expect(document.activeElement).toBe(buttons[0]); + }); + + describe('the drawer opened from the scrolled column', () => { + // jsdom has no layout: the list's items sit at fixed places in the + // content, further down in the longer tree of the drawer. + let scrollTop = 0; + const NAV_TOP = 100; + const place = (el: Element) => { + const key = (el as HTMLElement).dataset?.railKey; + if (key !== 'news') return null; + return drawer() ? 900 : 500; + }; + beforeEach(() => { + scrollTop = 0; + vi.spyOn(Element.prototype, 'getBoundingClientRect').mockImplementation(function (this: Element) { + // The list shows 600px; anything else is somewhere inside it. + const isNav = this.hasAttribute('data-folder-nav'); + const at = place(this); + const top = isNav ? NAV_TOP : at === null ? NAV_TOP + 10 : NAV_TOP + at - scrollTop; + const height = isNav ? 600 : 36; + return { top, bottom: top + height, left: 0, right: 48, width: 48, height, x: 0, y: top, toJSON: () => ({}) } as DOMRect; + }); + }); + const trackScroll = () => Object.defineProperty(nav(), 'scrollTop', { + configurable: true, + get: () => scrollTop, + set: (v: number) => { scrollTop = v; }, + }); + + it('puts the hovered item back under the pointer, and the column back where it was', () => { + vi.useFakeTimers(); + const { container } = render(); + trackScroll(); + scrollTop = 200; + const news = row('news'); + document.elementFromPoint = vi.fn(() => news.querySelector('button')); + // At 400 on screen in the column: 100 + 500 - 200. + fireEvent.mouseEnter(container.firstElementChild!, { clientX: 20, clientY: 410 }); + act(() => { vi.advanceTimersByTime(400); }); + expect(drawer()).not.toBeNull(); + // In the drawer it is 900 down the list: scrolled so it is at 400 again. + expect(scrollTop).toBe(600); + fireEvent.mouseLeave(container.firstElementChild!); + expect(drawer()).toBeNull(); + expect(scrollTop).toBe(200); + }); + + it('starts at the top when All folders opens it', () => { + render(); + trackScroll(); + scrollTop = 200; + fireEvent.click(screen.getByTestId('folded-all-folders')); + expect(drawer()).not.toBeNull(); + expect(scrollTop).toBe(0); + }); + }); +}); diff --git a/components/layout/sidebar.tsx b/components/layout/sidebar.tsx index dbe7db61f..2708f4f24 100644 --- a/components/layout/sidebar.tsx +++ b/components/layout/sidebar.tsx @@ -1,7 +1,7 @@ "use client"; import { hoverOpens } from "@/lib/open-on-hover"; -import { memo, useState, useEffect, useMemo, useCallback, useRef, useId, Fragment, ReactNode } from "react"; +import { memo, useState, useEffect, useLayoutEffect, useMemo, useCallback, useRef, useId, Fragment, ReactNode } from "react"; import { useTranslations } from "next-intl"; import { useRouter } from "@/i18n/navigation"; import { PluginSlot } from "@/components/plugins/plugin-slot"; @@ -396,6 +396,9 @@ interface SidebarRowProps { testName?: string; testMailboxId?: string; testShared?: boolean; + /** Names the row the same folded and unfolded, so the drawer opened by a + * hover can put the hovered item back under the pointer. */ + railKey?: string; } function SidebarRow({ @@ -420,6 +423,7 @@ function SidebarRow({ testName, testMailboxId, testShared, + railKey, }: SidebarRowProps) { const t = useTranslations('sidebar'); const tShell = useTranslations('shell'); @@ -436,6 +440,8 @@ function SidebarRow({ unread, total, role: testRole, mode: countMode, showTotalGlobally: showFolderTotalCount, mountainView, }).unread; const accessibleName = unreadForName > 0 ? tShell('name_unread', { name: label, count: unreadForName }) : undefined; + // The default layout's folded column: one grid of square items. + const railItem = isCollapsed && !mountainView; const [labelRef, shortenedLabel] = useShortenedText(labelCandidates ?? [label]); // Touch has no right-click: a long-press opens the same menu (R7), and a // ⋮ shows it too. @@ -473,21 +479,31 @@ function SidebarRow({ data-shared={testShared ? 'true' : undefined} data-selected={isSelected ? 'true' : undefined} data-has-unread={(unread ?? 0) > 0 && testRole !== 'drafts' ? '' : undefined} - style={{ paddingBlock: 'var(--density-sidebar-py)' }} + data-rail-key={railKey ?? testMailboxId ?? undefined} + style={railItem ? undefined : { paddingBlock: 'var(--density-sidebar-py)' }} className={cn( "group w-full flex items-center tap:min-h-[44px] text-sm transition-colors duration-150", - isCollapsed ? "justify-center px-1" : "pe-2", + // Folded, every item is the same square on one grid, as in Outlook's + // folded pane: the button carries the hover and the selection, and a + // bar at the column's edge marks the open one. + // Mountain View's 72px rail keeps its own pill rows (globals.css). + railItem ? "relative justify-center py-px" : isCollapsed ? "justify-center px-1" : "pe-2", openFromRow && "cursor-pointer select-none", - isVirtual - ? "text-muted-foreground" - : isSelected - ? "bg-accent text-accent-foreground font-semibold border-s-2 border-primary" - : "hover:bg-muted/50 text-foreground border-s-2 border-transparent", + railItem + ? (isVirtual ? "text-muted-foreground" : isSelected ? "text-accent-foreground font-semibold" : "text-foreground") + : isVirtual + ? "text-muted-foreground" + : isSelected + ? "bg-accent text-accent-foreground font-semibold border-s-2 border-primary" + : "hover:bg-muted/50 text-foreground border-s-2 border-transparent", // Only a folder that takes the drop lights up. Elsewhere the pointer // says no (dropEffect none), as in a native mail client. isValidDropTarget && "bg-primary/20 ring-2 ring-primary ring-inset" )} > + {railItem && isSelected && ( +