diff --git a/client/src/components/Planner/AddressInput.tsx b/client/src/components/Planner/AddressInput.tsx index 332fca8d1..ea415beb3 100644 --- a/client/src/components/Planner/AddressInput.tsx +++ b/client/src/components/Planner/AddressInput.tsx @@ -89,6 +89,8 @@ export default function AddressInput({ value, onChange, placeholder, className } value={value} placeholder={placeholder} onChange={e => { onChange(e.target.value); setOpen(true); search(e.target.value) }} + // Opens its list on focus, so a dialog must not focus it by itself (#1302). + data-no-autofocus onFocus={() => setOpen(true)} onKeyDown={onKey} className={className} diff --git a/client/src/components/Planner/AirportSelect.tsx b/client/src/components/Planner/AirportSelect.tsx index 42ea3c9b8..caec9ff69 100644 --- a/client/src/components/Planner/AirportSelect.tsx +++ b/client/src/components/Planner/AirportSelect.tsx @@ -113,6 +113,8 @@ export default function AirportSelect({ value, onChange, placeholder, style }: P value={query} placeholder={placeholder ?? t('airport.searchPlaceholder')} onChange={(e) => { setQuery(e.target.value); setOpen(true); if (value) onChange(null) }} + // Opens its list on focus, so a dialog must not focus it by itself (#1302). + data-no-autofocus onFocus={() => setOpen(true)} onKeyDown={onKey} className="bg-transparent text-content" diff --git a/client/src/components/Planner/LocationSelect.tsx b/client/src/components/Planner/LocationSelect.tsx index 9c45ff763..3d8dde4e5 100644 --- a/client/src/components/Planner/LocationSelect.tsx +++ b/client/src/components/Planner/LocationSelect.tsx @@ -115,6 +115,8 @@ export default function LocationSelect({ value, onChange, placeholder, style, pl value={query} placeholder={placeholder ?? t('reservations.searchLocation')} onChange={(e) => { setQuery(e.target.value); setOpen(true); setHighlight(-1); if (value) onChange(null) }} + // Opens its list on focus, so a dialog must not focus it by itself (#1302). + data-no-autofocus onFocus={() => setOpen(true)} onKeyDown={onKey} className="bg-transparent text-content" diff --git a/client/src/components/Planner/PlaceFormModal.tsx b/client/src/components/Planner/PlaceFormModal.tsx index 5914d557d..2b137b916 100644 --- a/client/src/components/Planner/PlaceFormModal.tsx +++ b/client/src/components/Planner/PlaceFormModal.tsx @@ -939,7 +939,9 @@ export default function PlaceFormModal(props: PlaceFormModalProps) { labelId={titleId} onClose={onClose} eyebrow={title} - // Never focused on opening: a new place starts at the search, an edit at the dialog. + // Not autoFocused itself: a new place starts at the search, and otherwise the + // dialog's own first-field focus lands here on a desktop (#1302), so a place + // brought in from the map or opened to edit is ready for its name. titleInput={{ value: form.name, onChange: value => handleChange('name', value), diff --git a/client/src/components/Planner/TransitSearchPanel.tsx b/client/src/components/Planner/TransitSearchPanel.tsx index b8ddbf7b2..86d9dc821 100644 --- a/client/src/components/Planner/TransitSearchPanel.tsx +++ b/client/src/components/Planner/TransitSearchPanel.tsx @@ -256,6 +256,8 @@ function StopPicker({ label, value, onPick, quickPicks, near, placeholder }: { search(e.target.value)} + // Opens its list on focus, so a dialog must not focus it by itself (#1302). + data-no-autofocus onFocus={() => setOpen(true)} placeholder={placeholder} className="text-content" diff --git a/client/src/components/Roadtrip/RoadtripStayModal.test.tsx b/client/src/components/Roadtrip/RoadtripStayModal.test.tsx index 8b456192b..da6bca46a 100644 --- a/client/src/components/Roadtrip/RoadtripStayModal.test.tsx +++ b/client/src/components/Roadtrip/RoadtripStayModal.test.tsx @@ -217,7 +217,10 @@ describe('RoadtripStayModal for a stop left at a set time', () => { it('FE-STAYMODAL-019: closing changes nothing', () => { const { onClose, onSave } = open(leaving) - fireEvent.click(screen.getByRole('button', { name: /^close$/i })) + // The footer's Close; the header's X answers to the same name and closes the same way. + const closeButtons = screen.getAllByRole('button', { name: /^close$/i }) + expect(closeButtons).toHaveLength(2) + fireEvent.click(closeButtons[1]) expect(onClose).toHaveBeenCalled() expect(onSave).not.toHaveBeenCalled() expect(setAssignmentTimes).not.toHaveBeenCalled() diff --git a/client/src/components/shared/DialogShell.tsx b/client/src/components/shared/DialogShell.tsx index 7daae2735..4f1ec48bc 100644 --- a/client/src/components/shared/DialogShell.tsx +++ b/client/src/components/shared/DialogShell.tsx @@ -4,6 +4,7 @@ import { Trash2, X } from 'lucide-react' import { useTranslation } from '../../i18n' import { lockBodyScroll } from '../../utils/bodyScrollLock' import { Tooltip } from './Tooltip' +import { focusDialog, trapTab } from './dialogFocus' /** A font size that follows the user's text size setting for its tier. */ export const fs = (px: number, tier: 'caption' | 'body' | 'subtitle' = 'caption'): CSSProperties => ({ fontSize: `calc(${px}px * var(--fs-scale-${tier}, 1))` }) @@ -61,13 +62,14 @@ function DialogFrame({ onClose, labelledBy, width = 'detail', align = 'center', return () => document.removeEventListener('keydown', onKey) }, [blocked, onClose]) - // The page stays put behind the dialog, and whatever opened it gets the focus back. + // The page stays put behind the dialog, the focus moves into it (the first text + // field on a desktop, #1302), and whatever opened it gets the focus back. useEffect(() => { const panel = panelRef.current const focusedInside = !!panel && panel.contains(document.activeElement) const opener = focusedInside ? focusedBefore : document.activeElement const release = lockBodyScroll() - if (!focusedInside) panel?.focus() + if (!focusedInside && panel) focusDialog(panel) return () => { release() if (opener instanceof HTMLElement) opener.focus() @@ -100,6 +102,7 @@ function DialogFrame({ onClose, labelledBy, width = 'detail', align = 'center', aria-labelledby={labelledBy} tabIndex={-1} onPaste={onPaste} + onKeyDown={e => { if (panelRef.current) trapTab(e, panelRef.current) }} className={`trek-modal-enter flex max-h-full w-full ${PANEL_WIDTH[width]} flex-col overflow-hidden rounded-[22px] bg-surface-card shadow-2xl outline-none`} > {header} diff --git a/client/src/components/shared/Modal.tsx b/client/src/components/shared/Modal.tsx index 57f0bb65c..4814166db 100644 --- a/client/src/components/shared/Modal.tsx +++ b/client/src/components/shared/Modal.tsx @@ -1,7 +1,10 @@ -import React, { useEffect, useCallback, useRef } from 'react' +import React, { useEffect, useCallback, useRef, useState } from 'react' import { createPortal } from 'react-dom' import { X } from 'lucide-react' import { lockBodyScroll } from '../../utils/bodyScrollLock' +import { useTranslation } from '../../i18n' +import { Tooltip } from './Tooltip' +import { focusDialog, trapTab } from './dialogFocus' const sizeClasses: Record = { sm: 'max-w-sm', @@ -32,8 +35,11 @@ interface ModalProps { align?: 'center' | 'top' } -export default function Modal({ - isOpen, +export default function Modal({ isOpen, ...frame }: ModalProps) { + return isOpen ? : null +} + +function ModalFrame({ onClose, title, children, @@ -41,28 +47,37 @@ export default function Modal({ footer, hideCloseButton = false, align = 'center', -}: ModalProps) { +}: Omit) { + const { t } = useTranslation() + const panelRef = useRef(null) + const mouseDownTarget = useRef(null) + // Read while rendering, before a field inside can take the focus with autoFocus. + const [focusedBefore] = useState(() => document.activeElement) + const handleEsc = useCallback((e: KeyboardEvent) => { if (e.key === 'Escape') onClose() }, [onClose]) useEffect(() => { - if (!isOpen) return document.addEventListener('keydown', handleEsc) return () => document.removeEventListener('keydown', handleEsc) - }, [isOpen, handleEsc]) + }, [handleEsc]) // Separate from the key listener so a new onClose identity does not release // and re-take the lock on every render. The shared lock is ref-counted: this // modal must not clear a lock another overlay is still holding (#1809). + useEffect(() => lockBodyScroll(), []) + + // The focus moves into the dialog when it opens (the first text field on a + // desktop) and back to whatever opened it when it closes, the way DialogShell + // does it (#1302). Without this Tab ran through the page behind first. useEffect(() => { - if (!isOpen) return - return lockBodyScroll() - }, [isOpen]) - - const mouseDownTarget = useRef(null) - - if (!isOpen) return null + const panel = panelRef.current + if (panel) focusDialog(panel) + return () => { + if (focusedBefore instanceof HTMLElement && focusedBefore.isConnected) focusedBefore.focus() + } + }, [focusedBefore]) return createPortal(
{ if (panelRef.current) trapTab(e, panelRef.current) }} className={` - trek-modal-enter + trek-modal-enter outline-none rounded-2xl overflow-hidden shadow-2xl w-full ${sizeClasses[size] || sizeClasses.md} flex flex-col max-h-[calc(100dvh-var(--bottom-nav-h)-90px)] sm:max-h-[calc(100dvh-90px)] @@ -93,12 +111,15 @@ export default function Modal({

{title}

{!hideCloseButton && ( + + )}
diff --git a/client/src/components/shared/dialogFocus.test.tsx b/client/src/components/shared/dialogFocus.test.tsx new file mode 100644 index 000000000..55e70178e --- /dev/null +++ b/client/src/components/shared/dialogFocus.test.tsx @@ -0,0 +1,141 @@ +// FE-SHARED-DIALOGFOCUS-001 to FE-SHARED-DIALOGFOCUS-009 +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useState } from 'react' +import { render, screen, fireEvent } from '../../../tests/helpers/render' +import { DialogShell } from './DialogShell' +import Modal from './Modal' +import { firstTextField } from './dialogFocus' + +/** A desktop with a mouse: the only case the first field takes the focus. */ +function asDesktop() { + const original = window.matchMedia + window.matchMedia = vi.fn().mockImplementation((query: string) => ({ + matches: query === '(pointer: fine)', + media: query, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + onchange: null, + dispatchEvent: vi.fn(), + })) as unknown as typeof window.matchMedia + return () => { window.matchMedia = original } +} + +let restore: (() => void) | null = null +afterEach(() => { restore?.(); restore = null }) + +function Shell({ children }: { children: React.ReactNode }) { + return ( + {}} labelledBy="t" header={

Title

}> + {children} + + ) +} + +describe('firstTextField', () => { + it('FE-SHARED-DIALOGFOCUS-001: skips buttons, read-only, disabled and list-opening fields', () => { + const panel = document.createElement('div') + panel.innerHTML = ` + + + + + + + + + ` + expect(firstTextField(panel)?.id).toBe('target') + }) + + it('FE-SHARED-DIALOGFOCUS-002: a dialog with no field has none', () => { + const panel = document.createElement('div') + panel.innerHTML = '' + expect(firstTextField(panel)).toBeNull() + }) +}) + +describe('DialogShell focus (#1302)', () => { + it('FE-SHARED-DIALOGFOCUS-003: on a desktop the first text field is ready to type into', () => { + restore = asDesktop() + render() + expect(screen.getByLabelText('Name')).toHaveFocus() + }) + + it('FE-SHARED-DIALOGFOCUS-004: on a touch screen the panel takes the focus, so no keyboard pops up', () => { + render() + expect(screen.getByRole('dialog')).toHaveFocus() + }) + + it('FE-SHARED-DIALOGFOCUS-005: a field that took the focus itself keeps it', () => { + restore = asDesktop() + render() + expect(screen.getByLabelText('Search')).toHaveFocus() + }) + + it('FE-SHARED-DIALOGFOCUS-006: Tab wraps from the last control to the first and back', () => { + render() + const one = screen.getByRole('button', { name: 'One' }) + const two = screen.getByRole('button', { name: 'Two' }) + + two.focus() + fireEvent.keyDown(two, { key: 'Tab' }) + expect(one).toHaveFocus() + + fireEvent.keyDown(one, { key: 'Tab', shiftKey: true }) + expect(two).toHaveFocus() + + // From the panel itself, Shift+Tab goes to the last control rather than the page. + const panel = screen.getByRole('dialog') + panel.focus() + fireEvent.keyDown(panel, { key: 'Tab', shiftKey: true }) + expect(two).toHaveFocus() + }) + + it('FE-SHARED-DIALOGFOCUS-007: Tab in the middle is left to the browser', () => { + render() + const two = screen.getByRole('button', { name: 'Two' }) + two.focus() + const event = new KeyboardEvent('keydown', { key: 'Tab', bubbles: true, cancelable: true }) + two.dispatchEvent(event) + expect(event.defaultPrevented).toBe(false) + }) +}) + +describe('Modal focus (#1302)', () => { + function Harness() { + const [open, setOpen] = useState(false) + return ( + <> + + setOpen(false)} title="Edit"> + + + + + ) + } + + it('FE-SHARED-DIALOGFOCUS-008: the focus moves in on open and back to the opener on close', () => { + restore = asDesktop() + render() + const opener = screen.getByRole('button', { name: 'Open' }) + opener.focus() + fireEvent.click(opener) + expect(screen.getByLabelText('Title')).toHaveFocus() + + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + expect(screen.queryByLabelText('Title')).toBeNull() + expect(opener).toHaveFocus() + }) + + it('FE-SHARED-DIALOGFOCUS-009: Tab stays inside the modal', () => { + render() + fireEvent.click(screen.getByRole('button', { name: 'Open' })) + const save = screen.getByRole('button', { name: 'Save' }) + save.focus() + fireEvent.keyDown(save, { key: 'Tab' }) + expect(screen.getByRole('button', { name: 'Close' })).toHaveFocus() + }) +}) diff --git a/client/src/components/shared/dialogFocus.ts b/client/src/components/shared/dialogFocus.ts new file mode 100644 index 000000000..0d45fc1a0 --- /dev/null +++ b/client/src/components/shared/dialogFocus.ts @@ -0,0 +1,81 @@ +import type { KeyboardEvent as ReactKeyboardEvent } from 'react' + +/** + * Keyboard focus for the app's dialogs (#1302): the first Tab lands in the dialog + * instead of running through the page behind it, Tab wraps around inside it, and + * on a desktop the first text field is ready to type into. + * + * Fields that open a list as soon as they get the focus (an address or airport + * search) carry role="combobox" and are skipped, so a dialog never opens with a + * dropdown already hanging off it. A field can opt out with data-no-autofocus. + */ +const TEXT_FIELD = [ + 'input:not([type]),input[type="text"],input[type="search"],input[type="email"],input[type="url"],input[type="tel"],input[type="number"],input[type="password"]', + 'textarea', +].join(',') + +const TABBABLE = [ + 'a[href]', 'button', 'input:not([type="hidden"])', 'select', 'textarea', + '[tabindex]', '[contenteditable="true"]', +].join(',') + +function usable(el: HTMLElement): boolean { + if ((el as HTMLInputElement).disabled) return false + if (el.closest('[hidden],[aria-hidden="true"],[inert]')) return false + return el.getAttribute('tabindex') !== '-1' +} + +/** The first field a user would type into, or null when the dialog has none. */ +export function firstTextField(panel: HTMLElement): HTMLElement | null { + for (const el of panel.querySelectorAll(TEXT_FIELD)) { + if (!usable(el)) continue + if ((el as HTMLInputElement).readOnly) continue + if (el.getAttribute('role') === 'combobox' || el.hasAttribute('aria-haspopup') || el.hasAttribute('data-no-autofocus')) continue + return el + } + return null +} + +/** Only with a mouse or trackpad: on a phone a focused field would throw the keyboard up. */ +function typingDevice(): boolean { + if (typeof window.matchMedia !== 'function') return false + return window.matchMedia('(pointer: fine)').matches && !window.matchMedia('(max-width: 767px)').matches +} + +/** + * Puts the focus into a dialog that just opened, unless something inside already + * took it (a field with autoFocus, a dialog's own choice). The panel itself is + * the fallback, so the next Tab starts inside the dialog either way. + */ +export function focusDialog(panel: HTMLElement): void { + if (panel.contains(document.activeElement)) return + const field = typingDevice() ? firstTextField(panel) : null + ;(field ?? panel).focus({ preventScroll: true }) +} + +/** + * Keeps Tab inside the dialog: past the last control it wraps to the first, and + * Shift+Tab from the first (or from the panel) to the last. A key pressed in a + * picker list the dialog opened in a portal on the body is left alone, because + * that list is not inside the panel and handles its own keys. + */ +export function trapTab(e: ReactKeyboardEvent, panel: HTMLElement): void { + if (e.key !== 'Tab' || e.defaultPrevented || e.altKey || e.ctrlKey || e.metaKey) return + const target = e.target as Node + if (target !== panel && !panel.contains(target)) return + const items = Array.from(panel.querySelectorAll(TABBABLE)).filter(usable) + if (items.length === 0) { + e.preventDefault() + panel.focus({ preventScroll: true }) + return + } + const first = items[0] + const last = items[items.length - 1] + if (e.shiftKey && (target === first || target === panel)) { + e.preventDefault() + last.focus() + } else if (!e.shiftKey && target === last) { + e.preventDefault() + first.focus() + } +}