From 69ec26fe333990444eeed68d06199d875a8ef00a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 08:43:29 +0100 Subject: [PATCH] fix: remove a recipient whole on the second Backspace instead of turning it into text The mention test's Backspace check now expects the chip to be selected, not put back into the field as text. --- .../email/__tests__/composer-a11y.test.tsx | 40 +++++++++++++++++++ .../email/__tests__/composer-mention.test.tsx | 6 ++- components/email/email-composer.tsx | 40 +++++++++++++++---- 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/components/email/__tests__/composer-a11y.test.tsx b/components/email/__tests__/composer-a11y.test.tsx index 9e9207abd..f71f24a0d 100644 --- a/components/email/__tests__/composer-a11y.test.tsx +++ b/components/email/__tests__/composer-a11y.test.tsx @@ -307,6 +307,46 @@ describe('recipient fields (SR-06, CV-02)', () => { } }); + // compose-6: Backspace never leaves half an address behind. + it('selects the last chip on the first Backspace and removes it whole on the second', () => { + render(, Zoe Fischer ' }} />); + const input = screen.getByRole('combobox', { name: 'to:' }) as HTMLInputElement; + input.focus(); + fireEvent.keyDown(input, { key: 'Backspace' }); + let chips = within(screen.getByRole('list', { name: 'to:' })).getAllByRole('listitem'); + expect(chips).toHaveLength(2); + expect(document.activeElement).toBe(chips[1]); + expect(input.value).toBe(''); + + fireEvent.keyDown(chips[1], { key: 'Backspace' }); + chips = within(screen.getByRole('list', { name: 'to:' })).getAllByRole('listitem'); + expect(chips).toHaveLength(1); + expect(chips[0]).toHaveTextContent('Anna Berg'); + expect(input.value).toBe(''); + expect(document.activeElement).toBe(input); + + // Three presses take out two recipients, never part of one. + fireEvent.keyDown(input, { key: 'Backspace' }); + fireEvent.keyDown(document.activeElement!, { key: 'Backspace' }); + expect(screen.queryAllByRole('listitem')).toHaveLength(0); + expect(input.value).toBe(''); + }); + + it('lets go of a selected chip on Escape and keeps the composer open', () => { + const onClose = vi.fn(); + render(); + const input = screen.getByRole('combobox', { name: 'to:' }) as HTMLInputElement; + input.focus(); + fireEvent.keyDown(input, { key: 'Backspace' }); + const chip = within(screen.getByRole('list', { name: 'to:' })).getByRole('listitem'); + expect(document.activeElement).toBe(chip); + fireEvent.keyDown(chip, { key: 'Escape' }); + expect(document.activeElement).toBe(input); + expect(screen.getAllByRole('listitem')).toHaveLength(1); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.queryByRole('alertdialog')).toBeNull(); + }); + it('announces a recipient added by typing', () => { render(); expect(liveRegion()).toHaveTextContent(''); diff --git a/components/email/__tests__/composer-mention.test.tsx b/components/email/__tests__/composer-mention.test.tsx index b3e8a9be7..ef4e15f1b 100644 --- a/components/email/__tests__/composer-mention.test.tsx +++ b/components/email/__tests__/composer-mention.test.tsx @@ -541,12 +541,14 @@ describe('composer with the @ recipient list', () => { expect(onClose).not.toHaveBeenCalled(); }); - it('pulls the last recipient back whole on Backspace, without the key deleting its last character', async () => { + // The first Backspace selects the last recipient, it doesn't turn it into text. + it('selects the last recipient on Backspace, without the key deleting anything', async () => { await renderComposer({ to: 'Sarah Kim ' }); const to = within(screen.getByTestId('composer-to')).getByRole('combobox') as HTMLInputElement; // fireEvent returns false when the handler stopped the key's default action. expect(fireEvent.keyDown(to, { key: 'Backspace' })).toBe(false); - expect(to.value).toBe('Sarah Kim '); + expect(to.value).toBe(''); + expect(document.activeElement).toHaveAttribute('data-recipient-chip'); }); // compose-17: the button was there only with a server directory, so diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index dbe0f2adc..e55816505 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -4811,6 +4811,8 @@ export function EmailComposer({ if (isPlainEscape) { if (activeAutoField) return; + // A selected recipient: Escape only lets go of it (its own handler). + if (e.target instanceof Element && e.target.closest('[data-recipient-chip]')) return; // Same for the "@" recipient list in the body: this handler runs in the // capture phase, before the editor could close the list itself. if (isRecipientMentionActive(editorRef.current)) return; @@ -6283,6 +6285,9 @@ export function RecipientChipInput({ // one by default: Shift+Tab from the input lands next to it. const [rovingChip, setRovingChip] = useState(null); const chipRefs = useRef>([]); + // The chip a Backspace in the empty input selected: a second one removes it + // and goes back to typing. + const backspaceChipRef = useRef(null); const errorId = `${labelId}-error`; const chipHintId = `${labelId}-chip-hint`; const fieldNames: Record<'to' | 'cc' | 'bcc', string> = { to: t('to'), cc: t('cc'), bcc: t('bcc') }; @@ -6316,6 +6321,25 @@ export function RecipientChipInput({ const handleChipKeyDown = (e: React.KeyboardEvent, index: number) => { if (e.target !== e.currentTarget) return; + const fromInput = backspaceChipRef.current === index; + backspaceChipRef.current = null; + // Escape lets go of the chip (the composer stays open); typing goes on in the input. + if (e.key === 'Escape') { + e.preventDefault(); + inputRef.current?.focus(); + return; + } + if (fromInput && (e.key === 'Backspace' || e.key === 'Delete')) { + e.preventDefault(); + onChipsChange(chips.filter((_, i) => i !== index)); + setRovingChip(null); + inputRef.current?.focus(); + return; + } + if (e.key.length === 1 && !e.ctrlKey && !e.metaKey && !e.altKey && e.key !== ' ') { + inputRef.current?.focus(); + return; + } const rtl = typeof window !== 'undefined' && getComputedStyle(e.currentTarget).direction === 'rtl'; const back = rtl ? 'ArrowRight' : 'ArrowLeft'; const forward = rtl ? 'ArrowLeft' : 'ArrowRight'; @@ -6500,14 +6524,13 @@ export function RecipientChipInput({ return; } - // Backspace on an empty input pulls the last chip back into the input for - // quick editing. + // Backspace on an empty input selects the last chip; the next one removes + // it whole. It used to put the address back as text, so a few presses + // left half an address behind. Editing is a double-click, Enter or F2. if (e.key === 'Backspace' && !inputText && chips.length > 0) { - // Or the key goes on to delete the closing ">" of what was just put back. e.preventDefault(); - const lastChip = chips[chips.length - 1]; - onChipsChange(chips.slice(0, -1)); - onInputChange(formatRecipient(lastChip.name, lastChip.email)); + backspaceChipRef.current = chips.length - 1; + focusChip(chips.length - 1); return; } }; @@ -6702,12 +6725,15 @@ export function RecipientChipInput({ : ( customColor && colorStyles[customColor] ? `${colorStyles[customColor]} cursor-grab active:cursor-grabbing` : "bg-secondary text-secondary-foreground hover:bg-accent cursor-grab active:cursor-grabbing"), - !isEditing && draggingIndex === i && "opacity-50" + !isEditing && draggingIndex === i && "opacity-50", + // Selected: by Backspace in the field, arrows or Tab. + !isEditing && "focus:outline-none focus:ring-2 focus:ring-primary focus:border-primary" )} onContextMenu={isEditing ? undefined : (e) => handleContextMenu(e, i, chip)} data-invalid={isInvalid || undefined} title={isInvalid ? t('invalid_recipient') : undefined} role="listitem" + data-recipient-chip="" ref={(el) => { chipRefs.current[i] = el; }} tabIndex={isEditing ? undefined : i === tabChip ? 0 : -1} aria-describedby={isEditing ? undefined : chipHintId}