From 3719014a9148f5f4177361b37fa49e9a8462b34c Mon Sep 17 00:00:00 2001 From: jbpenrath Date: Fri, 12 Jun 2026 00:55:49 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8(frontend)=20improve=20thread=20nav?= =?UTF-8?q?igation=20a11y=20and=20multiselect=20ux?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declare the thread list as a listbox with multiselectable elements. Now when multiselect is enable, clicking on a thread add it to the current selection, it does not reset the selection. Furthermore, the keyboard navigation has been improved. --- .../src/__tests__/attachment-preview.spec.ts | 2 +- .../src/__tests__/mailbox-settings.spec.ts | 7 +- src/e2e/src/__tests__/message-import.spec.ts | 2 +- .../__tests__/message-inline-image.spec.ts | 4 +- src/e2e/src/__tests__/message-send.spec.ts | 4 +- src/e2e/src/__tests__/outbox.spec.ts | 16 +- src/e2e/src/__tests__/thread-event.spec.ts | 10 +- .../src/__tests__/thread-starred-read.spec.ts | 28 +-- src/frontend/public/locales/common/en-US.json | 3 +- src/frontend/public/locales/common/fr-FR.json | 3 +- .../components/thread-item/_index.scss | 18 ++ .../components/thread-item/index.tsx | 64 ++++--- .../hooks/listbox-navigation.test.ts | 58 ++++++ .../thread-panel/hooks/listbox-navigation.ts | 36 ++++ .../thread-panel/hooks/use-thread-listbox.ts | 135 ++++++++++++++ .../layouts/components/thread-panel/index.tsx | 21 ++- .../providers/thread-listbox-focus.tsx | 46 +++++ .../providers/thread-selection-core.test.ts | 89 ++++++++++ .../providers/thread-selection-core.ts | 77 ++++++++ .../features/providers/thread-selection.tsx | 167 +++++------------- .../src/routes/mailbox/$mailboxId/route.tsx | 5 +- 21 files changed, 600 insertions(+), 195 deletions(-) create mode 100644 src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.test.ts create mode 100644 src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.ts create mode 100644 src/frontend/src/features/layouts/components/thread-panel/hooks/use-thread-listbox.ts create mode 100644 src/frontend/src/features/providers/thread-listbox-focus.tsx create mode 100644 src/frontend/src/features/providers/thread-selection-core.test.ts create mode 100644 src/frontend/src/features/providers/thread-selection-core.ts diff --git a/src/e2e/src/__tests__/attachment-preview.spec.ts b/src/e2e/src/__tests__/attachment-preview.spec.ts index c028a90e4..25d087459 100644 --- a/src/e2e/src/__tests__/attachment-preview.spec.ts +++ b/src/e2e/src/__tests__/attachment-preview.spec.ts @@ -59,7 +59,7 @@ test.describe("Attachment preview", () => { await page.getByText("Message sent successfully").waitFor({ state: "visible" }); await page.getByRole("link", { name: "Sent" }).click(); - await page.getByRole("link", { name: subject }).first().click(); + await page.getByRole("option", { name: subject }).first().click(); await page.getByRole("heading", { name: subject, level: 2 }).waitFor({ state: "visible" }); } diff --git a/src/e2e/src/__tests__/mailbox-settings.spec.ts b/src/e2e/src/__tests__/mailbox-settings.spec.ts index dd385f3cc..e411d1f68 100644 --- a/src/e2e/src/__tests__/mailbox-settings.spec.ts +++ b/src/e2e/src/__tests__/mailbox-settings.spec.ts @@ -81,8 +81,11 @@ test.describe("Mailbox settings modal", () => { // The switcher renders a single-select dropdown, so its entries expose a // `menuitemradio`/`option` role depending on the design-system version; match - // both so the assertion does not hinge on that internal detail. - const options = page.locator('[role^="menuitem"], [role="option"]'); + // both so the assertion does not hinge on that internal detail. Scope to the + // dropdown popover: it is portalled outside the modal, and the thread list is + // itself a listbox whose `option` entries would otherwise be matched too. + const switcherPopover = page.getByRole("menu"); + const options = switcherPopover.locator('[role^="menuitem"], [role="option"]'); await expect(options).toHaveCount(2); await expect( options.filter({ hasText: `user.e2e.${browserName}@example.local` }), diff --git a/src/e2e/src/__tests__/message-import.spec.ts b/src/e2e/src/__tests__/message-import.spec.ts index 644ca92ac..d8623497b 100644 --- a/src/e2e/src/__tests__/message-import.spec.ts +++ b/src/e2e/src/__tests__/message-import.spec.ts @@ -103,7 +103,7 @@ test.describe("Import Message", () => { // Then expect the new message to be visible in the thread list await expect( - page.getByRole("link", { name: "Sardine 18/11/2025 An old message" }) + page.getByRole("option", { name: "Sardine 18/11/2025 An old message" }) ).toBeVisible(); }); diff --git a/src/e2e/src/__tests__/message-inline-image.spec.ts b/src/e2e/src/__tests__/message-inline-image.spec.ts index 5daa56280..a10876c76 100644 --- a/src/e2e/src/__tests__/message-inline-image.spec.ts +++ b/src/e2e/src/__tests__/message-inline-image.spec.ts @@ -97,7 +97,7 @@ test.describe("Inline Image in Composer", () => { // Verify the message appears in sentbox await page.getByRole("link", { name: "Sent" }).click(); - const sentItem = page.getByRole("link", { name: "Message with inline image" }).first(); + const sentItem = page.getByRole("option", { name: "Message with inline image" }).first(); await expect(sentItem).toBeVisible(); // Open the message and check content @@ -117,7 +117,7 @@ test.describe("Inline Image in Composer", () => { await page.getByRole("link", { name: "Inbox" }).click(); // Open the message and check content - const receivedItem = await page.getByRole("link", { name: "Message with inline image" }).first(); + const receivedItem = await page.getByRole("option", { name: "Message with inline image" }).first(); await expect(receivedItem).toBeVisible(); await receivedItem.click(); await page.getByRole("heading", { name: "Message with inline image", level: 2 }).waitFor({ state: "visible" }); diff --git a/src/e2e/src/__tests__/message-send.spec.ts b/src/e2e/src/__tests__/message-send.spec.ts index 0a4518900..a26eca84b 100644 --- a/src/e2e/src/__tests__/message-send.spec.ts +++ b/src/e2e/src/__tests__/message-send.spec.ts @@ -53,7 +53,7 @@ test.describe("Send Message", () => { // Go to the sentbox and check if the message is there await page.getByRole("link", { name: "Sent" }).click(); - const threadItem = page.getByRole("link", { name: "Hello everyone!" }).first(); + const threadItem = page.getByRole("option", { name: "Hello everyone!" }).first(); await expect(threadItem).toBeVisible(); expect(await threadItem.textContent()).toMatch(new RegExp(`User E2E ${browserName}`, "i")); @@ -64,7 +64,7 @@ test.describe("Send Message", () => { await page.getByRole("link", { name: "Inbox" }).click(); - const messageItem = page.getByRole("link", { name: "Hello everyone!" }).first(); + const messageItem = page.getByRole("option", { name: "Hello everyone!" }).first(); await expect(messageItem).toBeVisible(); expect(await messageItem.textContent()).toMatch(new RegExp(`User E2E ${browserName}`, "i")); diff --git a/src/e2e/src/__tests__/outbox.spec.ts b/src/e2e/src/__tests__/outbox.spec.ts index 65a3e28fa..f28405d79 100644 --- a/src/e2e/src/__tests__/outbox.spec.ts +++ b/src/e2e/src/__tests__/outbox.spec.ts @@ -22,7 +22,7 @@ test.describe("Delivery failures", () => { // The thread with delivery issues should be visible in the list const threadItem = page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first(); await expect(threadItem).toBeVisible(); }); @@ -39,7 +39,7 @@ test.describe("Delivery failures", () => { // Click on the thread to open it const threadItem = page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first(); await threadItem.click(); @@ -79,7 +79,7 @@ test.describe("Delivery failures", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first() .click(); @@ -110,7 +110,7 @@ test.describe("Delivery failures", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first() .click(); @@ -144,7 +144,7 @@ test.describe("Delivery failures", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first() .click(); @@ -180,7 +180,7 @@ test.describe("Delivery failures", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first() .click(); @@ -225,7 +225,7 @@ test.describe("Delivery pending (retry only)", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Test message with pending delivery" }) + .getByRole("option", { name: "Test message with pending delivery" }) .first() .click(); @@ -258,7 +258,7 @@ test.describe("Delivery pending (retry only)", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Test message with pending delivery" }) + .getByRole("option", { name: "Test message with pending delivery" }) .first() .click(); diff --git a/src/e2e/src/__tests__/thread-event.spec.ts b/src/e2e/src/__tests__/thread-event.spec.ts index d972ac953..2aba2070c 100644 --- a/src/e2e/src/__tests__/thread-event.spec.ts +++ b/src/e2e/src/__tests__/thread-event.spec.ts @@ -42,7 +42,7 @@ async function navigateToSharedThread(page: Page, browserName: BrowserName) { await inboxFolderLink(page).click(); await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Shared inbox thread for IM" }) + .getByRole("option", { name: "Shared inbox thread for IM" }) .first() .click(); await page @@ -93,7 +93,7 @@ test.describe("Thread Events (Internal Messages)", () => { await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Inbox thread alpha" }) + .getByRole("option", { name: "Inbox thread alpha" }) .first() .click(); await page @@ -489,7 +489,7 @@ test.describe("Thread Events (Internal Messages)", () => { // "Unread mention" badge. The thread list is re-fetched on navigation, // which makes it the most reliable indicator that the mention landed. const threadLink = page - .getByRole("link", { name: "Shared inbox thread for IM" }) + .getByRole("option", { name: "Shared inbox thread for IM" }) .first(); await expect( threadLink.getByLabel("Unread mention").first(), @@ -591,7 +591,7 @@ test.describe("Thread Events (Assignations)", () => { await inboxFolderLink(page).click(); await page.waitForLoadState("networkidle"); await page - .getByRole("link", { name: "Inbox thread alpha" }) + .getByRole("option", { name: "Inbox thread alpha" }) .first() .click(); await page @@ -778,7 +778,7 @@ test.describe("Thread Events (Assignations)", () => { await page.waitForLoadState("networkidle"); await expect( - page.getByRole("link", { name: "Shared inbox thread for IM" }).first(), + page.getByRole("option", { name: "Shared inbox thread for IM" }).first(), ).toBeVisible(); // Cleanup: drop the assignment so the test is rerun-safe even when diff --git a/src/e2e/src/__tests__/thread-starred-read.spec.ts b/src/e2e/src/__tests__/thread-starred-read.spec.ts index ece5079fa..37d29b756 100644 --- a/src/e2e/src/__tests__/thread-starred-read.spec.ts +++ b/src/e2e/src/__tests__/thread-starred-read.spec.ts @@ -25,7 +25,7 @@ test.describe("Thread starred", () => { // Open the first thread await page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first() .click(); await page @@ -63,7 +63,7 @@ test.describe("Thread starred", () => { // Open the thread (starred from previous test) await page - .getByRole("link", { name: "Test message with delivery failure" }) + .getByRole("option", { name: "Test message with delivery failure" }) .first() .click(); await page @@ -112,7 +112,7 @@ test.describe("Thread read / unread", () => { // Open the thread (the IntersectionObserver auto-marks messages as read) await page - .getByRole("link", { name: "Inbox thread alpha" }) + .getByRole("option", { name: "Inbox thread alpha" }) .first() .click(); await page @@ -154,15 +154,15 @@ test.describe("Thread read / unread", () => { // Both threads should be visible (both unread) await expect( - page.getByRole("link", { name: "Inbox thread alpha" }).first(), + page.getByRole("option", { name: "Inbox thread alpha" }).first(), ).toBeVisible(); await expect( - page.getByRole("link", { name: "Inbox thread beta" }).first(), + page.getByRole("option", { name: "Inbox thread beta" }).first(), ).toBeVisible(); // Open a thread — the IntersectionObserver auto-marks it as read await page - .getByRole("link", { name: "Inbox thread alpha" }) + .getByRole("option", { name: "Inbox thread alpha" }) .first() .click(); await page @@ -176,7 +176,7 @@ test.describe("Thread read / unread", () => { // The thread should still be visible in the list thanks to thread pinning logic // Check @/features/providers/mailbox-cache.ts await expect( - page.getByRole("link", { name: "Inbox thread alpha" }).first(), + page.getByRole("option", { name: "Inbox thread alpha" }).first(), ).toBeVisible(); }); @@ -200,15 +200,15 @@ test.describe("Thread read / unread", () => { // Verify both threads are visible initially await expect( - page.getByRole("link", { name: "Inbox thread alpha" }).first(), + page.getByRole("option", { name: "Inbox thread alpha" }).first(), ).toBeVisible(); await expect( - page.getByRole("link", { name: "Inbox thread beta" }).first(), + page.getByRole("option", { name: "Inbox thread beta" }).first(), ).toBeVisible(); // Open the first thread to mark it as read (IntersectionObserver auto-read) await page - .getByRole("link", { name: "Inbox thread alpha" }) + .getByRole("option", { name: "Inbox thread alpha" }) .first() .click(); await page @@ -232,12 +232,12 @@ test.describe("Thread read / unread", () => { // The read thread should be filtered out await expect( - page.getByRole("link", { name: "Inbox thread alpha" }), + page.getByRole("option", { name: "Inbox thread alpha" }), ).not.toBeVisible(); // The unread thread (not opened) should still be visible await expect( - page.getByRole("link", { name: "Inbox thread beta" }).first(), + page.getByRole("option", { name: "Inbox thread beta" }).first(), ).toBeVisible(); // Click the filter button again to clear the filter @@ -246,10 +246,10 @@ test.describe("Thread read / unread", () => { // Both threads should be visible again await expect( - page.getByRole("link", { name: "Inbox thread alpha" }).first(), + page.getByRole("option", { name: "Inbox thread alpha" }).first(), ).toBeVisible(); await expect( - page.getByRole("link", { name: "Inbox thread beta" }).first(), + page.getByRole("option", { name: "Inbox thread beta" }).first(), ).toBeVisible(); }); }); diff --git a/src/frontend/public/locales/common/en-US.json b/src/frontend/public/locales/common/en-US.json index 881a83d52..ec4c58780 100755 --- a/src/frontend/public/locales/common/en-US.json +++ b/src/frontend/public/locales/common/en-US.json @@ -324,7 +324,6 @@ "Description": "Description", "Description must be less than 255 characters.": "Description must be less than 255 characters.", "Deselect all threads": "Deselect all threads", - "Deselect thread": "Deselect thread", "Did you forget an attachment?": "Did you forget an attachment?", "Disable thread selection": "Disable thread selection", "Display those images": "Display those images", @@ -654,7 +653,6 @@ "Select a thread": "Select a thread", "Select all threads": "Select all threads", "Select the mailbox to configure": "Select the mailbox to configure", - "Select thread": "Select thread", "Select threads": "Select threads", "Send": "Send", "Send and archive": "Send and archive", @@ -774,6 +772,7 @@ "This will move this message and all following messages to a new thread. Continue?": "This will move this message and all following messages to a new thread. Continue?", "Thread access removed": "Thread access removed", "Thread has been split successfully.": "Thread has been split successfully.", + "Thread list": "Thread list", "Thursday": "Thursday", "Timezone": "Timezone", "To": "To", diff --git a/src/frontend/public/locales/common/fr-FR.json b/src/frontend/public/locales/common/fr-FR.json index 99178c14a..4ddd62edb 100755 --- a/src/frontend/public/locales/common/fr-FR.json +++ b/src/frontend/public/locales/common/fr-FR.json @@ -394,7 +394,6 @@ "Description": "Description", "Description must be less than 255 characters.": "La description ne peut pas excéder 255 caractères.", "Deselect all threads": "Désélectionner toutes les conversations", - "Deselect thread": "Désélectionner la conversation", "Did you forget an attachment?": "N'avez-vous pas oublié une pièce jointe ?", "Disable thread selection": "Désactiver la sélection", "Display those images": "Afficher ces images", @@ -735,7 +734,6 @@ "Select a thread": "Sélectionner une conversation", "Select all threads": "Sélectionner toutes les conversations", "Select the mailbox to configure": "Sélectionner la boîte aux lettres à configurer", - "Select thread": "Sélectionner une conversation", "Select threads": "Sélectionner des conversations", "Send": "Envoyer", "Send and archive": "Envoyer et archiver", @@ -859,6 +857,7 @@ "This will move this message and all following messages to a new thread. Continue?": "Cela déplacera ce message et tous les messages suivants dans une nouvelle conversation. Continuer ?", "Thread access removed": "Accès à la conversation supprimé", "Thread has been split successfully.": "La conversation a été séparée avec succès.", + "Thread list": "Liste des conversations", "Thursday": "Jeudi", "Timezone": "Fuseau horaire", "To": "À", diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss index 7790ccec9..7143432cf 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss @@ -34,6 +34,14 @@ border-color: var(--c--contextuals--border--semantic--neutral--tertiary); } +// Keyboard focus ring, drawn inward so the scroll container does not clip +// it. Outline does not conflict with the state backgrounds, so it stays +// visible on selected and active rows too. +.thread-item:focus-visible { + outline: 2px solid var(--c--contextuals--border--focus); + outline-offset: -2px; +} + .thread-item.thread-item--active { background-color: var(--c--contextuals--background--semantic--brand--tertiary); border-color: var(--c--contextuals--border--semantic--brand--tertiary); @@ -78,6 +86,16 @@ flex: 1; } +// This wrapper carries aria-hidden="true", so it must stay a real box in the +// layout. `display: contents` is handled inconsistently across browser/AT +// combinations and can drop the aria-hidden boundary, leaking the checkbox +// semantics into the parent role="option". inline-flex shrink-wraps the +// checkbox so the visual layout stays identical. +.thread-item__checkbox-wrapper { + display: inline-flex; + flex-shrink: 0; +} + .thread-item__checkbox { flex-shrink: 0; margin: 0; diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx index d72c376a5..9d20901a9 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx @@ -16,16 +16,18 @@ import { LabelBadge } from "@/features/ui/components/label-badge" import { useLayoutDragContext } from "@/features/layouts/components/layout-context" import ViewHelper from "@/features/utils/view-helper" import useCanEditThreads from "@/features/message/use-can-edit-threads" +import { ThreadListboxItemProps } from "../../hooks/use-thread-listbox" type ThreadItemProps = { thread: Thread isSelected: boolean - onToggleSelection: (threadId: string, shiftKey: boolean, ctrlKey: boolean, arrowUpKey?: 'up' | 'down') => void + onToggle: (threadId: string) => void + onSelectRange: (threadId: string) => void selectedThreadIds: Set isSelectionMode: boolean -} +} & ThreadListboxItemProps -export const ThreadItem = ({ thread, isSelected, onToggleSelection, selectedThreadIds, isSelectionMode }: ThreadItemProps) => { +export const ThreadItem = ({ thread, isSelected, onToggle, onSelectRange, selectedThreadIds, isSelectionMode, tabIndex, itemRef, onFocusItem }: ThreadItemProps) => { const { t, i18n } = useTranslation(); const params = useParams({ strict: false }) as { mailboxId?: string; threadId?: string } const [isDragging, setIsDragging] = useState(false) @@ -76,26 +78,29 @@ export const ThreadItem = ({ thread, isSelected, onToggleSelection, selectedThre const handleCheckboxClick = (e: React.MouseEvent) => { e.stopPropagation(); - onToggleSelection(thread.id, e.shiftKey, e.ctrlKey || e.metaKey); + e.preventDefault(); + onFocusItem(); + if (e.shiftKey) { + onSelectRange(thread.id); + } else { + onToggle(thread.id); + } }; const handleItemClick = (e: React.MouseEvent) => { - // If using modifier keys or in selection mode, toggle selection instead of navigating - if (e.shiftKey || e.ctrlKey || e.metaKey || hasSelection) { + onFocusItem(); + // Keyboard activation (Enter on the focused option) fires a click + // with detail === 0: let it navigate even while a selection is + // active, it is the only pointer-free way to open a thread. + if (e.detail === 0) return; + if (e.shiftKey) { e.preventDefault(); - onToggleSelection(thread.id, e.shiftKey, e.ctrlKey || e.metaKey); - } - // Otherwise, let the Link handle navigation normally - }; - - const handleKeyDown = (e: React.KeyboardEvent) => { - if (!hasSelection) return; - const arrowUpKey = e.key === 'ArrowUp'; - const arrowDownKey = e.key === 'ArrowDown'; - if (e.shiftKey && (arrowUpKey || arrowDownKey)) { + onSelectRange(thread.id); + } else if (e.ctrlKey || e.metaKey || hasSelection) { e.preventDefault(); - onToggleSelection(thread.id, e.shiftKey, e.ctrlKey || e.metaKey, arrowUpKey ? 'up' : 'down'); + onToggle(thread.id); } + // Otherwise, let the Link handle navigation normally }; const handleDragStart = (e: React.DragEvent) => { @@ -159,23 +164,30 @@ export const ThreadItem = ({ thread, isSelected, onToggleSelection, selectedThre 'thread-item--selected': isSelected, }, )} - data-thread-id={thread.id} data-unread={hasUnread} draggable onDragStart={handleDragStart} onDragEnd={handleDragEnd} onClick={handleItemClick} - onKeyDown={handleKeyDown} - tabIndex={0} + onFocus={onFocusItem} + tabIndex={tabIndex} + ref={itemRef} + role="option" + aria-selected={isSelected} >
{showCheckbox && ( - + // Mouse-only affordance: the option itself carries the + // selection state (aria-selected), a focusable checkbox + // would add a second tab stop inside the option. + )}
diff --git a/src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.test.ts b/src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.test.ts new file mode 100644 index 000000000..a78ac663e --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import { getNextFocusId, isListboxNavKey } from "./listbox-navigation"; + +const threads = ['t1', 't2', 't3'].map((id) => ({ id })); + +describe('getNextFocusId', () => { + it('moves focus down', () => { + expect(getNextFocusId(threads, 't1', 'ArrowDown')).toBe('t2'); + }); + + it('moves focus up', () => { + expect(getNextFocusId(threads, 't3', 'ArrowUp')).toBe('t2'); + }); + + it('clamps at the bottom edge', () => { + expect(getNextFocusId(threads, 't3', 'ArrowDown')).toBe('t3'); + }); + + it('clamps at the top edge', () => { + expect(getNextFocusId(threads, 't1', 'ArrowUp')).toBe('t1'); + }); + + it('jumps to the first thread on Home', () => { + expect(getNextFocusId(threads, 't3', 'Home')).toBe('t1'); + }); + + it('jumps to the last loaded thread on End', () => { + expect(getNextFocusId(threads, 't1', 'End')).toBe('t3'); + }); + + it('falls back to the first thread when no thread is focused', () => { + expect(getNextFocusId(threads, null, 'ArrowDown')).toBe('t1'); + }); + + it('falls back to the first thread when the focused thread left the list', () => { + expect(getNextFocusId(threads, 'gone', 'ArrowUp')).toBe('t1'); + }); + + it('returns null on an empty list', () => { + expect(getNextFocusId([], 't1', 'ArrowDown')).toBeNull(); + expect(getNextFocusId([], null, 'Home')).toBeNull(); + }); +}); + +describe('isListboxNavKey', () => { + it('accepts navigation keys', () => { + expect(isListboxNavKey('ArrowUp')).toBe(true); + expect(isListboxNavKey('ArrowDown')).toBe(true); + expect(isListboxNavKey('Home')).toBe(true); + expect(isListboxNavKey('End')).toBe(true); + }); + + it('rejects other keys', () => { + expect(isListboxNavKey('Enter')).toBe(false); + expect(isListboxNavKey(' ')).toBe(false); + expect(isListboxNavKey('a')).toBe(false); + }); +}); diff --git a/src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.ts b/src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.ts new file mode 100644 index 000000000..8b1ddbdee --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/hooks/listbox-navigation.ts @@ -0,0 +1,36 @@ +/** + * Pure keyboard-navigation logic for the thread listbox. + * Kept framework-free so edge clamping can be unit-tested without React. + */ + +type ThreadRef = { id: string }; + +export type ListboxNavKey = 'ArrowUp' | 'ArrowDown' | 'Home' | 'End'; + +export const LISTBOX_NAV_KEYS: ReadonlyArray = ['ArrowUp', 'ArrowDown', 'Home', 'End']; + +export const isListboxNavKey = (key: string): key is ListboxNavKey => + (LISTBOX_NAV_KEYS as ReadonlyArray).includes(key); + +/** + * Resolve the thread that should receive focus after a navigation key. + * Arrow keys clamp at the list edges (no wrap-around). + * @returns the id of the thread to focus, or null when the list is empty + */ +export const getNextFocusId = ( + threads: ThreadRef[], + currentId: string | null, + key: ListboxNavKey, +): string | null => { + if (threads.length === 0) return null; + if (key === 'Home') return threads[0].id; + if (key === 'End') return threads[threads.length - 1].id; + + const currentIndex = currentId ? threads.findIndex((thread) => thread.id === currentId) : -1; + if (currentIndex === -1) return threads[0].id; + + const nextIndex = key === 'ArrowUp' + ? Math.max(0, currentIndex - 1) + : Math.min(threads.length - 1, currentIndex + 1); + return threads[nextIndex].id; +}; diff --git a/src/frontend/src/features/layouts/components/thread-panel/hooks/use-thread-listbox.ts b/src/frontend/src/features/layouts/components/thread-panel/hooks/use-thread-listbox.ts new file mode 100644 index 000000000..4dbc73501 --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/hooks/use-thread-listbox.ts @@ -0,0 +1,135 @@ +import { useCallback, useEffect, useRef } from "react"; +import { Thread } from "@/features/api/gen/models/thread"; +import { useThreadSelection } from "@/features/providers/thread-selection"; +import { useThreadListboxFocus } from "@/features/providers/thread-listbox-focus"; +import { getNextFocusId, isListboxNavKey } from "./listbox-navigation"; + +export type ThreadListboxItemProps = { + tabIndex: 0 | -1; + itemRef: (node: HTMLAnchorElement | null) => void; + onFocusItem: () => void; +}; + +/** + * Owns keyboard focus for the thread listbox (the selection provider owns + * what is selected): roving tabindex, arrow/Home/End navigation, Space + * toggle and Shift+Arrow range expansion. Enter needs no handling here — + * the focused anchor fires a native click (detail === 0) that the item's + * click handler lets through to navigation. + * + * The focus state itself lives in ThreadListboxFocusProvider because the + * ThreadPanel (and this hook with it) is remounted on route transitions; + * only the DOM refs map is local, it is rebuilt on every mount. + */ +export const useThreadListbox = (threads: Thread[] | undefined) => { + const { toggleThread, selectRange } = useThreadSelection(); + const { focusedThreadId, setFocusedThreadId, ownsFocusRef, lastFocusedIndexRef } = useThreadListboxFocus(); + const itemRefs = useRef(new Map()); + + const firstThreadId = threads?.[0]?.id ?? null; + + const getItemProps = useCallback((threadId: string): ThreadListboxItemProps => ({ + // Roving tabindex: a single tab stop, falling back to the first + // thread while nothing has been focused yet. + tabIndex: (focusedThreadId ?? firstThreadId) === threadId ? 0 : -1, + itemRef: (node: HTMLAnchorElement | null) => { + if (node) { + itemRefs.current.set(threadId, node); + } else { + itemRefs.current.delete(threadId); + } + }, + // Syncs state when focus arrives via Tab or click, and aligns DOM + // focus on the option anchor: Safari does not focus anchors on + // pointer click, and Chrome focuses the (aria-hidden) checkbox + // input on mousedown — both would leave Arrow/Home/End dead even + // though the roving state points at this item. The guard makes the + // call idempotent when invoked from the anchor's own focus event. + onFocusItem: () => { + ownsFocusRef.current = true; + setFocusedThreadId(threadId); + const node = itemRefs.current.get(threadId); + if (node && document.activeElement !== node) { + node.focus({ preventScroll: true }); + } + }, + }), [focusedThreadId, firstThreadId, setFocusedThreadId, ownsFocusRef]); + + // The list stops owning focus when it explicitly moves elsewhere. A blur + // with no relatedTarget (click on a non-focusable area) counts too — + // crucially, browsers fire NO blur at all when the focused node is + // unmounted, moved or recreated, which is how silent focus loss is told + // apart from a deliberate focus change. + const onBlur = useCallback((e: React.FocusEvent) => { + if (!(e.relatedTarget instanceof Node) || !e.currentTarget.contains(e.relatedTarget)) { + ownsFocusRef.current = false; + } + }, [ownsFocusRef]); + + // Restore focus dropped on when the focused item's node was + // unmounted, moved or recreated: list reorders (mark-as-read patch, + // pinned-threads merge) and full ThreadPanel remounts on route + // transitions. Without this, keyboard navigation dies after opening a + // thread. preventScroll: useScrollRestore already restores the list + // scroll position on remount, a focus-triggered scroll would fight it. + useEffect(() => { + if (!ownsFocusRef.current) return; + if (document.activeElement !== document.body) return; + const targetId = focusedThreadId ?? firstThreadId; + if (!targetId) return; + itemRefs.current.get(targetId)?.focus({ preventScroll: true }); + }); + + const onKeyDown = useCallback((e: React.KeyboardEvent) => { + if (!threads?.length) return; + + if (e.key === ' ') { + if (!focusedThreadId) return; + // Prevent page scroll on Space + e.preventDefault(); + toggleThread(focusedThreadId); + return; + } + + if (!isListboxNavKey(e.key) || e.ctrlKey || e.metaKey || e.altKey) return; + + const nextId = getNextFocusId(threads, focusedThreadId, e.key); + if (!nextId) return; + e.preventDefault(); + + const previousFocusId = focusedThreadId ?? undefined; + setFocusedThreadId(nextId); + const node = itemRefs.current.get(nextId); + node?.focus({ preventScroll: true }); + node?.scrollIntoView({ block: 'nearest' }); + + if (e.shiftKey && (e.key === 'ArrowUp' || e.key === 'ArrowDown')) { + // Finder-like expansion: the previous focus seeds the anchor + // when no anchor exists yet. + selectRange(nextId, previousFocusId); + } + }, [threads, focusedThreadId, toggleThread, selectRange, setFocusedThreadId]); + + // Keep the focused thread valid across refetch/prune: when it leaves + // the list, clamp to the same index. The restore effect above takes + // care of re-anchoring DOM focus after the state settles. + useEffect(() => { + if (!threads || focusedThreadId === null) return; + + const index = threads.findIndex((thread) => thread.id === focusedThreadId); + if (index !== -1) { + lastFocusedIndexRef.current = index; + return; + } + + if (threads.length === 0) { + setFocusedThreadId(null); + return; + } + + const clampedIndex = Math.min(lastFocusedIndexRef.current, threads.length - 1); + setFocusedThreadId(threads[clampedIndex].id); + }, [threads, focusedThreadId, setFocusedThreadId, lastFocusedIndexRef]); + + return { focusedThreadId, getItemProps, onKeyDown, onBlur }; +}; diff --git a/src/frontend/src/features/layouts/components/thread-panel/index.tsx b/src/frontend/src/features/layouts/components/thread-panel/index.tsx index 664f48d05..f48104dd3 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-panel/index.tsx @@ -10,6 +10,7 @@ import ThreadPanelHeader from "./components/thread-panel-header"; import { useThreadSelection } from "@/features/providers/thread-selection"; import { useScrollRestore } from "@/features/providers/scroll-restore"; import { useThreadPanelFilters } from "./hooks/use-thread-panel-filters"; +import { useThreadListbox } from "./hooks/use-thread-listbox"; export const ThreadPanel = () => { const { threads, queryStates, unselectThread, loadNextThreads, selectedThread, selectedMailbox } = useMailboxContext(); @@ -26,7 +27,8 @@ export const ThreadPanel = () => { const { selectedThreadIds, isSelectionMode, - toggleThreadSelection, + toggleThread, + selectRange, selectAllThreads, clearSelection, enableSelectionMode, @@ -36,6 +38,8 @@ export const ThreadPanel = () => { selectionStarredStatus, } = useThreadSelection(); + const { getItemProps, onKeyDown: handleListboxKeyDown, onBlur: handleListboxBlur } = useThreadListbox(threads?.results); + const handleObserver = useCallback((entries: IntersectionObserverEntry[]) => { const target = entries[0]; if (target.isIntersecting && threads?.next && !queryStates.threads.isFetchingNextPage) { @@ -114,15 +118,26 @@ export const ThreadPanel = () => {
) : ( -
+
{threads?.results.map((thread) => ( ))} {threads!.next && ( diff --git a/src/frontend/src/features/providers/thread-listbox-focus.tsx b/src/frontend/src/features/providers/thread-listbox-focus.tsx new file mode 100644 index 000000000..c74058e8b --- /dev/null +++ b/src/frontend/src/features/providers/thread-listbox-focus.tsx @@ -0,0 +1,46 @@ +import { createContext, Dispatch, PropsWithChildren, SetStateAction, useContext, useMemo, useRef, useState } from "react"; + +interface ThreadListboxFocusState { + focusedThreadId: string | null; + setFocusedThreadId: Dispatch>; + /** Whether the listbox currently owns keyboard focus. */ + ownsFocusRef: { current: boolean }; + /** Index of the last focused thread, used to clamp focus after prune. */ + lastFocusedIndexRef: { current: number }; +} + +const ThreadListboxFocusContext = createContext(null); + +/** + * Holds the thread listbox roving-focus state outside the ThreadPanel + * component: the panel is remounted on every route transition between + * "no thread open" and "thread open" (they are two distinct routes each + * mounting their own ThreadPanel), which would otherwise reset the + * focused thread and break keyboard navigation after opening a thread. + */ +export const ThreadListboxFocusProvider = ({ children }: PropsWithChildren) => { + const [focusedThreadId, setFocusedThreadId] = useState(null); + const ownsFocusRef = useRef(false); + const lastFocusedIndexRef = useRef(0); + + const value = useMemo(() => ({ + focusedThreadId, + setFocusedThreadId, + ownsFocusRef, + lastFocusedIndexRef, + }), [focusedThreadId]); + + return ( + + {children} + + ); +}; + +export const useThreadListboxFocus = () => { + const context = useContext(ThreadListboxFocusContext); + if (!context) { + throw new Error("useThreadListboxFocus must be used within a ThreadListboxFocusProvider"); + } + return context; +}; diff --git a/src/frontend/src/features/providers/thread-selection-core.test.ts b/src/frontend/src/features/providers/thread-selection-core.test.ts new file mode 100644 index 000000000..e85760df5 --- /dev/null +++ b/src/frontend/src/features/providers/thread-selection-core.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "vitest"; +import { + computeRange, + computeToggle, + pruneSelection, + resolveAnchorIndex, +} from "./thread-selection-core"; + +const threads = ['t1', 't2', 't3', 't4', 't5'].map((id) => ({ id })); + +describe('computeToggle', () => { + it('adds an unselected thread without affecting the others', () => { + const next = computeToggle(new Set(['t1', 't2']), 't4'); + expect(next).toEqual(new Set(['t1', 't2', 't4'])); + }); + + it('removes a selected thread without affecting the others', () => { + const next = computeToggle(new Set(['t1', 't2', 't4']), 't2'); + expect(next).toEqual(new Set(['t1', 't4'])); + }); + + it('does not mutate the previous selection', () => { + const prev = new Set(['t1']); + computeToggle(prev, 't2'); + expect(prev).toEqual(new Set(['t1'])); + }); + + it('selects a thread when the selection is empty', () => { + expect(computeToggle(new Set(), 't3')).toEqual(new Set(['t3'])); + }); + + it('empties the selection when toggling off the last thread', () => { + expect(computeToggle(new Set(['t3']), 't3')).toEqual(new Set()); + }); +}); + +describe('resolveAnchorIndex', () => { + it('uses the current anchor when it is still in the list', () => { + expect(resolveAnchorIndex(threads, 4, 't2', 't3', 't1')).toBe(1); + }); + + it('falls back to the fallback anchor when the anchor is gone', () => { + expect(resolveAnchorIndex(threads, 4, 'gone', 't3', 't1')).toBe(2); + }); + + it('seeds from the open thread when no anchor is usable', () => { + expect(resolveAnchorIndex(threads, 4, null, undefined, 't1')).toBe(0); + }); + + it('anchors on the target itself as a last resort', () => { + expect(resolveAnchorIndex(threads, 3, null)).toBe(3); + expect(resolveAnchorIndex(threads, 3, 'gone', 'gone-too', 'gone-as-well')).toBe(3); + }); +}); + +describe('computeRange', () => { + it('selects the inclusive range between anchor and target', () => { + expect(computeRange(threads, 1, 3)).toEqual(new Set(['t2', 't3', 't4'])); + }); + + it('handles a reversed range (target above anchor)', () => { + expect(computeRange(threads, 3, 1)).toEqual(new Set(['t2', 't3', 't4'])); + }); + + it('selects a single thread when anchor and target match', () => { + expect(computeRange(threads, 2, 2)).toEqual(new Set(['t3'])); + }); +}); + +describe('pruneSelection', () => { + it('drops ids that left the thread list', () => { + const pruned = pruneSelection(new Set(['t1', 'gone', 't3']), threads); + expect(pruned).toEqual(new Set(['t1', 't3'])); + }); + + it('returns the same reference when nothing changed', () => { + const prev = new Set(['t1', 't3']); + expect(pruneSelection(prev, threads)).toBe(prev); + }); + + it('returns the same reference when the selection is empty', () => { + const prev = new Set(); + expect(pruneSelection(prev, [])).toBe(prev); + }); + + it('prunes to empty when no selected thread remains', () => { + expect(pruneSelection(new Set(['gone']), threads)).toEqual(new Set()); + }); +}); diff --git a/src/frontend/src/features/providers/thread-selection-core.ts b/src/frontend/src/features/providers/thread-selection-core.ts new file mode 100644 index 000000000..eef053739 --- /dev/null +++ b/src/frontend/src/features/providers/thread-selection-core.ts @@ -0,0 +1,77 @@ +/** + * Pure selection logic for the thread multi-selection feature. + * + * These helpers are framework-free so the selection semantics (additive + * toggle, range anchoring, pruning) can be unit-tested without React. + */ + +type ThreadRef = { id: string }; + +/** + * Additively toggle a thread in/out of the selection without affecting + * the other selected threads. + * @returns a new Set with the thread added or removed + */ +export const computeToggle = (prev: Set, threadId: string): Set => { + const next = new Set(prev); + if (next.has(threadId)) { + next.delete(threadId); + } else { + next.add(threadId); + } + return next; +}; + +/** + * Resolve the index of the range-selection anchor. + * + * Resolution order: the current anchor (if still in the list), then the + * provided fallback (e.g. the previously focused thread), then the thread + * currently opened in the view, then the target itself. + * + * @param threads ordered list of visible threads + * @param targetIndex index of the thread the range extends to + * @param anchorId id of the current selection anchor, if any + * @param fallbackAnchorId id used to seed the anchor when none is set + * @param openThreadId id of the thread opened in the thread view, if any + * @returns the index to anchor the range on + */ +export const resolveAnchorIndex = ( + threads: ThreadRef[], + targetIndex: number, + anchorId: string | null, + fallbackAnchorId?: string, + openThreadId?: string, +): number => { + for (const candidate of [anchorId, fallbackAnchorId, openThreadId]) { + if (!candidate) continue; + const index = threads.findIndex((thread) => thread.id === candidate); + if (index !== -1) return index; + } + return targetIndex; +}; + +/** + * Build the selection covering the inclusive range between two indices. + * @returns a new Set containing exactly the threads in the range + */ +export const computeRange = ( + threads: ThreadRef[], + anchorIndex: number, + targetIndex: number, +): Set => { + const start = Math.min(anchorIndex, targetIndex); + const end = Math.max(anchorIndex, targetIndex); + return new Set(threads.slice(start, end + 1).map((thread) => thread.id)); +}; + +/** + * Drop selected ids that no longer exist in the thread list. + * @returns the same Set reference when nothing changed (to avoid re-renders) + */ +export const pruneSelection = (prev: Set, threads: ThreadRef[]): Set => { + if (prev.size === 0) return prev; + const threadIds = new Set(threads.map((thread) => thread.id)); + const pruned = new Set([...prev].filter((id) => threadIds.has(id))); + return pruned.size === prev.size ? prev : pruned; +}; diff --git a/src/frontend/src/features/providers/thread-selection.tsx b/src/frontend/src/features/providers/thread-selection.tsx index becf27093..460589a93 100644 --- a/src/frontend/src/features/providers/thread-selection.tsx +++ b/src/frontend/src/features/providers/thread-selection.tsx @@ -2,6 +2,7 @@ import { createContext, PropsWithChildren, useCallback, useContext, useEffect, u import { useUrlSearchParams } from "@/hooks/use-url-search-params"; import { useMailboxContext } from "./mailbox"; import { Thread } from "@/features/api/gen/models/thread"; +import { computeRange, computeToggle, pruneSelection, resolveAnchorIndex } from "./thread-selection-core"; export enum SelectionReadStatus { NONE = 'none', @@ -19,12 +20,8 @@ export enum SelectionStarredStatus { interface ThreadSelectionState { selectedThreadIds: Set; isSelectionMode: boolean; - toggleThreadSelection: ( - threadId: string, - shiftKey?: boolean, - ctrlKey?: boolean, - arrowUpKey?: 'up' | 'down' - ) => void; + toggleThread: (threadId: string) => void; + selectRange: (threadId: string, fallbackAnchorId?: string) => void; selectAllThreads: () => void; clearSelection: () => void; enableSelectionMode: () => void; @@ -40,118 +37,42 @@ const useThreadSelectionState = (threads: Thread[] | undefined, selectedThread: const searchParams = useUrlSearchParams(); const [selectedThreadIds, setSelectedThreadIds] = useState>(new Set()); const [isSelectionMode, setIsSelectionMode] = useState(false); - const lastActiveThreadIdRef = useRef(null); const anchorThreadIdRef = useRef(null); - const focusThreadIdRef = useRef(null); - const toggleThreadSelection = useCallback(( - threadId: string, - shiftKey: boolean = false, - ctrlKey: boolean = false, - arrowUpKey?: 'up' | 'down' - ) => { - if (!threads) return; - - setSelectedThreadIds((prev) => { - let newSet: Set; - - if (shiftKey && arrowUpKey) { - // Shift+Arrow key: macOS Finder-like behavior - if (anchorThreadIdRef.current === null || focusThreadIdRef.current === null) { - if (prev.size > 0) { - const firstSelectedId = Array.from(prev)[0]; - anchorThreadIdRef.current = firstSelectedId; - focusThreadIdRef.current = firstSelectedId; - } else { - anchorThreadIdRef.current = threadId; - focusThreadIdRef.current = threadId; - } - } - - const currentFocusIndex = threads.findIndex((t) => t.id === focusThreadIdRef.current); - if (currentFocusIndex === -1) { - // Focused thread was removed from list, reset to current thread - focusThreadIdRef.current = threadId; - anchorThreadIdRef.current = threadId; - newSet = new Set([threadId]); - } else { - let newFocusIndex = currentFocusIndex; - if (arrowUpKey === 'up' && newFocusIndex > 0) { - newFocusIndex = newFocusIndex - 1; - } else if (arrowUpKey === 'down' && newFocusIndex < threads.length - 1) { - newFocusIndex = newFocusIndex + 1; - } - - const newFocusThreadId = threads[newFocusIndex].id; - focusThreadIdRef.current = newFocusThreadId; - - const anchorIndex = threads.findIndex((t) => t.id === anchorThreadIdRef.current); - const effectiveAnchorIndex = anchorIndex !== -1 ? anchorIndex : newFocusIndex; - const start = Math.min(effectiveAnchorIndex, newFocusIndex); - const end = Math.max(effectiveAnchorIndex, newFocusIndex); - const range = threads.slice(start, end + 1); - newSet = new Set(range.map((thread) => thread.id)); - - setTimeout(() => { - const threadItem = document.querySelector(`[data-thread-id="${newFocusThreadId}"]`); - threadItem?.focus(); - }, 0); - } - } - else if (shiftKey) { - // Shift+Click: range selection - const index = threads.findIndex((t) => t.id === threadId); - let anchorIndex: number; - - if (lastActiveThreadIdRef.current !== null) { - const foundIndex = threads.findIndex((t) => t.id === lastActiveThreadIdRef.current); - anchorIndex = foundIndex !== -1 ? foundIndex : index; - } else if (selectedThread) { - const activeThreadIndex = threads.findIndex((t) => t.id === selectedThread.id); - anchorIndex = activeThreadIndex !== -1 ? activeThreadIndex : index; - lastActiveThreadIdRef.current = selectedThread.id; - } else { - anchorIndex = index; - lastActiveThreadIdRef.current = threadId; - } - - anchorThreadIdRef.current = threads[anchorIndex]?.id ?? null; - focusThreadIdRef.current = threadId; - - const start = Math.min(anchorIndex, index); - const end = Math.max(anchorIndex, index); - const range = threads.slice(start, end + 1); - newSet = new Set(range.map((thread) => thread.id)); - } else if (ctrlKey) { - // Ctrl/Cmd+Click: toggle individual without affecting others - newSet = new Set(prev); - if (newSet.has(threadId)) { - newSet.delete(threadId); - } else { - newSet.add(threadId); - } - lastActiveThreadIdRef.current = threadId; - focusThreadIdRef.current = threadId; - } else { - // Normal click: if already selected, unselect it; otherwise, clear others and select only this one - if (prev.has(threadId)) { - newSet = new Set(prev); - newSet.delete(threadId); - } else { - newSet = new Set([threadId]); - } - lastActiveThreadIdRef.current = threadId; - anchorThreadIdRef.current = threadId; - focusThreadIdRef.current = threadId; - } - - if (newSet.size > 0) { - setIsSelectionMode(true); - } + /** + * Additively toggle a thread in/out of the selection. The toggled + * thread becomes the anchor for subsequent range selections. + * Selection mode stays on even when the selection empties, so the + * bulk-action header does not flicker mid-interaction. + */ + const toggleThread = useCallback((threadId: string) => { + setSelectedThreadIds((prev) => computeToggle(prev, threadId)); + anchorThreadIdRef.current = threadId; + setIsSelectionMode(true); + }, []); - return newSet; - }); - }, [threads, selectedThread]); + /** + * Select the range between the current anchor and the given thread. + * Successive range selections pivot from the same anchor. + * @param fallbackAnchorId seeds the anchor when none is set (e.g. the + * previously focused thread during Shift+Arrow keyboard expansion) + */ + const selectRange = useCallback((threadId: string, fallbackAnchorId?: string) => { + if (!threads) return; + const targetIndex = threads.findIndex((thread) => thread.id === threadId); + if (targetIndex === -1) return; + + const anchorIndex = resolveAnchorIndex( + threads, + targetIndex, + selectedThreadIds.size > 0 ? anchorThreadIdRef.current : null, + fallbackAnchorId, + selectedThread?.id, + ); + anchorThreadIdRef.current = threads[anchorIndex].id; + setSelectedThreadIds(computeRange(threads, anchorIndex, targetIndex)); + setIsSelectionMode(true); + }, [threads, selectedThread, selectedThreadIds.size]); const selectAllThreads = useCallback(() => { if (!threads) return; @@ -162,9 +83,7 @@ const useThreadSelectionState = (threads: Thread[] | undefined, selectedThread: const clearSelection = useCallback(() => { setSelectedThreadIds(new Set()); - lastActiveThreadIdRef.current = null; anchorThreadIdRef.current = null; - focusThreadIdRef.current = null; setIsSelectionMode(false); }, []); @@ -202,11 +121,8 @@ const useThreadSelectionState = (threads: Thread[] | undefined, selectedThread: useEffect(() => { if (!threads) return; setSelectedThreadIds((prev) => { - if (prev.size === 0) return prev; - const threadIds = new Set(threads.map((t) => t.id)); - const pruned = new Set([...prev].filter((id) => threadIds.has(id))); - if (pruned.size === prev.size) return prev; - if (pruned.size === 0) { + const pruned = pruneSelection(prev, threads); + if (pruned !== prev && pruned.size === 0) { setIsSelectionMode(false); } return pruned; @@ -216,9 +132,7 @@ const useThreadSelectionState = (threads: Thread[] | undefined, selectedThread: // Clear selection when search params change useEffect(() => { setSelectedThreadIds(new Set()); - lastActiveThreadIdRef.current = null; anchorThreadIdRef.current = null; - focusThreadIdRef.current = null; setIsSelectionMode(false); }, [searchParams]); @@ -257,12 +171,13 @@ const useThreadSelectionState = (threads: Thread[] | undefined, selectedThread: return () => { document.removeEventListener('keydown', handleKeyDown, true); }; - }, [selectedThreadIds.size, isSelectionMode, clearSelection, selectAllThreads]); + }, [isSelectionMode, clearSelection, selectAllThreads]); return { selectedThreadIds, isSelectionMode, - toggleThreadSelection, + toggleThread, + selectRange, selectAllThreads, clearSelection, enableSelectionMode, diff --git a/src/frontend/src/routes/mailbox/$mailboxId/route.tsx b/src/frontend/src/routes/mailbox/$mailboxId/route.tsx index 101da8be3..b5c2af4aa 100644 --- a/src/frontend/src/routes/mailbox/$mailboxId/route.tsx +++ b/src/frontend/src/routes/mailbox/$mailboxId/route.tsx @@ -1,12 +1,15 @@ import { createFileRoute, Outlet } from "@tanstack/react-router"; import { MainLayout } from "@/features/layouts/components/main"; +import { ThreadListboxFocusProvider } from "@/features/providers/thread-listbox-focus"; import { ThreadSelectionProvider } from "@/features/providers/thread-selection"; const MailboxLayoutRoute = () => ( - + + + );