From 508e17bc119563a8894e8c6ee8142bc1b2f31daf Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 24 Oct 2026 01:21:28 +0000 Subject: [PATCH] fix(assistant): the phone chat holds the focus as a dialog, and its lists close on Escape The full screen chat is a modal dialog but left the focus on the trip behind it. It now takes the focus when shown, keeps Tab inside and hands the focus back when set aside or closed, through the helper MSheet now shares. The dropdown panels take the focus onto their first entry, close on Escape and give the focus back to their button; the history and menu buttons say whether their panel is open, and the open chat is announced as current. --- .../src/mobile/components/MDropdownPanel.tsx | 32 +++++++++++- client/src/mobile/components/MIconBtn.tsx | 10 +++- client/src/mobile/components/MListRow.tsx | 4 ++ client/src/mobile/components/MSheet.tsx | 31 ++--------- client/src/mobile/components/modalFocus.ts | 41 +++++++++++++++ .../trip/assistant/MAssistantScreen.tsx | 20 ++++++-- .../mobile/trip/MAssistantScreen.test.tsx | 51 ++++++++++++++++++- 7 files changed, 153 insertions(+), 36 deletions(-) create mode 100644 client/src/mobile/components/modalFocus.ts diff --git a/client/src/mobile/components/MDropdownPanel.tsx b/client/src/mobile/components/MDropdownPanel.tsx index 0d442b403..6e5c9e11a 100644 --- a/client/src/mobile/components/MDropdownPanel.tsx +++ b/client/src/mobile/components/MDropdownPanel.tsx @@ -1,4 +1,4 @@ -import { ReactNode } from 'react' +import { ReactNode, useEffect, useLayoutEffect, useRef } from 'react' interface MDropdownPanelProps { open: boolean @@ -12,9 +12,36 @@ interface MDropdownPanelProps { * Floating popover panel (user menu, filter dropdowns): heavy glass, r20, * closed by a transparent full-screen backdrop — no dim, unlike sheets. * Layered above the scrolling content but below the bottom nav (z-40) and - * sheets (z-60), matching the design's stacking order. + * sheets (z-60), matching the design's stacking order. Opened, it takes the + * focus onto its first button and Escape closes it; closed, the focus goes + * back to the button it was opened from, unless a pick moved it on. */ export default function MDropdownPanel({ open, onClose, className = '', children }: MDropdownPanelProps) { + const panelRef = useRef(null) + const onCloseRef = useRef(onClose) + useLayoutEffect(() => { + onCloseRef.current = onClose + }) + + useEffect(() => { + if (!open) return + const opener = document.activeElement instanceof HTMLElement ? document.activeElement : null + panelRef.current?.querySelector('button:not(:disabled)')?.focus({ preventScroll: true }) + // Captured and stopped, so the key closes the panel and nothing under it. + const onKey = (e: KeyboardEvent) => { + if (e.key !== 'Escape') return + e.stopPropagation() + onCloseRef.current() + } + document.addEventListener('keydown', onKey, true) + return () => { + document.removeEventListener('keydown', onKey, true) + // The focused button left with the panel: the focus fell back to the page, and goes to the opener. + const lost = document.activeElement === null || document.activeElement === document.body + if (lost && opener?.isConnected) opener.focus({ preventScroll: true }) + } + }, [open]) + if (!open) return null return ( @@ -23,6 +50,7 @@ export default function MDropdownPanel({ open, onClose, className = '', children no role of its own; the panel's own controls are the keyboard path. */}
{children} diff --git a/client/src/mobile/components/MIconBtn.tsx b/client/src/mobile/components/MIconBtn.tsx index 6a703c675..a8acc00c0 100644 --- a/client/src/mobile/components/MIconBtn.tsx +++ b/client/src/mobile/components/MIconBtn.tsx @@ -1,4 +1,4 @@ -import { ReactNode } from 'react' +import { type AriaAttributes, ReactNode } from 'react' interface MIconBtnProps { onClick?: () => void @@ -10,6 +10,10 @@ interface MIconBtnProps { className?: string /** Dimmed and inert. For a button whose position should stay put even when it cannot act. */ disabled?: boolean + /** For a button that opens a panel: whether it is open now, said to a screen reader. */ + ariaExpanded?: boolean + /** For a button that opens a panel: what kind it is (`menu` for a menu). */ + ariaHaspopup?: AriaAttributes['aria-haspopup'] children: ReactNode } @@ -21,6 +25,8 @@ export default function MIconBtn({ size = 38, className = '', disabled = false, + ariaExpanded, + ariaHaspopup, children, }: MIconBtnProps) { return ( @@ -28,6 +34,8 @@ export default function MIconBtn({ type="button" onClick={onClick} aria-label={ariaLabel} + aria-expanded={ariaExpanded} + aria-haspopup={ariaHaspopup} disabled={disabled} className={`relative flex flex-none items-center justify-center rounded-full text-m-ink disabled:opacity-35 ${ variant === 'glass' diff --git a/client/src/mobile/components/MListRow.tsx b/client/src/mobile/components/MListRow.tsx index 1f2bfecc1..4caec7387 100644 --- a/client/src/mobile/components/MListRow.tsx +++ b/client/src/mobile/components/MListRow.tsx @@ -8,6 +8,8 @@ interface MListRowProps { danger?: boolean onClick?: () => void className?: string + /** The row is the one open now (a chat in a history list), said so to a screen reader too. */ + current?: boolean } /** Menu/list row (user menu style): icon 16 in --m-muted, 13.5px semibold label. */ @@ -18,11 +20,13 @@ export default function MListRow({ danger = false, onClick, className = '', + current = false, }: MListRowProps) { return (
)}
- + - setPanel('menu')}> + setPanel('menu')} + > @@ -242,6 +255,7 @@ export function AssistantScreen({ tripId, tripTitle, hidden, hops, onOpenTripCha key={th.id} label={th.title} className={th.id === thread.threadId ? 'bg-[color:var(--m-ic)]' : ''} + current={th.id === thread.threadId} onClick={() => pick(() => thread.openThread(th.id))} trailing={ diff --git a/client/tests/unit/mobile/trip/MAssistantScreen.test.tsx b/client/tests/unit/mobile/trip/MAssistantScreen.test.tsx index fda1aa4ef..958c95386 100644 --- a/client/tests/unit/mobile/trip/MAssistantScreen.test.tsx +++ b/client/tests/unit/mobile/trip/MAssistantScreen.test.tsx @@ -8,11 +8,11 @@ import { chatOf, useAssistantStore } from '../../../../src/store/assistantStore' import { buildPlanner, buildShell } from '../../../helpers/mobileTrip'; import { act, cleanup, fireEvent, render, screen } from '../../../helpers/render'; -// FE-MOB-ASSIST-001 to FE-MOB-ASSIST-015 +// FE-MOB-ASSIST-001 to FE-MOB-ASSIST-017 // // The phone's full-screen chat around the shared chat surface, which is // replaced here: what is tested is the frame, the header and the way to the -// map and back. +// map and back, and where the focus goes as the chat and its lists open. const surface = vi.hoisted(() => ({ onOpen: null as ((target: EntityTarget) => void) | null, @@ -129,6 +129,53 @@ describe('MAssistantScreen', () => { expect(thread.openThread).toHaveBeenCalledWith(3); }); + it('FE-MOB-ASSIST-016: the chat takes the focus when it opens, keeps Tab inside and hands the focus back', () => { + const outside = document.createElement('button'); + document.body.appendChild(outside); + outside.focus(); + try { + renderScreen(); + const dialog = screen.getByRole('dialog', { name: 'Assistant' }); + expect(dialog).toHaveFocus(); + fireEvent.keyDown(dialog, { key: 'Tab', shiftKey: true }); + const more = screen.getByRole('button', { name: 'More options' }); + expect(more).toHaveFocus(); + fireEvent.keyDown(more, { key: 'Tab' }); + expect(screen.getByRole('button', { name: 'Back' })).toHaveFocus(); + + // Set aside for the map, the chat gives the focus back to where it was. + act(() => useAssistantStore.getState().setMinimized(true)); + expect(outside).toHaveFocus(); + } finally { + outside.remove(); + } + }); + + it('FE-MOB-ASSIST-017: the recent chats and the menu say whether they are open, and Escape closes them onto their button', () => { + renderScreen(); + const history = screen.getByRole('button', { name: 'Recent chats' }); + expect(history).toHaveAttribute('aria-expanded', 'false'); + history.focus(); + fireEvent.click(history); + expect(history).toHaveAttribute('aria-expanded', 'true'); + // The list takes the focus onto its first entry and says which chat is open. + expect(screen.getAllByRole('button', { name: 'New chat' })[1]).toHaveFocus(); + expect(screen.getByRole('button', { name: /Temples on day 2/ })).toHaveAttribute('aria-current', 'true'); + fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' }); + expect(screen.queryByText('Chats are deleted after 90 days.')).not.toBeInTheDocument(); + expect(history).toHaveFocus(); + expect(history).toHaveAttribute('aria-expanded', 'false'); + + const more = screen.getByRole('button', { name: 'More options' }); + expect(more).toHaveAttribute('aria-haspopup', 'menu'); + more.focus(); + fireEvent.click(more); + expect(more).toHaveAttribute('aria-expanded', 'true'); + fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' }); + expect(screen.queryByRole('button', { name: 'Delete chat' })).not.toBeInTheDocument(); + expect(more).toHaveFocus(); + }); + it('FE-MOB-ASSIST-006: deleting the chat asks first', () => { renderScreen(); fireEvent.click(screen.getByRole('button', { name: 'More options' }));