feat(dialogs): the focus starts inside a dialog and Tab stays there

Every dialog, the shared frame and the older modal alike, moves the focus in when it opens: on a desktop into the first text field, on a phone onto the panel so no keyboard pops up. Tab wraps around inside instead of running through the page behind, and closing hands the focus back to what opened it. Fields that open a list on focus are skipped, and the older modal's close button got a label and a tooltip. (#1302)

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}

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"

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"

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),

client/src/components/Planner/TransitSearchPanel.tsx

@@ -256,6 +256,8 @@ function StopPicker({ label, value, onPick, quickPicks, near, placeholder }: {
         <input
           value={display}
           onChange={e => 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"

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()

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}

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<string, string> = {
   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 ? <ModalFrame {...frame} /> : null
+}
+
+function ModalFrame({
   onClose,
   title,
   children,
@@ -41,28 +47,37 @@ export default function Modal({
   footer,
   hideCloseButton = false,
   align = 'center',
-}: ModalProps) {
+}: Omit<ModalProps, 'isOpen'>) {
+  const { t } = useTranslation()
+  const panelRef = useRef<HTMLDivElement>(null)
+  const mouseDownTarget = useRef<EventTarget | null>(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<EventTarget | null>(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(
     <div
@@ -79,9 +94,12 @@ export default function Modal({
       }}
     >
       <div
+        ref={panelRef}
         role="presentation"
+        tabIndex={-1}
+        onKeyDown={e => { 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({
         <div className="flex items-center justify-between p-6 flex-shrink-0 border-b border-edge-secondary">
           <h2 className="text-lg font-semibold text-content">{title}</h2>
           {!hideCloseButton && (
+            <Tooltip label={t('common.close')}>
             <button type="button"
               onClick={onClose}
-              className="p-2 rounded-lg text-slate-400 hover:text-slate-600 hover:bg-slate-100 transition-colors"
+              aria-label={t('common.close')}
+              className="p-2 rounded-lg text-content-faint hover:text-content-secondary hover:bg-surface-hover transition-colors"
             >
               <X className="w-5 h-5" />
             </button>
+            </Tooltip>
           )}
         </div>
 

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 (
+    <DialogShell onClose={() => {}} labelledBy="t" header={<h2 id="t">Title</h2>}>
+      {children}
+    </DialogShell>
+  )
+}
+
+describe('firstTextField', () => {
+  it('FE-SHARED-DIALOGFOCUS-001: skips buttons, read-only, disabled and list-opening fields', () => {
+    const panel = document.createElement('div')
+    panel.innerHTML = `
+      <button>Close</button>
+      <input type="checkbox" />
+      <input type="text" readonly />
+      <input type="text" disabled />
+      <input type="text" data-no-autofocus />
+      <input role="combobox" />
+      <div hidden><input type="text" /></div>
+      <input type="text" id="target" />
+      <textarea></textarea>`
+    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 = '<button>OK</button>'
+    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(<Shell><button>Pick</button><input aria-label="Name" /></Shell>)
+    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(<Shell><input aria-label="Name" /></Shell>)
+    expect(screen.getByRole('dialog')).toHaveFocus()
+  })
+
+  it('FE-SHARED-DIALOGFOCUS-005: a field that took the focus itself keeps it', () => {
+    restore = asDesktop()
+    render(<Shell><input aria-label="First" /><input aria-label="Search" autoFocus /></Shell>)
+    expect(screen.getByLabelText('Search')).toHaveFocus()
+  })
+
+  it('FE-SHARED-DIALOGFOCUS-006: Tab wraps from the last control to the first and back', () => {
+    render(<Shell><button>One</button><button>Two</button></Shell>)
+    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(<Shell><button>One</button><button>Two</button><button>Three</button></Shell>)
+    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 (
+      <>
+        <button onClick={() => setOpen(true)}>Open</button>
+        <Modal isOpen={open} onClose={() => setOpen(false)} title="Edit">
+          <input aria-label="Title" />
+          <button>Save</button>
+        </Modal>
+      </>
+    )
+  }
+
+  it('FE-SHARED-DIALOGFOCUS-008: the focus moves in on open and back to the opener on close', () => {
+    restore = asDesktop()
+    render(<Harness />)
+    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(<Harness />)
+    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()
+  })
+})

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<HTMLElement>(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<HTMLElement>, 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<HTMLElement>(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()
+  }
+}