From 1069c9fac0ba93d6ca2963f39c2d4c8a0f628576 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 05:30:37 +0100 Subject: [PATCH] fix: keep the composer's colour and table menus on screen and let Escape close only them --- .../email/__tests__/composer-mention.test.tsx | 10 +++++++ .../rich-text-editor-popover-shift.test.ts | 22 ++++++++++++++ components/email/email-composer.tsx | 6 ++-- components/email/rich-text-editor.tsx | 29 +++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 components/email/__tests__/rich-text-editor-popover-shift.test.ts diff --git a/components/email/__tests__/composer-mention.test.tsx b/components/email/__tests__/composer-mention.test.tsx index 22bb38aca..f11d5ad8f 100644 --- a/components/email/__tests__/composer-mention.test.tsx +++ b/components/email/__tests__/composer-mention.test.tsx @@ -528,6 +528,16 @@ describe('composer with the @ recipient list', () => { expect(onClose).not.toHaveBeenCalled(); }); + it('lets Escape in a formatting menu close only the menu', async () => { + const { onClose } = await renderComposer(); + fireEvent.click(screen.getByTitle('text_color')); + const menu = await screen.findByRole('menu', { name: 'text_color' }); + fireEvent.keyDown(menu.querySelector('button')!, { key: 'Escape' }); + expect(screen.queryByRole('menu', { name: 'text_color' })).toBeNull(); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(onClose).not.toHaveBeenCalled(); + }); + // The directory picker is drawn inside the composer itself, not portaled. it('lets Escape in the directory picker leave the composer alone', async () => { // Every other capability answers nothing. diff --git a/components/email/__tests__/rich-text-editor-popover-shift.test.ts b/components/email/__tests__/rich-text-editor-popover-shift.test.ts new file mode 100644 index 000000000..377c7836f --- /dev/null +++ b/components/email/__tests__/rich-text-editor-popover-shift.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; +import { popoverShift } from '../rich-text-editor'; + +describe('popoverShift', () => { + const bounds = { left: 0, right: 320 }; + + it('leaves a popover that fits where it is', () => { + expect(popoverShift({ left: 20, right: 200 }, bounds)).toBe(0); + }); + + it('pulls one past the end back inside, with the margin', () => { + expect(popoverShift({ left: 180, right: 350 }, bounds)).toBe(-38); + }); + + it('pushes one past the start back inside, for right-to-left', () => { + expect(popoverShift({ left: -30, right: 140 }, bounds)).toBe(38); + }); + + it('keeps the start edge inside when it is wider than the bounds', () => { + expect(popoverShift({ left: 100, right: 500 }, bounds)).toBe(-92); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 88af7572e..0a08d2a28 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -4206,8 +4206,10 @@ export function EmailComposer({ // through React's tree although the dialog portals to the body: they // belong to that dialog, and Escape must close only it. if (!(e.target instanceof Node) || !e.currentTarget.contains(e.target)) return; - // Same for a dialog drawn inside the composer (the directory picker). - const targetDialog = e.target instanceof Element ? e.target.closest('[role="dialog"],[role="alertdialog"]') : null; + // Same for what is drawn inside the composer: the directory picker and the + // formatting toolbar's menus, which close on Escape themselves. This + // handler runs in the capture phase, so they would never see it. + const targetDialog = e.target instanceof Element ? e.target.closest('[role="dialog"],[role="alertdialog"],[role="menu"]') : null; if (targetDialog && e.currentTarget.contains(targetDialog)) return; const isPlainEscape = e.key === 'Escape' && !e.ctrlKey && !e.metaKey && !e.altKey && !e.shiftKey; diff --git a/components/email/rich-text-editor.tsx b/components/email/rich-text-editor.tsx index 2b013c6ae..2c24d8e0d 100644 --- a/components/email/rich-text-editor.tsx +++ b/components/email/rich-text-editor.tsx @@ -205,6 +205,22 @@ const ToolbarButton = React.forwardRef 0 ? -overEnd : 0; + const overStart = bounds.left + margin - (popover.left + shift); + return overStart > 0 ? shift + overStart : shift; +} + /** * A toolbar popover (font size, colours, table) that works from the * keyboard: it takes the focus when it opens (the checked entry, else the @@ -239,6 +255,19 @@ function ToolbarPopover({ (checked ?? list[0])?.focus(); }, []); + // Anchored to its button, it would run off a narrow composer's edge. + useLayoutEffect(() => { + const el = ref.current; + const toolbar = el?.closest('[role="toolbar"]'); + if (!el || !toolbar) return; + const box = toolbar.getBoundingClientRect(); + const shift = popoverShift(el.getBoundingClientRect(), { + left: Math.max(box.left, 0), + right: Math.min(box.right, window.innerWidth), + }); + if (shift !== 0) el.style.transform = `translateX(${shift}px)`; + }, []); + const close = (returnFocus: boolean) => { onClose(); if (returnFocus) triggerRef.current?.focus();