From 7bbfe6178331b9be7caf10a219b1b39a2c9f44ff Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 28 Oct 2026 04:18:11 +0100 Subject: [PATCH] fix: keep keys pressed in the external recipient confirm away from the mail shortcuts --- .../external-recipient-confirm-keys.test.tsx | 64 +++++++++++++++++++ .../email/external-recipient-confirm.tsx | 22 ++++++- 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 components/email/__tests__/external-recipient-confirm-keys.test.tsx diff --git a/components/email/__tests__/external-recipient-confirm-keys.test.tsx b/components/email/__tests__/external-recipient-confirm-keys.test.tsx new file mode 100644 index 000000000..1fd546120 --- /dev/null +++ b/components/email/__tests__/external-recipient-confirm-keys.test.tsx @@ -0,0 +1,64 @@ +import { render, act, fireEvent, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { ExternalRecipientConfirmHost } from '../external-recipient-confirm'; +import { useExternalRecipientConfirmStore } from '@/stores/external-recipient-confirm-store'; +import { resetSendPipeline } from '@/lib/outgoing/send-pipeline'; + +// The confirm dialog keeps its keys: the mail view's shortcuts listen on the +// window, and must not see Escape (close the message), j/k (move the list) +// or Enter (open a message) while the question is open. Real focus trap. + +const RECIPIENTS = [{ address: 'org-dave@org-two.test', domain: 'org-two.test', field: 'to' as const }]; +const pageKeys = vi.fn(); + +beforeEach(() => { + useExternalRecipientConfirmStore.setState({ pending: null, queue: [] }); + window.addEventListener('keydown', pageKeys); +}); +afterEach(() => { + window.removeEventListener('keydown', pageKeys); + pageKeys.mockReset(); + cleanup(); + resetSendPipeline(); +}); + +function open() { + const view = render(); + let answer!: Promise; + act(() => { answer = useExternalRecipientConfirmStore.getState().ask(RECIPIENTS); }); + const back = view.container.querySelector('[data-external-recipient-back]') as HTMLButtonElement; + const send = view.container.querySelector('[data-external-recipient-send]') as HTMLButtonElement; + return { answer, back, send }; +} + +describe('keys in the external recipient confirm', () => { + it('puts the focus on Back and keeps shortcut keys from the page', () => { + const { back } = open(); + expect(document.activeElement).toBe(back); + for (const key of ['j', 'k', 'Enter', 'Escape']) { + act(() => { fireEvent.keyDown(document.activeElement!, { key }); }); + // Re-open after Escape and Enter answered it. + if (!useExternalRecipientConfirmStore.getState().pending) act(() => { void useExternalRecipientConfirmStore.getState().ask(RECIPIENTS); }); + } + expect(pageKeys).not.toHaveBeenCalled(); + }); + + it('answers Back on Escape', async () => { + const { answer } = open(); + act(() => { fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); }); + expect(await answer).toBe(false); + }); + + it('presses the focused button on Enter and on Space', async () => { + const first = open(); + first.send.focus(); + act(() => { fireEvent.keyDown(first.send, { key: 'Enter' }); }); + expect(await first.answer).toBe(true); + + let second!: Promise; + act(() => { second = useExternalRecipientConfirmStore.getState().ask(RECIPIENTS); }); + act(() => { fireEvent.keyDown(document.activeElement!, { key: ' ' }); }); + // The focus went back to the first button, Back. + expect(await second).toBe(false); + }); +}); diff --git a/components/email/external-recipient-confirm.tsx b/components/email/external-recipient-confirm.tsx index 29ceafb05..0a3130a28 100644 --- a/components/email/external-recipient-confirm.tsx +++ b/components/email/external-recipient-confirm.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useEffect, useId } from 'react'; +import { useEffect, useId, type KeyboardEvent } from 'react'; import { useTranslations } from 'next-intl'; import { Building } from '@/components/icons'; import { Button } from '@/components/ui/button'; @@ -25,7 +25,24 @@ export function ExternalRecipientConfirmHost() { // A send can't be waiting on a dialog that is gone. useEffect(() => () => useExternalRecipientConfirmStore.getState().cancelAll(), []); - const dialogRef = useFocusTrap({ isActive: !!pending, onEscape: () => answer(false), restoreFocus: true }); + // Escape is handled below with the other keys, not by the trap: one key, one answer. + const dialogRef = useFocusTrap({ isActive: !!pending, restoreFocus: true }); + + // Keys stay in the dialog: Escape must not also close the message behind + // it, j/k must not move the list, Enter must not open a message. Enter and + // Space press the focused button here, whatever a page listener did. + const onKeyDown = (e: KeyboardEvent) => { + e.stopPropagation(); + if (e.key === 'Escape') { + e.preventDefault(); + answer(false); + return; + } + if ((e.key === 'Enter' || e.key === ' ') && e.target instanceof HTMLButtonElement) { + e.preventDefault(); + e.target.click(); + } + }; if (!pending) return null; const { recipients } = pending; @@ -37,6 +54,7 @@ export function ExternalRecipientConfirmHost() { onMouseDown={(e) => { if (e.target === e.currentTarget) answer(false); }} + onKeyDown={onKeyDown} >