fix(ui): convert overlay modals to native dialog elements

frontend/package.json

@@ -28,6 +28,7 @@
     "react-router-dom": "^6.28.0"
   },
   "devDependencies": {
+    "@playwright/test": "1.63.0",
     "@types/react": "^18.3.18",
     "@types/react-dom": "^18.3.5",
     "@vitejs/plugin-react": "^4.3.4",

frontend/src/components/AnnotationPanelControls.tsx

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 // © 2026 Joshua Benjamin Jewell. All rights reserved.
 // Licensed under the GNU Affero General Public License version 3 (AGPLv3).
-import { useEffect, useState } from 'react'
+import { useEffect, useRef, useState } from 'react'
 import { api } from '../api/client'
 import { errorMessage } from '../api/errorMessage'
 import { useToast } from './Toast'
@@ -88,23 +88,25 @@ export function AddFuncdbModal({
     }
   }
 
-  // Escape is the keyboard equivalent of clicking the backdrop. Without it this
-  // dialog can be opened but not dismissed without a pointer.
-  useEffect(() => {
-    const onKey = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose() }
-    document.addEventListener('keydown', onKey)
-    return () => document.removeEventListener('keydown', onKey)
-  }, [onClose])
+  const dialogRef = useRef<HTMLDialogElement>(null)
+
+  useEffect(() => { dialogRef.current?.showModal() }, [])
+
+  // Native <dialog> via showModal() (#32): the browser supplies the dialog
+  // role, aria-modal, the focus trap and Escape-to-cancel that the old
+  // presentational backdrop div lacked. Dismissals route through the dialog's
+  // own close(), so the `close` event -- and therefore onClose -- fires
+  // exactly once. Backdrop-click-to-dismiss is dropped deliberately: this is a
+  // FORM dialog, and losing typed input to a stray click outside the panel is
+  // hostile. Escape and Cancel remain, both keyboard-reachable.
+  const dismiss = () => dialogRef.current?.close()
 
   return (
-    <div
-      role="presentation"
-      style={{
-        position: 'fixed', inset: 0, zIndex: 1000,
-        background: 'rgba(0,0,0,.45)', display: 'flex',
-        alignItems: 'center', justifyContent: 'center',
-      }}
-      onClick={event => { if (event.target === event.currentTarget) onClose() }}
+    <dialog
+      ref={dialogRef}
+      className="mm-modal"
+      aria-label="Add FuncDB Entry"
+      onClose={onClose}
     >
       <div style={{
         background: 'var(--color-bg)', borderRadius: 8, padding: '20px 24px',
@@ -150,13 +152,13 @@ export function AddFuncdbModal({
             />
           </label>
           <div style={{ display: 'flex', gap: 8, justifyContent: 'flex-end' }}>
-            <button type="button" className="btn" onClick={onClose}>Cancel</button>
+            <button type="button" className="btn" onClick={dismiss}>Cancel</button>
             <button type="submit" className="btn btn-primary" disabled={saving}>
               {saving ? 'Saving...' : 'Add entry'}
             </button>
           </div>
         </form>
       </div>
-    </div>
+    </dialog>
   )
 }

frontend/src/components/NameDialog.tsx

@@ -10,16 +10,38 @@ interface Props {
   onClose: () => void
 }
 
+// Native <dialog> opened with showModal() (#32). The browser supplies what the
+// old presentational backdrop div hand-rolled badly or not at all: the dialog
+// role, aria-modal, a focus trap, Escape-to-cancel, and focus restoration to
+// the invoking control on close.
+//
+// Backdrop-click-to-dismiss is dropped deliberately, not lost (#32, criterion
+// 6): these are FORM dialogs, and discarding typed input on a stray click
+// outside the panel is hostile. Escape and Cancel stay as the dismissals, and
+// both are keyboard-reachable. Dropping the click handler is also the only
+// way to hold SonarCloud S1082/S6819 at zero on these files — the issue's own
+// history is one rule's cure minting the other rule's finding.
 export function NameDialog({ title, initialValue = '', placeholder = 'Name', onConfirm, onClose }: Props) {
   const [value, setValue] = useState(initialValue)
   const [error, setError] = useState('')
   const [busy, setBusy] = useState(false)
   const inputRef = useRef<HTMLInputElement>(null)
+  const dialogRef = useRef<HTMLDialogElement>(null)
 
   useEffect(() => {
+    // showModal() records the currently focused element -- the invoking
+    // control -- as the one to restore on close. That record is taken at THIS
+    // instant, so the input must not be focused before it: an autoFocus
+    // attribute would move focus first and the dialog would then restore
+    // focus to an element inside itself, dropping it on unmount. Focus the
+    // input explicitly after the modal opens instead.
+    dialogRef.current?.showModal()
+    inputRef.current?.focus()
     inputRef.current?.select()
   }, [])
 
+  const dismiss = () => dialogRef.current?.close()
+
   const handleSubmit = async (e: React.FormEvent) => {
     e.preventDefault()
     if (!value.trim()) return
@@ -34,14 +56,11 @@ export function NameDialog({ title, initialValue = '', placeholder = 'Name', onC
   }
 
   return (
-    <div
-      role="presentation"
-      style={{
-        position: 'fixed', inset: 0, zIndex: 1000,
-        background: 'rgba(0,0,0,.45)',
-        display: 'flex', alignItems: 'center', justifyContent: 'center',
-      }}
-      onClick={e => { if (e.target === e.currentTarget) onClose() }}
+    <dialog
+      ref={dialogRef}
+      className="mm-modal"
+      aria-label={title}
+      onClose={onClose}
     >
       <div style={{
         background: 'var(--color-bg)',
@@ -57,10 +76,8 @@ export function NameDialog({ title, initialValue = '', placeholder = 'Name', onC
             ref={inputRef}
             value={value}
             onChange={e => setValue(e.target.value)}
-            onKeyDown={e => { if (e.key === 'Escape') onClose() }}
             placeholder={placeholder}
             disabled={busy}
-            autoFocus
             style={{
               width: '100%',
               padding: '7px 10px',
@@ -77,7 +94,7 @@ export function NameDialog({ title, initialValue = '', placeholder = 'Name', onC
             <p style={{ color: '#c92a2a', fontSize: '.82rem', marginBottom: 8 }}>{error}</p>
           )}
           <div style={{ display: 'flex', gap: 8, justifyContent: 'flex-end' }}>
-            <button type="button" className="btn" onClick={onClose} disabled={busy}>
+            <button type="button" className="btn" onClick={dismiss} disabled={busy}>
               Cancel
             </button>
             <button type="submit" className="btn btn-primary" disabled={busy || !value.trim()}>
@@ -86,6 +103,6 @@ export function NameDialog({ title, initialValue = '', placeholder = 'Name', onC
           </div>
         </form>
       </div>
-    </div>
+    </dialog>
   )
 }

frontend/src/styles/app.css

@@ -279,3 +279,29 @@ a:hover  { text-decoration: underline; }
   color:       inherit;
   text-align:  inherit;
 }
+
+/* Native modal dialogs (#32). showModal() places the element in the top layer,
+   which brings the dialog role, aria-modal, focus containment and
+   Escape-to-cancel for free -- the behaviour the old presentational backdrop
+   divs hand-rolled badly or not at all.
+
+   The UA default border and padding are reset and `margin: auto` keeps the
+   user agent's centreing; the dimmed overlay lives on the ::backdrop
+   pseudo-element, which replaces the hand-painted rgba(0,0,0,.45) the old
+   fixed divs carried. Backdrop-click-to-dismiss is deliberately NOT provided:
+   these are form dialogs, Escape and Cancel are the dismissals, and a click
+   handler here would only re-mint the SonarCloud S1082 finding (#32).
+
+   The user agent's closed-state rule `dialog:not([open]) { display: none }`
+   outranks any author display rule, so nothing extra is needed to hide a
+   closed dialog. */
+.mm-modal {
+  margin: auto;
+  padding: 0;
+  border: none;
+  background: var(--color-bg);
+}
+
+.mm-modal::backdrop {
+  background: rgba(0, 0, 0, 0.45);
+}

frontend/tests/e2e/dialogs.e2e.ts

@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: MPL-2.0
+// SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk>
+// Dialog semantics e2e (#32). The two overlay modals were converted from
+// presentational backdrop divs to native <dialog> elements opened with
+// showModal(); the behaviours that conversion claims -- implicit dialog role,
+// Escape-to-cancel, focus containment, focus restoration -- are only
+// observable in a real browser, which is what this lane exists to provide.
+// Backdrop-click-to-dismiss is dropped deliberately (form dialogs: a stray
+// click must not discard typed input); the last test pins that decision.
+import { test, expect, type Page, type Locator } from '@playwright/test'
+
+// NameDialog is mounted by several views; StudiesView's "new study" flow is the
+// shallowest route to it. The backend may be absent under the dev server, which
+// does not matter here: the dialog's open/dismiss contract is client-side.
+async function openNameDialog(page: Page): Promise<Locator> {
+  await page.goto('/')
+  const newStudy = page.getByRole('button', { name: /new study/i }).first()
+  await expect(newStudy).toBeVisible({ timeout: 15000 })
+  await newStudy.click()
+  const dialog = page.locator('dialog')
+  await expect(dialog).toBeVisible()
+  return dialog
+}
+
+test('NameDialog is a native modal dialog with the implicit role', async ({ page }) => {
+  const dialog = await openNameDialog(page)
+  await expect(dialog).toHaveAttribute('open', '')
+  // The claim under test is what the ACCESSIBILITY TREE exposes: showModal()
+  // must put an implicit dialog role in front of assistive tech without the
+  // component remembering to set role or aria-modal attributes itself.
+  await expect(page.getByRole('dialog')).toBeVisible()
+  await expect(dialog).toHaveJSProperty('open', true)
+})
+
+test('Escape closes the dialog with no hand-rolled keydown listener', async ({ page }) => {
+  const dialog = await openNameDialog(page)
+  await page.keyboard.press('Escape')
+  await expect(dialog).not.toBeVisible()
+})
+
+test('Cancel closes the dialog and restores focus to the invoking control', async ({ page }) => {
+  const trigger = page.getByRole('button', { name: /new study/i }).first()
+  await page.goto('/')
+  await expect(trigger).toBeVisible({ timeout: 15000 })
+  await trigger.focus()
+  await trigger.click()
+  const dialog = page.locator('dialog')
+  await expect(dialog).toBeVisible()
+  await dialog.getByRole('button', { name: /cancel/i }).click()
+  await expect(dialog).not.toBeVisible()
+  // showModal() remembers the previously focused element; close() restores it.
+  await expect(trigger).toBeFocused()
+})
+
+test('focus stays inside the open dialog: Tab never reaches the page behind', async ({ page }) => {
+  await openNameDialog(page)
+  // Cycle Tab far enough to wrap if the trap were broken.
+  for (let i = 0; i < 12; i++) await page.keyboard.press('Tab')
+  const activeInDialog = await page.evaluate(() => {
+    const dialog = document.querySelector('dialog')
+    const active = document.activeElement
+    return !!dialog && !!active && dialog.contains(active)
+  })
+  expect(activeInDialog).toBe(true)
+})
+
+test('backdrop click deliberately does NOT dismiss a form dialog', async ({ page }) => {
+  // Decision recorded per #32 criterion 6: these dialogs carry typed input, so
+  // a stray click outside the panel must not discard it. Escape and Cancel are
+  // the dismissals. This test pins the decision so a "restore backdrop click"
+  // change cannot land silently.
+  const dialog = await openNameDialog(page)
+  await page.locator('#root').click({ position: { x: 5, y: 5 }, force: true })
+  await expect(dialog).toBeVisible()
+  await page.keyboard.press('Escape')
+  await expect(dialog).not.toBeVisible()
+})