Adopt platform-bible-react components in place of raw HTML (Tier 3)

AGENTS.md

@@ -73,6 +73,8 @@ Prefer Platform.Bible's `platform-bible-react` components (`Button`, `Input`, `T
 
 Size icons inside a `Button` with `size-*` (e.g. `tw:size-3`), never `h-*`/`w-*` — `buttonVariants` forces any child SVG lacking a `size-` class to `size-4`, silently overriding `h-*`/`w-*`.
 
+Modals go through [ModalShell](src/components/modals/ModalShell.tsx) rather than building their own overlay — it supplies the platform `Dialog`, which brings a focus trap, scroll lock, focus restore, and Escape-to-dismiss. Whether a modal passes `onClose` is the single switch governing every dismissal route: supplying it enables both Escape and outside-click, and a modal that is mid-submission passes none, so neither route can abandon in-flight work. Suppress it only for work that is genuinely being abandoned — a read-only load has nothing to abandon and leaves the modal dismissable. Modals tag their title with a `data-testid` because end-to-end tests locate them that way; never give the title an `id` instead, since that displaces the one the platform `Dialog` generates and the dialog then logs that its title is missing. The platform `Dialog` and `Popover` both render `role="dialog"` — select a modal by `[data-slot="dialog-content"]` when the two must be told apart.
+
 ### Styling
 
 All UI uses Tailwind CSS (via `src/tailwind.css`). Every Tailwind class is prefixed `tw:` to avoid collisions with Platform.Bible's own styles (configured in `tailwind.config.ts`). For modifier variants the prefix comes first: `tw:hover:px-3`, not `hover:tw-px-3`.

__mocks__/platform-bible-react.tsx

