diff --git a/AGENTS.md b/AGENTS.md index fe2e19fc..9348b775 100644 --- a/AGENTS.md +++ b/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`. diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index fa8da279..4c44d889 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -11,6 +11,7 @@ import { isValidElement, useContext, useEffect, + useId, useLayoutEffect, useMemo, useRef, @@ -581,6 +582,128 @@ 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 {children}; +} + +/** + * 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 `
` — 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 ( +
+ {children} +
+ ); +} + +/** + * Stub dialog title rendered as the `

` 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 ( +

+ {children} +

+ ); +} + +/** + * 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 @@ -588,8 +711,33 @@ export function RadioGroupItem({ */ 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 {children}; +} + +/** + * 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 `
`. 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 `
` — 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,8 +771,10 @@ 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, @@ -630,8 +782,10 @@ export function PopoverContent({ 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
; + if (!portalMounted) return
; return ( // eslint-disable-next-line jsx-a11y/no-static-element-interactions
{ if (e.key === 'Escape') onEscapeKeyDown?.(e.nativeEvent); diff --git a/e2e-tests/README.md b/e2e-tests/README.md index 24bee04c..e9bcfe4d 100644 --- a/e2e-tests/README.md +++ b/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. diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index df2d9b69..b6c32d83 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/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 `` (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 { const frame = getInterlinearizerFrame(page); - // The `` 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 { .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 { .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 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 { 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 diff --git a/e2e-tests/global-setup.ts b/e2e-tests/global-setup.ts index bcf25e49..b0a2d6f6 100644 --- a/e2e-tests/global-setup.ts +++ b/e2e-tests/global-setup.ts @@ -139,9 +139,9 @@ export function waitForPort(port: number, timeout: number): Promise { /** * Bootstrap everything an Electron launch needs, short of launching Electron itself: verify no - * conflicting instance is running, clear stale singleton locks, confirm the extension is built, - * ensure the paranext-core dev main bundle exists, and start the renderer dev server on port 1212 - * (recording its PID for teardown). + * conflicting instance is running, clear stale singleton locks, build the extension, ensure the + * paranext-core dev main bundle exists, and start the renderer dev server on port 1212 (recording + * its PID for teardown). * * Self-cleaning on failure: if a dev server started here never becomes ready, it is killed before * the error propagates (see {@link killSpawnedDevServer}), so it cannot leak. Callers therefore need @@ -149,7 +149,7 @@ export function waitForPort(port: number, timeout: number): Promise { * * @returns Resolves when the renderer dev server is ready. * @throws {Error} If port 8876 is already in use (a running Platform.Bible would conflict). - * @throws {Error} If the extension dist is missing. + * @throws {Error} If the extension fails to build. * @throws {Error} If a dev server started here does not open port 1212 within 60s, or (outside CI) * fails the HTTP compilation probe within 120s. */ @@ -187,15 +187,12 @@ export async function bootstrapRendererDevServer(): Promise { }, ); - // Fail fast if the extension dist is missing — tests cannot run without a built extension - const extensionMain = path.join(extensionRoot, 'dist/src/main.js'); - if (!fs.existsSync(extensionMain)) { - throw new Error( - `Extension dist not found at ${extensionMain}. ` + - 'Run "npm run build" in interlinearizer-extension before running E2E tests.', - ); - } - console.log('Extension dist found.'); + // Rebuild rather than merely checking dist exists. A dist left over from another branch loads an + // extension the tests were not written against, and the selector mismatches that follow read as + // real regressions rather than a build problem. A no-op rebuild costs seconds against a + // multi-minute run, so always paying it is cheaper than ever debugging the stale case. + console.log('Building the extension...'); + execSync('npm run build', { cwd: extensionRoot, stdio: 'inherit' }); // Ensure the paranext-core dev main bundle exists const devMainPath = path.join(coreDir, '.erb/dll/main.bundle.dev.js'); @@ -325,7 +322,7 @@ function removeDevServerPidMarker(): void { * @param _config - Playwright config object — unused; required by Playwright's global-setup * interface. * @throws {Error} If port 8876 is already in use. - * @throws {Error} If the extension dist is missing. + * @throws {Error} If the extension fails to build. */ // eslint-disable-next-line @typescript-eslint/no-unused-vars export default async function globalSetup(_config: FullConfig): Promise { diff --git a/e2e-tests/tests/features/project-modals.spec.ts b/e2e-tests/tests/features/project-modals.spec.ts index d66c0910..da64273d 100644 --- a/e2e-tests/tests/features/project-modals.spec.ts +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -2,31 +2,32 @@ import { expect, test } from '../../fixtures/cdp.fixture'; import { ensureInterlinearizerOpenOnWeb, getInterlinearizerFrame, + modalDialog, openInterlinearizerProjectMenu, waitForAppAndInterlinearizerReady, } from '../../fixtures/helpers'; /** * The project-related modals reachable from the Interlinearizer's ≡ (Project) menu, each with the - * menu item that opens it and the title element that identifies it (from ModalShell's `titleId`). - * The tour is read-only: each modal is opened, verified, and canceled — no project is created, - * saved, or deleted, so the shared CDP instance is left untouched. + * menu item that opens it and the test id on the title element that identifies it. The tour is + * read-only: each modal is opened, verified, and canceled — no project is created, saved, or + * deleted, so the shared CDP instance is left untouched. */ const MODAL_TOURS = [ { name: 'Select Interlinear Project', menuItem: /Select Interlinear Project/i, - titleSelector: '#select-project-modal-title', + titleTestId: 'select-project-modal-title', }, { name: 'New Interlinear Project', menuItem: /New Interlinear Project/i, - titleSelector: '#create-project-modal-title', + titleTestId: 'create-project-modal-title', }, { name: 'Save As', menuItem: /^Save As/i, - titleSelector: '#save-as-modal-title', + titleTestId: 'save-as-modal-title', }, ]; @@ -42,10 +43,10 @@ test.describe('Project modals cancel tour', () => { const frame = await openInterlinearizerProjectMenu(mainPage); await frame.getByRole('menuitem', { name: modal.menuItem }).first().click(); - const modalTitle = frame.locator(modal.titleSelector); + const modalTitle = frame.getByTestId(modal.titleTestId); await expect(modalTitle).toBeVisible({ timeout: 5_000 }); - await frame.locator('dialog').getByRole('button', { name: 'Cancel' }).click(); + await modalDialog(frame).getByRole('button', { name: 'Cancel' }).click(); await expect(modalTitle).not.toBeVisible({ timeout: 5_000 }); // The underlying view must still be interactive after the modal unmounts — a stuck diff --git a/e2e-tests/tests/smoke/open-interlinearizer.spec.ts b/e2e-tests/tests/smoke/open-interlinearizer.spec.ts index 39b8fac8..d8a7627e 100644 --- a/e2e-tests/tests/smoke/open-interlinearizer.spec.ts +++ b/e2e-tests/tests/smoke/open-interlinearizer.spec.ts @@ -28,5 +28,12 @@ test.describe('Open Interlinearizer', () => { await expect(interlinearizerFrame.getByTestId('view-options-panel')).toBeVisible({ timeout: 5_000, }); + + // Closing on a second gear click needs a trigger press and an outside interaction to resolve in + // the right order, which only the real popover does — hence checking it here, not in jsdom. + await viewOptionsButton.click(); + await expect(interlinearizerFrame.getByTestId('view-options-panel')).not.toBeVisible({ + timeout: 5_000, + }); }); }); diff --git a/package-lock.json b/package-lock.json index e6341efc..485045bb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1467,9 +1467,9 @@ "license": "MIT" }, "node_modules/@eslint/eslintrc/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -1588,9 +1588,9 @@ "license": "MIT" }, "node_modules/@humanwhocodes/config-array/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -4977,9 +4977,9 @@ "license": "MIT" }, "node_modules/archiver-utils/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -6972,9 +6972,9 @@ "license": "MIT" }, "node_modules/electron-to-chromium": { - "version": "1.5.397", - "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.397.tgz", - "integrity": "sha512-khGTy9U9x02KEtsKM8vx5A62BsRmcOsIgDpWr1ImE32Ax8GxHGPHZf+Eu9H8zOOyHJnB0jTbseyTHbq2XCT8yw==", + "version": "1.5.398", + "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.398.tgz", + "integrity": "sha512-AsvhAxopJGh6museTDMIjn6JpDYOfgu4RLlygomt87MUwBUqTfd/1EiPtx10/LZE8xpTvkP2E9Gafq7lkLtodQ==", "dev": true, "license": "ISC" }, @@ -7599,9 +7599,9 @@ "license": "MIT" }, "node_modules/eslint-plugin-import/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -7765,9 +7765,9 @@ "license": "MIT" }, "node_modules/eslint-plugin-jsx-a11y/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -7916,9 +7916,9 @@ "license": "MIT" }, "node_modules/eslint-plugin-react/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -8000,9 +8000,9 @@ "license": "MIT" }, "node_modules/eslint/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -13870,9 +13870,9 @@ } }, "node_modules/postcss": { - "version": "8.5.24", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.24.tgz", - "integrity": "sha512-8RyVklq0owXUTa4xlpzu4l9AaVKIdQvAcOHZWaMh98HgySsUtxRVf/chRe3dsSLqb6i40BzGRzEUddRaI+9TSw==", + "version": "8.5.25", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.25.tgz", + "integrity": "sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==", "dev": true, "funding": [ { @@ -14831,9 +14831,9 @@ "license": "MIT" }, "node_modules/rimraf/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -16646,9 +16646,9 @@ "license": "MIT" }, "node_modules/test-exclude/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { @@ -17469,9 +17469,9 @@ } }, "node_modules/webpack": { - "version": "5.109.1", - "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.109.1.tgz", - "integrity": "sha512-Q4XQscWSLNQSaMFsIWUZ+IVpvwLqD+MvjIuSQC1hG8m1ZMK78VAsNMhTjD3icJelypp6aM5Hq8BZNEX3sc1PJw==", + "version": "5.109.2", + "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.109.2.tgz", + "integrity": "sha512-U9/cvLzxObKNEZ9+TtdqrHM5/9z3lgl2c+c4BzbqGxFQvQvBAq87yql5A8pQ+rrMbS496MZJeF5enVBndIy2hw==", "dev": true, "license": "MIT", "dependencies": { @@ -17483,7 +17483,7 @@ "acorn": "^8.16.0", "browserslist": "^4.28.1", "chrome-trace-event": "^1.0.2", - "enhanced-resolve": "^5.24.2", + "enhanced-resolve": "^5.24.4", "es-module-lexer": "^2.1.0", "eslint-scope": "5.1.1", "events": "^3.2.0", @@ -18276,9 +18276,9 @@ "license": "MIT" }, "node_modules/zip-stream/node_modules/brace-expansion": { - "version": "1.1.16", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", - "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", + "version": "1.1.17", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.17.tgz", + "integrity": "sha512-w+aeW/mkgM4PyRMOJCgi3fOrTm5Q8QY1OSfn2TO2iuDj3ezIHqejmuxbjfPrqUkgqRew1iqkyAn0tr0ZwHD9+w==", "dev": true, "license": "MIT", "dependencies": { diff --git a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx index a8a78fdc..920a0545 100644 --- a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx +++ b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx @@ -2,7 +2,7 @@ /// import { useLocalizedStrings } from '@papi/frontend/react'; -import { act, render, screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import ViewOptionsDropdown from '../../../components/controls/ViewOptionsDropdown'; @@ -60,17 +60,6 @@ describe('ViewOptionsDropdown', () => { expect(screen.queryByTestId('view-options-panel')).not.toBeInTheDocument(); }); - it('closes the panel when the backdrop is clicked', async () => { - render(); - - await userEvent.click(screen.getByTestId('view-options-button')); - // The backdrop is the fixed overlay beneath the panel. - const backdrop = document.querySelector('[aria-hidden="true"]'); - if (backdrop instanceof HTMLElement) await userEvent.click(backdrop); - - expect(screen.queryByTestId('view-options-panel')).not.toBeInTheDocument(); - }); - it('renders labels from useLocalizedStrings for every toggle', async () => { render(); await userEvent.click(screen.getByTestId('view-options-button')); @@ -89,43 +78,6 @@ describe('ViewOptionsDropdown', () => { expect(screen.getByText('%interlinearizer_viewOption_showSuggestions%')).toBeInTheDocument(); }); - describe('panel positioning', () => { - it('repositions the panel when the window resizes while open', async () => { - let bottom = 30; - let right = 200; - jest.spyOn(HTMLButtonElement.prototype, 'getBoundingClientRect').mockImplementation(() => { - const rect = { top: 10, bottom, left: 100, right, width: 100, height: 20, x: 100, y: 10 }; - return { ...rect, toJSON: () => rect }; - }); - Object.defineProperty(window, 'innerWidth', { value: 1000, configurable: true }); - - render(); - await userEvent.click(screen.getByTestId('view-options-button')); - - const panel = screen.getByTestId('view-options-panel'); - expect(panel).toHaveStyle({ top: '34px', right: '800px' }); - - // Simulate a layout shift, then fire resize: the panel should re-anchor to the button. - bottom = 50; - right = 300; - act(() => { - window.dispatchEvent(new Event('resize')); - }); - - expect(panel).toHaveStyle({ top: '54px', right: '700px' }); - }); - - it('removes the resize listener when the panel closes', async () => { - const removeSpy = jest.spyOn(window, 'removeEventListener'); - - render(); - await userEvent.click(screen.getByTestId('view-options-button')); - await userEvent.click(screen.getByTestId('view-options-button')); - - expect(removeSpy).toHaveBeenCalledWith('resize', expect.any(Function)); - }); - }); - describe('continuous scroll toggle', () => { it('reflects the checked value', async () => { render(); diff --git a/src/__tests__/components/modals/ModalShell.test.tsx b/src/__tests__/components/modals/ModalShell.test.tsx new file mode 100644 index 00000000..4ce67b11 --- /dev/null +++ b/src/__tests__/components/modals/ModalShell.test.tsx @@ -0,0 +1,91 @@ +/// +/// + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { ModalShell } from '../../../components/modals/ModalShell'; + +// Popovers carry the dialog role too, so the modal surface is reached by the slot that is its own. +const DIALOG_SELECTOR = '[data-slot="dialog-content"]'; + +const defaultProps = { + titleTestId: 'test-modal-title', + title: 'Test modal', + width: 'tw:w-96', +}; + +describe('ModalShell', () => { + it('names the dialog with the title heading', () => { + render( + +

Body

+ , + ); + + // The platform dialog generates the heading's id and aims its own `aria-labelledby` at it, so + // the shell must leave both alone; overriding the id makes the dialog report a missing title. + const heading = screen.getByRole('heading', { name: 'Test modal' }); + expect(heading.id).toBeTruthy(); + expect(document.querySelector(DIALOG_SELECTOR)).toHaveAttribute('aria-labelledby', heading.id); + }); + + it('tags the title with the test id end-to-end tests locate the modal by', () => { + render(); + + expect(screen.getByTestId('test-modal-title')).toHaveTextContent('Test modal'); + }); + + it('renders the body below the title', () => { + render( + +

Body

+ , + ); + + expect(screen.getByText('Body')).toBeInTheDocument(); + }); + + it('renders without a body while the modal is still resolving its content', () => { + render(); + + expect(screen.getByRole('heading', { name: 'Test modal' })).toBeInTheDocument(); + }); + + it('calls onClose when the user presses Escape', async () => { + const onClose = jest.fn(); + render(); + + await userEvent.keyboard('{Escape}'); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('ignores Escape when no onClose is supplied, so a busy modal cannot be abandoned', async () => { + render(); + + await userEvent.keyboard('{Escape}'); + + expect(document.querySelector(DIALOG_SELECTOR)).toBeInTheDocument(); + }); + + it('dismisses only the topmost modal on Escape when one overlays another', async () => { + const onCloseUnder = jest.fn(); + const onCloseOver = jest.fn(); + render( + <> + + + , + ); + + await userEvent.keyboard('{Escape}'); + + expect(onCloseOver).toHaveBeenCalledTimes(1); + expect(onCloseUnder).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/components/modals/ProjectMetadataModal.test.tsx b/src/__tests__/components/modals/ProjectMetadataModal.test.tsx index 3ec9a710..4657f415 100644 --- a/src/__tests__/components/modals/ProjectMetadataModal.test.tsx +++ b/src/__tests__/components/modals/ProjectMetadataModal.test.tsx @@ -358,6 +358,30 @@ describe('ProjectMetadataModal', () => { expect(screen.queryByText('Delete project?')).not.toBeInTheDocument(); }); + it('hides delete confirmation on Escape, keeping the modal and its edits open', async () => { + const onClose = jest.fn(); + render(); + await userEvent.type(screen.getByLabelText(/^name$/i), 'Renamed'); + await userEvent.click(screen.getByRole('button', { name: /^delete$/i })); + + await userEvent.keyboard('{Escape}'); + + expect(screen.queryByText('Delete project?')).not.toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByLabelText(/^name$/i)).toHaveValue('Renamed'); + }); + + it('closes on a second Escape once the delete confirmation has collapsed', async () => { + const onClose = jest.fn(); + render(); + await userEvent.click(screen.getByRole('button', { name: /^delete$/i })); + + await userEvent.keyboard('{Escape}'); + await userEvent.keyboard('{Escape}'); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + it('calls deleteProject command when delete is confirmed', async () => { render(); diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index ff3abd95..ed980d18 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -69,17 +69,19 @@ const MOCK_DRAFT_WITH_SEGMENTATION: DraftProject = { jest.mock('../../../components/modals/SelectInterlinearProjectModal', () => ({ __esModule: true, SelectInterlinearProjectModal: ({ + isOpening, onSelect, onCreateNew, onClose, onViewInfo, }: { + isOpening?: boolean; onSelect: (p: InterlinearProjectSummary) => void; onCreateNew: () => void; onClose: () => void; onViewInfo: (p: InterlinearProjectSummary) => void; }) => ( -
+
@@ -397,6 +399,28 @@ describe('ProjectModals', () => { expect(setModal).toHaveBeenCalledWith('none'); }); + it('marks the select modal as opening while the chosen project loads, and clears it after', async () => { + let resolveGet: (v: string) => void = () => {}; + jest.mocked(papi.commands.sendCommand).mockImplementationOnce( + () => + new Promise((resolve) => { + resolveGet = resolve; + }), + ); + render(); + expect(screen.getByTestId('select-modal')).toHaveAttribute('data-is-opening', 'false'); + + await userEvent.click(screen.getByTestId('select-select')); + await waitFor(() => + expect(screen.getByTestId('select-modal')).toHaveAttribute('data-is-opening', 'true'), + ); + + resolveGet(JSON.stringify(MOCK_FULL_PROJECT)); + await waitFor(() => + expect(screen.getByTestId('select-modal')).toHaveAttribute('data-is-opening', 'false'), + ); + }); + it('carries the target project id into the draft for a bilateral project', async () => { jest .mocked(papi.commands.sendCommand) diff --git a/src/__tests__/components/modals/SaveAsProjectModal.test.tsx b/src/__tests__/components/modals/SaveAsProjectModal.test.tsx index 673f0fb6..ec373235 100644 --- a/src/__tests__/components/modals/SaveAsProjectModal.test.tsx +++ b/src/__tests__/components/modals/SaveAsProjectModal.test.tsx @@ -5,6 +5,7 @@ import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import papi, { logger } from '@papi/frontend'; import { useLocalizedStrings } from '@papi/frontend/react'; +import { useState } from 'react'; import { SaveAsProjectModal } from '../../../components/modals/SaveAsProjectModal'; import type { InterlinearProjectSummary } from '../../../types/interlinear-project-summary'; @@ -193,6 +194,113 @@ describe('SaveAsProjectModal', () => { expect(screen.queryByText('Overwrite this project with the draft?')).not.toBeInTheDocument(); }); + it('hides the inline overwrite confirm on Escape, keeping the modal and its inputs open', async () => { + const onClose = jest.fn(); + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + render(); + + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + await userEvent.type(screen.getByLabelText(/^name$/i), 'Draft name'); + const row = screen.getByText('Unnamed').closest('li'); + if (!row) throw new Error('expected the project row to be present'); + await userEvent.click(within(row).getByRole('button', { name: 'Overwrite' })); + + await userEvent.keyboard('{Escape}'); + + expect(screen.queryByText('Overwrite this project with the draft?')).not.toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByLabelText(/^name$/i)).toHaveValue('Draft name'); + }); + + it('retires an armed overwrite confirm when the source changes under it', async () => { + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + const { rerender } = render(); + + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + const row = screen.getByText('Unnamed').closest('li'); + if (!row) throw new Error('expected the project row to be present'); + await userEvent.click(within(row).getByRole('button', { name: 'Overwrite' })); + + rerender(); + + // The same row comes back under the new source, so a confirm left armed would resurface with it. + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + expect(screen.queryByText('Overwrite this project with the draft?')).not.toBeInTheDocument(); + }); + + it('closes on a second Escape once the overwrite confirm has collapsed', async () => { + const onClose = jest.fn(); + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + render(); + + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + const row = screen.getByText('Unnamed').closest('li'); + if (!row) throw new Error('expected the project row to be present'); + await userEvent.click(within(row).getByRole('button', { name: 'Overwrite' })); + + await userEvent.keyboard('{Escape}'); + await userEvent.keyboard('{Escape}'); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('closes on Escape while the overwrite list is still loading, since the fetch has nothing to abandon', async () => { + const onClose = jest.fn(); + // Hold the load in-flight so Escape lands while the modal is still waiting on the list. + let resolveLoad: (v: string) => void = () => {}; + mockSendCommand.mockImplementation( + () => + new Promise((resolve) => { + resolveLoad = resolve; + }), + ); + render(); + await waitFor(() => expect(screen.getByRole('button', { name: /^cancel$/i })).toBeDisabled()); + + await userEvent.keyboard('{Escape}'); + + expect(onClose).toHaveBeenCalledTimes(1); + + // Settle the held load so its state update lands inside the test rather than after teardown. + resolveLoad('[]'); + await waitFor(() => + expect(screen.getByRole('button', { name: /^cancel$/i })).not.toBeDisabled(), + ); + }); + + it('stays silent when the overwrite-list load fails after the modal has been dismissed', async () => { + // Dismissal unmounts the modal in the real tree, so the host below mirrors that: a load that + // fails afterwards must not raise an error notification over a list nobody is waiting on. + let rejectLoad: (reason: unknown) => void = () => {}; + mockSendCommand.mockImplementation( + () => + new Promise((_resolve, reject) => { + rejectLoad = reject; + }), + ); + function Host() { + const [isOpen, setIsOpen] = useState(true); + return isOpen ? ( + setIsOpen(false)} /> + ) : undefined; + } + render(); + await waitFor(() => expect(screen.getByRole('button', { name: /^cancel$/i })).toBeDisabled()); + + await userEvent.keyboard('{Escape}'); + await waitFor(() => + expect(screen.queryByRole('button', { name: /^cancel$/i })).not.toBeInTheDocument(), + ); + + rejectLoad(new Error('failed after dismissal')); + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + expect(logger.error).not.toHaveBeenCalled(); + expect(papi.notifications.send).not.toHaveBeenCalled(); + }); + it('clears an armed overwrite confirm when the source changes so a stale target cannot be used', async () => { mockSendCommand .mockResolvedValueOnce(JSON.stringify([STUB_PROJECT])) @@ -220,7 +328,7 @@ describe('SaveAsProjectModal', () => { await waitFor(() => expect(logger.error).toHaveBeenCalledWith( - 'Interlinearizer: failed to load projects for Save As', + 'Interlinearizer: failed to load projects for source', loadError, ), ); diff --git a/src/__tests__/components/modals/SelectInterlinearProjectModal.test.tsx b/src/__tests__/components/modals/SelectInterlinearProjectModal.test.tsx index 7f128c7f..73577cad 100644 --- a/src/__tests__/components/modals/SelectInterlinearProjectModal.test.tsx +++ b/src/__tests__/components/modals/SelectInterlinearProjectModal.test.tsx @@ -3,8 +3,9 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import papi from '@papi/frontend'; +import papi, { logger } from '@papi/frontend'; import { useLocalizedStrings } from '@papi/frontend/react'; +import { useState } from 'react'; import { SelectInterlinearProjectModal } from '../../../components/modals/SelectInterlinearProjectModal'; import type { InterlinearProjectSummary } from '../../../types/interlinear-project-summary'; @@ -241,6 +242,108 @@ describe('SelectInterlinearProjectModal', () => { ); }); + it('disables Cancel and Create New while a chosen project is being opened', async () => { + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + render(); + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + + expect(screen.getByRole('button', { name: /^cancel$/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: /create new/i })).toBeDisabled(); + }); + + it('ignores clicks on a project row and its info button while a chosen project is being opened', async () => { + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + render(); + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + + const row = screen.getByRole('button', { name: /unnamed/i }); + const info = screen.getByRole('button', { name: /project info/i }); + expect(row).toBeDisabled(); + expect(info).toBeDisabled(); + + await userEvent.click(row); + await userEvent.click(info); + + expect(defaultProps.onSelect).not.toHaveBeenCalled(); + expect(defaultProps.onViewInfo).not.toHaveBeenCalled(); + }); + + it('ignores Escape while a chosen project is being opened, since the open completes regardless', async () => { + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + render(); + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + + await userEvent.keyboard('{Escape}'); + + expect(defaultProps.onClose).not.toHaveBeenCalled(); + }); + + it('closes on Escape once no open is in flight', async () => { + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); + render(); + await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument()); + + await userEvent.keyboard('{Escape}'); + + expect(defaultProps.onClose).toHaveBeenCalledTimes(1); + }); + + it('closes on Escape while the project list is still loading, since the fetch has nothing to abandon', async () => { + // Hold the load in-flight so Escape lands while the modal is still waiting on the list. + let resolveLoad: (v: string) => void = () => {}; + mockSendCommand.mockImplementation( + () => + new Promise((resolve) => { + resolveLoad = resolve; + }), + ); + render(); + await waitFor(() => expect(screen.getByRole('button', { name: /^cancel$/i })).toBeDisabled()); + + await userEvent.keyboard('{Escape}'); + + expect(defaultProps.onClose).toHaveBeenCalledTimes(1); + + // Settle the held load so its state update lands inside the test rather than after teardown. + resolveLoad('[]'); + await waitFor(() => + expect(screen.getByRole('button', { name: /^cancel$/i })).not.toBeDisabled(), + ); + }); + + it('stays silent when the project-list load fails after the modal has been dismissed', async () => { + // Dismissal unmounts the modal in the real tree, so the host below mirrors that: a load that + // fails afterwards must not raise an error notification over a list nobody is waiting on. + let rejectLoad: (reason: unknown) => void = () => {}; + mockSendCommand.mockImplementation( + () => + new Promise((_resolve, reject) => { + rejectLoad = reject; + }), + ); + function Host() { + const [isOpen, setIsOpen] = useState(true); + return isOpen ? ( + setIsOpen(false)} /> + ) : undefined; + } + render(); + await waitFor(() => expect(screen.getByRole('button', { name: /^cancel$/i })).toBeDisabled()); + + await userEvent.keyboard('{Escape}'); + await waitFor(() => + expect(screen.queryByRole('button', { name: /^cancel$/i })).not.toBeInTheDocument(), + ); + + rejectLoad(new Error('failed after dismissal')); + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + expect(logger.error).not.toHaveBeenCalled(); + expect(papi.notifications.send).not.toHaveBeenCalled(); + }); + it('clears the project list immediately when a new load begins', async () => { mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT])); const { rerender } = render(); diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index 72d2a034..9aab83a8 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -233,7 +233,7 @@ export function MorphemeBreakdownPopover({ return ( (undefined); - const [panelStyle, setPanelStyle] = useState<{ top: number; right: number }>({ - top: 0, - right: 0, - }); - - /** - * Ref callback that stores the toggle button element for future focus restoration. - * - * @param el - The mounted button, or `null` on unmount. - */ - const setButtonRef = (el: HTMLButtonElement | null) => { - buttonRef.current = el ?? undefined; - }; - - /** - * Closes the dropdown and returns focus to the toggle button so keyboard users don't lose their - * position. - */ - const close = () => { - setOpen(false); - buttonRef.current?.focus(); - }; - - // Position the panel under the button when the dropdown opens, then keep it anchored if the window - // resizes while it stays open (a resize shifts the button without remounting this effect). - useEffect(() => { - const button = buttonRef.current; - if (!open || !button) return undefined; - const updatePosition = () => { - const rect = button.getBoundingClientRect(); - setPanelStyle({ - top: rect.bottom + 4, - right: window.innerWidth - rect.right, - }); - }; - updatePosition(); - window.addEventListener('resize', updatePosition); - return () => window.removeEventListener('resize', updatePosition); - }, [open]); return (
- + + + + - {open && - createPortal( - /* Clicking outside the panel closes it. */ - <> - {/* The invisible backdrop must stay BELOW the toolbar's stacking context (tw:z-10, - from the sticky TabToolbarContainer) so the gear button's onClick — not the - backdrop — closes the dropdown. Raising the button instead wouldn't work: the - toolbar caps its descendants at z-10 against this portaled sibling. - Keep panel (z-30) > backdrop. */} - ); } diff --git a/src/components/modals/CreateProjectModal.tsx b/src/components/modals/CreateProjectModal.tsx index 1422d208..94349e8a 100644 --- a/src/components/modals/CreateProjectModal.tsx +++ b/src/components/modals/CreateProjectModal.tsx @@ -80,9 +80,10 @@ export function CreateProjectModal({ return (