@@ -11,6 +11,7 @@ import {
   isValidElement,
   useContext,
   useEffect,
+  useId,
   useLayoutEffect,
   useMemo,
   useRef,
@@ -581,15 +582,162 @@ export function RadioGroupItem({
   );
 }
 
+/**
+ * Context carrying the {@link Dialog}'s open-state change handler and generated title id down to
+ * {@link DialogContent} and {@link DialogTitle}, mirroring how the real Radix-based component
+ * reaches its parts from the root.
+ */
+const DialogContext = createContext<{ onOpenChange?: (open: boolean) => void; titleId?: string }>(
+  {},
+);
+
+/**
+ * Stub dialog root that renders its children unconditionally. The extension mounts a modal only
+ * while it should be showing and holds `open` at `true`, so visibility needs no simulation here.
+ *
+ * Generates the title id the way the real component does, so the automatic `aria-labelledby`
+ * wiring between the surface and its heading is exercised rather than assumed.
+ */
+export function Dialog({
+  children,
+  onOpenChange,
+}: Readonly<{
+  children?: ReactNode;
+  onOpenChange?: (open: boolean) => void;
+}>): ReactElement {
+  const titleId = useId();
+  const contextValue = useMemo(() => ({ onOpenChange, titleId }), [onOpenChange, titleId]);
+  return <DialogContext.Provider value={contextValue}>{children}</DialogContext.Provider>;
+}
+
+/**
+ * Mounted {@link DialogContent} surfaces in mount order, so an Escape can be routed to the topmost
+ * one alone. The real component stacks dismissal layers this way; without the stack, a dialog
+ * overlaying another would dismiss both at once.
+ */
+const mountedDialogs: { current?: (open: boolean) => void }[] = [];
+
+/**
+ * Stub dialog surface rendered as a `<div role="dialog" data-slot="dialog-content">` — the slot
+ * being what tells a modal apart from a popover, since both carry the dialog role — that reports
+ * Escape back through the root's change handler, which is the one dismissal path the extension's
+ * own code implements. The
+ * real component additionally traps focus, locks scrolling, and restores focus on close; those are
+ * behaviors of the platform package rather than of this extension, so they are left to end-to-end
+ * coverage rather than faked here.
+ *
+ * `onInteractOutside` is accepted and ignored — there is no outside region to click in this stub.
+ * A close button is never rendered because the extension always suppresses it.
+ */
+export function DialogContent({
+  children,
+  className,
+}: Readonly<{
+  'aria-describedby'?: undefined;
+  children?: ReactNode;
+  className?: string;
+  onInteractOutside?: (event: { preventDefault: () => void }) => void;
+  showCloseButton?: boolean;
+}>): ReactElement {
+  const { onOpenChange, titleId } = useContext(DialogContext);
+  const onOpenChangeRef = useRef(onOpenChange);
+  useEffect(() => {
+    onOpenChangeRef.current = onOpenChange;
+  });
+
+  // The real component dismisses on Escape from anywhere in the document rather than only when the
+  // surface itself holds focus, so listen the same way here — but only the topmost dialog reacts.
+  useEffect(() => {
+    const entry = onOpenChangeRef;
+    mountedDialogs.push(entry);
+    const handleKeyDown = (event: KeyboardEvent) => {
+      if (event.key === 'Escape' && mountedDialogs.at(-1) === entry) entry.current?.(false);
+    };
+    document.addEventListener('keydown', handleKeyDown);
+    return () => {
+      mountedDialogs.splice(mountedDialogs.indexOf(entry), 1);
+      document.removeEventListener('keydown', handleKeyDown);
+    };
+  }, []);
+
+  return (
+    <div
+      aria-labelledby={titleId}
+      aria-modal="true"
+      className={className}
+      data-slot="dialog-content"
+      role="dialog"
+    >
+      {children}
+    </div>
+  );
+}
+
+/**
+ * Stub dialog title rendered as the `<h2>` the real component produces, keeping the heading role and
+ * taking its `id` from the root so the dialog's `aria-labelledby` resolves to it. Forwards
+ * `data-testid` as the real component does, since that is the handle a modal is identified by.
+ */
+export function DialogTitle({
+  children,
+  className,
+  'data-testid': testId,
+}: Readonly<{
+  children?: ReactNode;
+  className?: string;
+  'data-testid'?: string;
+}>): ReactElement {
+  const { titleId } = useContext(DialogContext);
+  return (
+    <h2 className={className} data-testid={testId} id={titleId}>
+      {children}
+    </h2>
+  );
+}
+
+/**
+ * Context carrying the {@link Popover}'s open state and change handler down to
+ * {@link PopoverTrigger}, mirroring how the real Radix-based component coordinates the two.
+ */
+const PopoverContext = createContext<{
+  onOpenChange?: (open: boolean) => void;
+  open?: boolean;
+}>({});
+
 /**
  * Stub popover root that renders its children unconditionally. The extension conditionally mounts
  * the content component while open (so its draft state re-initializes per open), so visibility
  * needs no simulation here.
  */
 export function Popover({
   children,
-}: Readonly<{ children?: ReactNode; open?: boolean; modal?: boolean }>): ReactElement {
-  return <>{children}</>;
+  onOpenChange,
+  open,
+}: Readonly<{
+  children?: ReactNode;
+  modal?: boolean;
+  onOpenChange?: (open: boolean) => void;
+  open?: boolean;
+}>): ReactElement {
+  const contextValue = useMemo(() => ({ onOpenChange, open }), [onOpenChange, open]);
+  return <PopoverContext.Provider value={contextValue}>{children}</PopoverContext.Provider>;
+}
+
+/**
+ * Stub popover trigger. With `asChild` (the only mode the extension uses) the real component merges
+ * its trigger behavior onto the single child element rather than rendering a wrapper, so this stub
+ * clones the child with the open-state attributes and the toggle handler Radix would supply.
+ */
+export function PopoverTrigger({
+  children,
+}: Readonly<{ children?: ReactNode; asChild?: boolean }>): ReactNode {
+  const { onOpenChange, open = false } = useContext(PopoverContext);
+  if (!isValidElement(children)) return <>{children}</>;
+  return cloneElement(children, {
+    'aria-expanded': open,
+    'aria-haspopup': 'dialog',
+    onClick: () => onOpenChange?.(!open),
+  });
 }
 
 /**
@@ -603,9 +751,11 @@ export function PopoverAnchor({
 }
 
 /**
- * Stub popover content rendered as a plain `<div data-testid="popover-content">`. The real
- * component implements positioning, portaling, and dismissal internally; this stub exposes the
- * dismissal callbacks so tests can simulate them:
+ * Stub popover content rendered as a `<div role="dialog" data-testid="popover-content">` — the role
+ * matching the real component, which is what makes its `aria-label` meaningful, and which is why a
+ * test that must reach a modal instead selects on `[data-slot="dialog-content"]`. The real component
+ * implements positioning, portaling, and dismissal internally; this stub exposes the dismissal
+ * callbacks so tests can simulate them:
  *
  * - The panel's children render only from the second commit, mirroring Radix's portal (which renders
  *   nothing until its own layout effect flips its `mounted` state). Consumers must therefore not
@@ -621,17 +771,21 @@ export function PopoverAnchor({
  *   simulating Radix's focus-restoration event fired as the popover closes.
  */
 export function PopoverContent({
+  'aria-label': ariaLabel,
   children,
   className,
+  'data-testid': testId = 'popover-content',
   onEscapeKeyDown,
   onPointerDownOutside,
   onOpenAutoFocus,
   onCloseAutoFocus,
   onClick,
   onMouseDown,
 }: Readonly<{
+  'aria-label'?: string;
   children?: ReactNode;
   className?: string;
+  'data-testid'?: string;
   align?: 'start' | 'center' | 'end';
   sideOffset?: number;
   onEscapeKeyDown?: (event: KeyboardEvent) => void;
@@ -660,13 +814,15 @@ export function PopoverContent({
     first?.focus();
     if (first instanceof HTMLInputElement) first.select();
   }, [portalMounted]);
-  if (!portalMounted) return <div data-testid="popover-content" />;
+  if (!portalMounted) return <div data-testid={testId} />;
   return (
     // eslint-disable-next-line jsx-a11y/no-static-element-interactions
     <div
       ref={contentRef}
+      aria-label={ariaLabel}
       className={className}
-      data-testid="popover-content"
+      data-testid={testId}
+      role="dialog"
       onClick={onClick}
       onKeyDown={(e) => {
         if (e.key === 'Escape') onEscapeKeyDown?.(e.nativeEvent);

e2e-tests/README.md

@@ -7,6 +7,8 @@ End-to-end tests for the interlinearizer extension using Playwright + Electron.
 
 Run everything with `npm run test:e2e` (smoke tier then CDP tier). Each tier can be run alone with `npm run test:e2e:smoke` and `npm run test:e2e:cdp`.
 
+Global setup rebuilds the extension before launching, so there is no separate build step to remember. This matters more than the few seconds it costs: a `dist/` left over from another branch runs an extension the tests were not written against, and the selector failures that follow look exactly like real regressions.
+
 Both tiers are self-launching: the CDP tier's `globalSetup` launches its own Platform.Bible instance (with `--remote-debugging-port=9223`) in an isolated user-data dir and tears it down afterward, so `npm run test:e2e:cdp` needs no manual `npm run start:cdp` first. To iterate against a warm instance instead, run `npm run start:cdp` in one terminal, then run the CDP config directly with `npx playwright test --config e2e-tests/playwright-cdp.config.ts`: the setup detects the in-use CDP port, reuses that instance, and leaves it running.
 
 In CI (`.github/workflows/test.yml`, `e2e` job) the full suite runs on both Linux and Windows.
@@ -40,5 +42,5 @@ Feature tests run with `npm run test:e2e:cdp`. That command launches a fresh, is
 - **Reset at the start, tidy at the end.** Correctness rests on the start-of-test sequence, which self-heals whatever a failed run left behind (leftover modals, stray project pickers, another test's dirty draft); see the JSDocs on `ensureInterlinearizerOpenOnWeb()` and `ensureE2eProjectActive()` for the mechanism. This self-healing is also what lets the CDP config safely `retries` in CI. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers — a courtesy to the next run, not something correctness depends on.
 - **Use unique per-run values** (e.g. `` `e2e-gloss-${Date.now()}` ``) for anything written into the draft, so a stale leftover can never satisfy an assertion.
 - **Drive only the visible UI.** No JSON-RPC/WebSocket calls to set up or assert state (the rpc.discover readiness polls in the shared helpers are the one sanctioned exception).
-- **Prefer existing accessible selectors** (roles, aria-labels like `Gloss for {word}`, ModalShell title ids) over adding new `data-testid`s to production code.
+- **Prefer existing selectors** (roles, aria-labels like `Gloss for {word}`, the modal title `data-testid`s the shell already sets) over adding new `data-testid`s to production code. Modal titles carry no author-supplied `id` — that one belongs to the platform dialog, which generates it and points its own `aria-labelledby` at it.
 - **Mutating tests must not overwrite or delete projects, and must not create any beyond what `ensureE2eProjectActive()` creates** (the e2e project itself, plus rescue projects). The current modal coverage is a read-only cancel tour; a create/delete lifecycle test needs its own self-healing cleanup (e.g. deleting leftover `e2e-*` projects at start) before it's safe on a shared instance.

e2e-tests/fixtures/helpers.ts

@@ -24,6 +24,27 @@ const DEFAULT_WEBSOCKET_PORT = 8876;
 const RPC_DISCOVER_POLL_INTERVAL_MS = 250;
 export const PROCESS_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000;
 
+/**
+ * Selects a project modal's dialog surface. The platform dialog marks its surface with this slot,
+ * which distinguishes a modal from the view-options popover — both carry `role="dialog"`.
+ */
+const MODAL_DIALOG_SELECTOR = '[data-slot="dialog-content"]';
+
+/**
+ * The frontmost project modal's dialog surface, for scoping a lookup to the modal's own controls.
+ *
+ * Resolves to a single element on purpose. The discard-draft guard overlays the modal that
+ * triggered it rather than replacing it, and the platform dialog stays mounted through its closing
+ * animation, so two surfaces can legitimately coexist; an unqualified match would fail strict mode
+ * at those moments. Last-in-DOM is the frontmost surface: the platform dialog portals in mount
+ * order, so the most recently opened modal sorts last. That is the one a caller can actually reach
+ * — a stacked guard makes the modal beneath it inert, and a surface still playing its closing
+ * animation is on its way out.
+ */
+export function modalDialog(frame: FrameLocator): Locator {
+  return frame.locator(MODAL_DIALOG_SELECTOR).last();
+}
+
 /**
  * Fail-fast readiness budget (ms) for a CDP feature test's per-test wait (the `{ cdp: true }`
  * profile). The shared instance is already settled by global setup, so a long per-test wait means
@@ -1290,23 +1311,22 @@ export async function waitForAppAndInterlinearizerReady(
 
 /**
  * Dismiss any modal left mounted inside the Interlinearizer iframe by a prior failed test, so its
- * full-viewport `tw:modal-overlay` (fixed inset-0 z-50, see src/components/modals/ModalShell.tsx)
- * can't intercept every click in the run that follows.
+ * full-viewport backdrop can't intercept every click in the run that follows.
  *
  * This is the shared-instance recovery step: the CDP fixture never resets its DOM between tests, so
  * a test that dies with a modal open leaves that overlay blocking the next test. Running this at
  * the start of the open-Interlinearizer precondition self-heals it, which is also what lets a CDP
  * retry land on a clean instance.
  *
- * Each project modal's only reliable dismiss affordance is its Cancel/secondary button — the
- * dialogs are a plain `<dialog open>` (not `showModal()`), so native Escape doesn't fire their
- * onCancel. Modals can chain, so cancel in a bounded loop until no overlay remains.
+ * Drive Cancel rather than Escape: a modal may spend an Escape collapsing an inline confirmation of
+ * its own, and one mid-submission answers neither route. Modals can chain, so cancel in a bounded
+ * loop until no overlay remains.
  */
 export async function dismissLeftoverModals(page: Page): Promise<void> {
   const frame = getInterlinearizerFrame(page);
-  // The `<dialog>` ModalShell renders is the only one in the iframe, so this both detects an open
-  // modal and scopes the Cancel lookup — no separate overlay selector needed.
-  const dialog = frame.locator('dialog').first();
+  // The modal surface both detects an open modal and scopes the Cancel lookup — no separate
+  // overlay selector needed.
+  const dialog = modalDialog(frame);
 
   // Bounded; a couple of chained confirmations is the realistic worst case.
   for (let attempt = 0; attempt < 3; attempt += 1) {
@@ -1528,8 +1548,8 @@ async function openSelectProjectModal(page: Page): Promise<FrameLocator> {
     .getByRole('menuitem', { name: /Select Interlinear Project/i })
     .first()
     .click();
-  await expect(frame.locator('#select-project-modal-title')).toBeVisible({ timeout: 10_000 });
-  await expect(frame.locator('dialog').getByRole('button', { name: 'Cancel' })).toBeEnabled({
+  await expect(frame.getByTestId('select-project-modal-title')).toBeVisible({ timeout: 10_000 });
+  await expect(modalDialog(frame).getByRole('button', { name: 'Cancel' })).toBeEnabled({
     timeout: 10_000,
   });
   return frame;
@@ -1551,7 +1571,7 @@ async function rescueDraftToNewProject(page: Page): Promise<void> {
     .first()
     .click();
 
-  const saveAsTitle = frame.locator('#save-as-modal-title');
+  const saveAsTitle = frame.getByTestId('save-as-modal-title');
   await expect(saveAsTitle).toBeVisible({ timeout: 10_000 });
   await frame.locator('#save-as-name').fill(`${RESCUE_PROJECT_PREFIX}-${Date.now()}`);
   await frame.getByTestId('save-as-new').click();
@@ -1590,7 +1610,7 @@ export async function ensureE2eProjectActive(
 
   let dirty = await isDraftDirty(page);
   let frame = await openSelectProjectModal(page);
-  let dialog = frame.locator('dialog');
+  let dialog = modalDialog(frame);
 
   // Match the E2E entry by its project-name element with EXACT text, not the button's accessible
   // name: the modal renders name, an optional "Active" badge, and the languages as adjacent <span>s
@@ -1603,14 +1623,16 @@ export async function ensureE2eProjectActive(
 
   if (dirty && !activeIsE2e && rescueDirtyDraft) {
     await dialog.getByRole('button', { name: 'Cancel' }).click();
-    await expect(frame.locator('#select-project-modal-title')).not.toBeVisible({ timeout: 5_000 });
+    await expect(frame.getByTestId('select-project-modal-title')).not.toBeVisible({
+      timeout: 5_000,
+    });
     await rescueDraftToNewProject(page);
     dirty = false;
     frame = await openSelectProjectModal(page);
-    dialog = frame.locator('dialog');
+    dialog = modalDialog(frame);
   }
 
-  const selectTitle = frame.locator('#select-project-modal-title');
+  const selectTitle = frame.getByTestId('select-project-modal-title');
   // Rebuilt against the (possibly re-opened) dialog so it targets the current modal instance.
   const e2eEntry = dialog
     .locator('button', { has: frame.getByText(E2E_PROJECT_NAME, { exact: true }) })
@@ -1625,10 +1647,10 @@ export async function ensureE2eProjectActive(
     }
   } else {
     await dialog.getByRole('button', { name: 'Create New' }).click();
-    const createTitle = frame.locator('#create-project-modal-title');
+    const createTitle = frame.getByTestId('create-project-modal-title');
     await expect(createTitle).toBeVisible({ timeout: 5_000 });
     await frame.locator('#project-name').fill(E2E_PROJECT_NAME);
-    await frame.locator('dialog').getByRole('button', { name: 'Create' }).click();
+    await modalDialog(frame).getByRole('button', { name: 'Create' }).click();
     // Creating a draft over a dirty one defers behind the discard confirmation instead of closing
     // the create modal (handleCreateDraft in ProjectModals.tsx), so dismiss it when dirty.
     if (dirty) {
@@ -1653,7 +1675,7 @@ export async function wipeDraft(page: Page): Promise<void> {
   const frame = await openInterlinearizerProjectMenu(page);
   await frame.getByRole('menuitem', { name: /Wipe/i }).first().click();
 
-  const wipeDialogTitle = frame.locator('#wipe-modal-title');
+  const wipeDialogTitle = frame.getByTestId('wipe-modal-title');
   await expect(wipeDialogTitle).toBeVisible({ timeout: 5_000 });
   const scopeAll = frame.getByTestId('wipe-scope-all');
   // `force`: on a slow/software-rendered CI display the just-opened modal overlay hasn't won the