diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 435d9058..f8e6a427 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -52,9 +52,11 @@ jobs: working-directory: extension-repo run: npm run test:coverage - e2e-smoke: + e2e: runs-on: ${{ matrix.os }} strategy: + # Don't cancel the other OS's e2e run when one fails — we want both reports. + fail-fast: false matrix: os: [ubuntu-latest, windows-latest] steps: @@ -112,15 +114,16 @@ jobs: working-directory: paranext-core run: npm run build:dll - - name: Run e2e smoke tests (Linux) + - name: Run e2e tests (Linux) if: matrix.os == 'ubuntu-latest' working-directory: extension-repo - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e:smoke + # Linux needs an xvfb display + run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e - - name: Run e2e smoke tests (Windows) + - name: Run e2e tests (Windows) if: matrix.os == 'windows-latest' working-directory: extension-repo - run: npm run test:e2e:smoke + run: npm run test:e2e - name: Upload Playwright test results uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -129,6 +132,7 @@ jobs: name: playwright-results-${{ matrix.os }} retention-days: 7 path: | + extension-repo/e2e-tests/.cdp-app-startup.log extension-repo/e2e-tests/playwright-report/ extension-repo/e2e-tests/test-results/ if-no-files-found: warn diff --git a/.gitignore b/.gitignore index a1dbba9e..13e0fd65 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +36,8 @@ temp-build # AI files .claude -# Playwright test output +# Playwright output +e2e-tests/.cdp-* +e2e-tests/.dev-* e2e-tests/playwright-report e2e-tests/test-results diff --git a/cspell.json b/cspell.json index 3046472d..09ef9b56 100644 --- a/cspell.json +++ b/cspell.json @@ -20,6 +20,7 @@ "BBCCCVVV", "believ", "clickability", + "cmdk", "cullable", "deconflict", "deconfliction", @@ -65,6 +66,7 @@ "Stylesheet", "typedefs", "unanalyzed", + "unglossed", "unhover", "unobserves", "unphrased", diff --git a/e2e-tests/README.md b/e2e-tests/README.md index db053ae1..94e2060c 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -1,6 +1,15 @@ # e2e-tests -End-to-end tests for the interlinearizer extension using Playwright + Electron. The suite launches a real Platform.Bible instance with the extension loaded via `--extensions` and verifies the extension starts up correctly. Currently contains one smoke test confirming the extension activates and registers its PAPI command. +End-to-end tests for the interlinearizer extension using Playwright + Electron. The suite has two tiers: + +- **Smoke tests** (`tests/smoke/`, `app.fixture`) launch a fresh Platform.Bible instance with the extension loaded via `--extensions` and verify the extension starts up correctly. +- **Feature tests** (`tests/features/`, `cdp.fixture`) connect over CDP to a running Platform.Bible instance and exercise interlinearizer UI flows (glossing, draft persistence, project modals). + +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`. + +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. Each tier writes its Playwright HTML report to its own subfolder (`playwright-report/smoke`, `playwright-report/cdp`) so a combined run keeps both. **Contents:** @@ -17,3 +26,17 @@ These tests are adapted from `paranext-core`'s e2e suite with changes to support - **Extension launch helper** — `fixtures/helpers.ts` uses `launchElectronWithExtension()` instead of `launchElectronApp()`. It passes `--extensions ` to the Electron process, resolves the Electron binary from paranext-core's `node_modules`, and polls `rpc.discover` for the extension's PAPI method to confirm activation. - **Window finding** — `fixtures/app.fixture.ts` manually polls `electronApp.windows()` by URL instead of calling `electronApp.firstWindow()`, because the extension injects content into an existing window rather than being the sole owner of the renderer. - **Renderer readiness** — `global-setup.ts` adds an HTTP GET probe after the TCP port check to wait for webpack compilation to finish, rather than assuming the port being open means the bundle is ready. + +## Writing feature tests + +Feature tests run with `npm run test:e2e:cdp`. That command launches a fresh, isolated instance, but the tests are also run against a shared, long-lived `npm run start:cdp` instance during local iteration (see above), so they must assume nothing about the instance's state and must leave nothing behind that could poison the next run. The protocol: + +- **Import from `cdp.fixture`, never `app.fixture`.** The CDP config already serializes execution (`workers: 1`), so tests never race each other on the shared instance. +- **The instance is only ever used with the WEB project.** This is an operating assumption, not something tests verify: `ensureInterlinearizerOpenOnWeb()` trusts an existing Interlinearizer tab and only picks WEB when opening fresh. Don't point `start:cdp` at other projects. +- **Mutating tests operate on the dedicated "E2E Test Project", never on a developer's own projects.** `ensureE2eProjectActive()` opens it (creating it on first use) at the start of each mutating test. Because the draft is the single per-source working buffer, replacing it could destroy unsaved developer work — so when the draft is dirty and the active project is _not_ the e2e project, the helper first saves the draft into a new `e2e-rescued-work-` project. Rescue projects are backups, not junk: delete them manually once recovered. Dirty state left while the e2e project is active is treated as leftover test data and discarded. +- **Self-establish every precondition.** Each mutating test starts with `ensureInterlinearizerOpenOnWeb()` → `ensureE2eProjectActive()` → `navigateToScriptureRef()` (the scroll-group reference could be anywhere) → `wipeDraft()`. +- **Reset at the start, tidy at the end.** The start-of-test sequence is what guarantees correctness — it self-heals whatever a failed run left behind. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers; this is a courtesy so the next run doesn't misread test junk as rescuable developer work, 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. +- **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/cdp.fixture.ts b/e2e-tests/fixtures/cdp.fixture.ts index 381306a9..b5d5e921 100644 --- a/e2e-tests/fixtures/cdp.fixture.ts +++ b/e2e-tests/fixtures/cdp.fixture.ts @@ -7,10 +7,12 @@ * * - No app restart needed (no port 8876 conflict) * - Tests run against the same app instance used during development - * - No teardown/shutdown of the app on completion + * - No teardown/shutdown of the app by the fixture on completion * - * Prerequisite: Platform.Bible running with --remote-debugging-port=9223 and the interlinearizer - * extension loaded. + * The app is provided by the CDP config's `globalSetup` (which self-launches one for `npm run + * test:e2e:cdp`) or by a developer's own `npm run start:cdp`; either way it must be running with + * `--remote-debugging-port=9223` and the interlinearizer extension loaded by the time this fixture + * connects. */ import { test as base, chromium, Page } from '@playwright/test'; diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index a8c604a0..538e24de 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -1,5 +1,13 @@ // Adapted from paranext-core/e2e-tests/fixtures/helpers.ts -import { _electron as electron, ElectronApplication, expect, Page } from '@playwright/test'; +import { + _electron as electron, + ElectronApplication, + expect, + FrameLocator, + Locator, + Page, +} from '@playwright/test'; +import escapeStringRegexp from 'escape-string-regexp'; import fs from 'fs'; import { createRequire } from 'module'; import os from 'os'; @@ -400,19 +408,35 @@ export async function waitForInterlinearizerReady(timeoutMs = 90_000): Promise { - // Focus the Scripture Editor tab (opens without a project at fresh start). - const editorTab = page.locator('.dock-tab', { hasText: 'Scripture Editor' }).first(); + // A Scripture Editor tab is titled by the project short name once a project is loaded (e.g. + // "WEB (Editable)"), and "Scripture Editor" only when no project is loaded. Escape projectName so + // a short name with regex metacharacters can't corrupt the pattern. + const escapedProjectName = escapeStringRegexp(projectName); + const editorTab = page + .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${escapedProjectName})\\b`) }) + .first(); + const homeTab = page.locator('.dock-tab', { hasText: 'Home' }).first(); + + // Wait for the dock layout to actually mount before deciding which path to take — a fresh profile + // briefly reports zero tabs, and a non-waiting `count()` would misread that as "no editor". + // `.first()` on the whole `.or()`: when both the editor and Home tabs are present the union + // resolves to two elements, which would trip strict mode on this visibility assertion. + await expect(editorTab.or(homeTab).first()).toBeVisible({ timeout: 45_000 }); + + // If the layout came up without a Scripture Editor (single-tab Home layout), open the project + // from Home so the editor (and its ≡ menu) exists before we try to focus it. + if ((await editorTab.count()) === 0) { + await homeTab.click(); + const homeFrame = page.frameLocator('iframe[title="Home"]'); + await homeFrame.locator(`tr:has-text("${projectName}") button:has-text("Open")`).click(); + } + await expect(editorTab).toBeVisible({ timeout: 15_000 }); await editorTab.click(); // The Scripture Editor renders its own toolbar inside its iframe. Click the ≡ ("Project") button. - const editorFrame = page.frameLocator('iframe[title*="Scripture Editor" i]'); + const editorFrame = page + .locator(`iframe[title*="Scripture Editor" i], iframe[title^="${projectName}"]`) + .first() + .contentFrame(); await editorFrame.locator("button[aria-label='Project']").first().click(); // Click the "Open Interlinearizer for this Project" item contributed by this extension. @@ -435,16 +483,339 @@ export async function openInterlinearizerFromScriptureEditor( .first() .click(); - // The command calls papi.dialogs.selectProject (because no project is selected in a fresh start), - // which opens a floating "Open Interlinearizer" dock tab with the project list. + // When the editor has no project selected, the command calls papi.dialogs.selectProject, which + // opens a floating "Open Interlinearizer" dock tab with the project list. When the editor + // already has a project (a warm instance), the Interlinearizer tab opens directly instead. const selectProjectDialog = page.locator('.select-project-dialog'); - await expect(selectProjectDialog).toBeVisible({ timeout: 15_000 }); - const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); - await selectProjectDialog.getByRole('button', { name: projectNameRegex }).click(); + const interlinearizerTab = interlinearizerTabLocator(page); + // `.first()` on the whole `.or()`: if the dialog and the tab are ever both present the union + // resolves to two elements, which would trip strict mode on this visibility assertion. + await expect(selectProjectDialog.or(interlinearizerTab).first()).toBeVisible({ timeout: 15_000 }); + if (await selectProjectDialog.isVisible()) { + const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); + await selectProjectDialog.getByRole('button', { name: projectNameRegex }).click(); + } // Wait for the Interlinearizer tab to appear and focus it. - const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); await interlinearizerTab.click(); } + +/** + * Frame locator for the Interlinearizer WebView's iframe, where all of the extension's own UI + * (toolbar, token strips, modals) renders. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns A `FrameLocator` scoped to the Interlinearizer WebView iframe. + */ +export function getInterlinearizerFrame(page: Page): FrameLocator { + // Anchor on titles that START with "Interlinearizer" so this never matches the project-picker + // dialog ("Open Interlinearizer"), whose title also contains the word. The real WebView title is + // "Interlinearizer" (optionally suffixed with the unsaved-changes marker), so a prefix match keeps + // the dirty-state title while excluding the "Open …" picker. + return page.frameLocator('iframe[title^="Interlinearizer" i]'); +} + +/** + * Locator for the Interlinearizer WebView's dock tab. Matches the tab whose title contains + * "Interlinearizer" while excluding the project-picker dialog's own dock tab ("Open + * Interlinearizer"), whose title also contains the word. Centralizes the exclusion so callers can't + * forget it (the `getInterlinearizerFrame` iframe uses a prefix match for the same purpose). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns A `Locator` for the Interlinearizer WebView dock tab. + */ +function interlinearizerTabLocator(page: Page): Locator { + return page + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) + .first(); +} + +/** + * Wait for Platform.Bible and the interlinearizer extension to finish starting up. Combines + * {@link waitForAppReady} and {@link waitForInterlinearizerReady}. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when `interlinearizer.openForWebView` is listed in `rpc.discover`. + * @throws If the app or extension do not finish starting up within their default timeouts. + */ +export async function waitForAppAndInterlinearizerReady(page: Page): Promise { + await waitForAppReady(page); + await waitForInterlinearizerReady(); +} + +/** + * Ensure the Interlinearizer is open and focused, reusing an existing tab when one is present. + * Standard precondition for feature tests running against the shared CDP instance: an existing + * Interlinearizer tab is trusted to be on the WEB project (the shared instance is only ever used + * with WEB — see e2e-tests/README.md); otherwise the tab is opened fresh via the Scripture Editor + * menu flow. Resolves only once the extension's toolbar has rendered inside the iframe. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the Interlinearizer tab is focused and its toolbar is interactive. + * @throws If the Interlinearizer cannot be opened or its toolbar does not render within the + * timeouts. + */ +export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise { + const interlinearizerTab = interlinearizerTabLocator(page); + + // Settle the dock layout before the non-retrying isVisible() branch below. The readiness helpers + // only poll rpc.discover, not the DOM, so without this a not-yet-painted Interlinearizer tab (or + // one just closed by a prior test) would read as "absent" and send us needlessly down the full + // open-from-editor flow. Wait until either the Interlinearizer tab or some editor/Home anchor tab + // is mounted, so isVisible() reflects a settled layout. When BOTH are present (the common case: + // an Interlinearizer tab alongside the WEB/editor tab), the union resolves to two elements, so + // `.first()` on the whole `.or()` keeps the visibility assertion out of strict-mode violation — + // per-operand `.first()` does not collapse the union to a single match. + const anchorTab = page.locator('.dock-tab', { hasText: /Scripture Editor|Home|WEB/ }).first(); + await expect(interlinearizerTab.or(anchorTab).first()).toBeVisible({ timeout: 30_000 }); + + if (await interlinearizerTab.isVisible()) { + await interlinearizerTab.click(); + } else { + await openInterlinearizerFromScriptureEditor(page); + } + const frame = getInterlinearizerFrame(page); + await expect(frame.locator("button[aria-label='Project']").first()).toBeVisible({ + timeout: 30_000, + }); +} + +/** + * Navigate the platform's book-chapter-verse control to the given scripture reference so tests can + * assert against known text. Opens the toolbar's reference combobox, types the reference, and + * submits it. Requires a fully-qualified reference (book, chapter, and verse — e.g. `"GEN 1:1"`); + * partial references are ambiguous and are not auto-submitted by the control. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param reference Fully-qualified scripture reference to navigate to (e.g. `"GEN 1:1"`). + * @returns Resolves when the reference popover has closed after submitting. + * @throws If the reference control does not open, or the popover does not close after submitting. + */ +export async function navigateToScriptureRef(page: Page, reference: string): Promise { + const trigger = page.locator('button[aria-label="book-chapter-trigger"]').first(); + await trigger.click(); + const input = page.locator('input[cmdk-input]').first(); + await expect(input).toBeVisible({ timeout: 5_000 }); + await input.fill(reference); + await input.press('Enter'); + // The popover closes when the reference is accepted. + await expect(input).not.toBeVisible({ timeout: 5_000 }); +} + +/** + * Open the Interlinearizer's ≡ ("Project") top menu inside its iframe and wait for the dropdown to + * appear. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns The frame locator for the Interlinearizer iframe, for chaining menu-item clicks. + * @throws If the menu button or the opened menu does not become visible within the timeouts. + */ +export async function openInterlinearizerProjectMenu(page: Page): Promise { + const frame = getInterlinearizerFrame(page); + const projectMenuButton = frame.locator("button[aria-label='Project']").first(); + await expect(projectMenuButton).toBeVisible({ timeout: 15_000 }); + await projectMenuButton.click(); + await expect(frame.locator('[role="menu"]')).toBeVisible({ timeout: 5_000 }); + return frame; +} + +/** Name of the dedicated interlinear project the mutating feature tests operate on. */ +export const E2E_PROJECT_NAME = 'E2E Test Project'; + +/** Name prefix for projects created to rescue a developer's unsaved draft work. */ +const RESCUE_PROJECT_PREFIX = 'e2e-rescued-work'; + +/** + * Glyph the Interlinearizer appends to its dock tab title while the draft has unsaved changes. Only + * the glyph must match production's UNSAVED_TAB_MARKER in src/components/InterlinearizerLoader.tsx + * (which prefixes it with a space); this constant is used exclusively in substring checks, so the + * surrounding whitespace is deliberately not replicated. + */ +const UNSAVED_TAB_MARKER = '●'; + +/** + * Read whether the draft has unsaved changes from the Interlinearizer dock tab's title marker (the + * only place the dirty state is observable outside the WebView). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns `true` when the tab title carries the unsaved-changes glyph. + */ +async function isDraftDirty(page: Page): Promise { + const tabText = await interlinearizerTabLocator(page).textContent(); + return (tabText ?? '').includes(UNSAVED_TAB_MARKER); +} + +/** + * Open the "Select Interlinear Project" modal from the Project menu and wait for its project list + * to finish loading (the modal's buttons are disabled while the fetch is in flight). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns The frame locator for the Interlinearizer iframe, for chaining clicks in the modal. + * @throws If the modal does not open or its list does not finish loading within the timeouts. + */ +async function openSelectProjectModal(page: Page): Promise { + const frame = await openInterlinearizerProjectMenu(page); + await frame + .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({ + timeout: 10_000, + }); + return frame; +} + +/** + * Preserve the current draft by saving it as a brand-new, timestamped rescue project (Project menu + * → Save As… → "Save as New Project"). Used before the e2e project replaces a dirty draft that may + * hold a developer's unsaved work, so nothing is silently discarded. Clears the draft's + * unsaved-changes state as a side effect (the rescue project becomes the active Save target). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the Save As modal has closed and the unsaved marker has cleared. + * @throws If the Save As modal does not open, close, or clear the unsaved marker within the + * timeouts. + */ +async function rescueDraftToNewProject(page: Page): Promise { + const frame = await openInterlinearizerProjectMenu(page); + await frame + .getByRole('menuitem', { name: /^Save As/i }) + .first() + .click(); + + const saveAsTitle = frame.locator('#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(); + await expect(saveAsTitle).not.toBeVisible({ timeout: 10_000 }); + + // The save clears the unsaved marker; wait for it so later dirty checks read the new state. + await expect(interlinearizerTabLocator(page)).not.toContainText(UNSAVED_TAB_MARKER, { + timeout: 10_000, + }); +} + +/** + * Make the dedicated e2e project ({@link E2E_PROJECT_NAME}) the active project, creating it if it + * does not exist yet, so mutating tests never touch a developer's own projects. Opening a project + * replaces the draft (the single per-source working buffer), so when the draft is dirty and the + * active project is NOT the e2e project — i.e. the unsaved work may be a developer's, not leftover + * test data — it is first rescued into a new `e2e-rescued-work-*` project instead of being + * discarded. Dirty state left while the e2e project is active is treated as leftover test data and + * discarded via the confirm dialog. + * + * Mutating tests call this at the START (with rescue on) to establish their precondition, and again + * at the END (with rescue off) to discard their own leftovers so the next run starts clean. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param opts Options object. + * @param opts.rescueDirtyDraft Whether a dirty draft not owned by the e2e project is rescued before + * being replaced (default `true`). Pass `false` only at the end of a test, where the dirty state + * is known to be the test's own leftovers. + * @returns Resolves when the e2e project is active and all modals have closed. + * @throws If the modals do not open/close or the project cannot be selected or created within the + * timeouts. + */ +export async function ensureE2eProjectActive( + page: Page, + opts: { rescueDirtyDraft?: boolean } = {}, +): Promise { + const { rescueDirtyDraft = true } = opts; + + let dirty = await isDraftDirty(page); + let frame = await openSelectProjectModal(page); + let dialog = frame.locator('dialog'); + + // Locate the E2E entry by its project-name element with an EXACT text match, then walk up to the + // enclosing entry button. Matching the whole button's accessible name doesn't work: the modal + // renders the name, an optional "Active" badge, and the analysis languages as adjacent inline + // s with no separating whitespace, so the accessible name reads "E2E Test Projecten" — + // there is no space after the name to anchor on. An exact-text match on the name element also + // avoids matching a different project whose name merely starts with E2E_PROJECT_NAME (e.g. "E2E + // Test Project 2"). Keep in sync with SelectInterlinearProjectModal's entry markup. + const activeEntry = dialog.locator('button[aria-current="true"]'); + const activeIsE2e = + (await activeEntry.count()) > 0 && + (await activeEntry.first().getByText(E2E_PROJECT_NAME, { exact: true }).count()) > 0; + + 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 rescueDraftToNewProject(page); + dirty = false; + frame = await openSelectProjectModal(page); + dialog = frame.locator('dialog'); + } + + const selectTitle = frame.locator('#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 }) }) + .first(); + if ((await e2eEntry.count()) > 0) { + await e2eEntry.click(); + if (dirty) { + // Replacing a dirty draft asks for confirmation; anything worth keeping was rescued above. + const discardConfirm = frame.getByTestId('discard-draft-confirm'); + await expect(discardConfirm).toBeVisible({ timeout: 5_000 }); + await discardConfirm.click(); + } + } else { + await dialog.getByRole('button', { name: 'Create New' }).click(); + const createTitle = frame.locator('#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(); + // 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) { + const discardConfirm = frame.getByTestId('discard-draft-confirm'); + await expect(discardConfirm).toBeVisible({ timeout: 5_000 }); + await discardConfirm.click(); + } + await expect(createTitle).not.toBeVisible({ timeout: 10_000 }); + } + await expect(selectTitle).not.toBeVisible({ timeout: 10_000 }); +} + +/** + * Wipe the entire draft's analysis via the visible UI (Project menu → Wipe… → "Entire draft" → + * Wipe). Standard reset step for mutating feature tests on the shared CDP instance: run it at the + * START of a test (never at the end) so a previously failed run self-heals instead of poisoning the + * next one. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the wipe dialog has closed after confirming. + * @throws If the wipe dialog does not open or does not close after confirming. + */ +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'); + await expect(wipeDialogTitle).toBeVisible({ timeout: 5_000 }); + await frame.getByTestId('wipe-scope-all').check(); + await frame.getByTestId('wipe-confirm').click(); + await expect(wipeDialogTitle).not.toBeVisible({ timeout: 10_000 }); +} + +/** + * Close the Interlinearizer dock tab via its close button and wait for it to disappear. Used by + * tests that verify draft persistence across a close/reopen cycle. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the Interlinearizer tab is gone. + * @throws If the tab is not visible, or the tab does not close within the timeout. + */ +export async function closeInterlinearizerTab(page: Page): Promise { + const interlinearizerTab = interlinearizerTabLocator(page); + await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); + // Dispatch the click rather than hover()+click(): the close button is only laid out on hover, and + // on small CI viewports the tab can overflow the tab strip and sit outside the viewport, where a + // real click (even with force) fails. dispatchEvent doesn't require the element to be in-viewport. + // Mirrors paranext-core's own dock-tab close helpers. + await interlinearizerTab.locator('.dock-tab-close-btn').dispatchEvent('click'); + await expect(interlinearizerTab).not.toBeVisible({ timeout: 10_000 }); +} diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts new file mode 100644 index 00000000..b05c2c0f --- /dev/null +++ b/e2e-tests/global-setup-cdp.ts @@ -0,0 +1,179 @@ +// Self-launching global setup for the CDP (feature-test) config. +import type { FullConfig } from '@playwright/test'; +import { spawn } from 'child_process'; +import { createRequire } from 'module'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + bootstrapRendererDevServer, + isPortInUse, + waitForPort, + WEBSOCKET_PORT, +} from './global-setup'; + +/** + * Chromium remote-debugging port the self-launched Electron instance exposes and the CDP fixture + * connects to. Kept in sync with the `CDP_URL` default in fixtures/cdp.fixture.ts and the + * `--remote-debugging-port` in the `start:cdp` npm script. + */ +export const CDP_PORT = 9223; + +/** File the launched Electron PID is written to, for {@link globalTeardownCdp} to kill it. */ +export const CDP_PID_FILE = path.join(__dirname, '.cdp-app.pid'); + +/** File the launched Electron's isolated user-data dir is written to, for teardown to remove it. */ +export const CDP_USER_DATA_FILE = path.join(__dirname, '.cdp-app.user-data-dir'); + +/** + * File the launched app's stdout/stderr is streamed to. Kept alongside the other `.cdp-*` marker + * files in `e2e-tests/` — a location Playwright does not clear (unlike `outputDir`) — and added to + * the CI artifact upload so it survives a failed run. Without this the app is spawned `stdio: + * 'ignore'` and a startup crash surfaces only as an opaque WebSocket-port timeout with no cause. + */ +export const CDP_APP_LOG_FILE = path.join(__dirname, '.cdp-app-startup.log'); + +/** How long to wait for the launched app's WebSocket / CDP port before failing setup. */ +const APP_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; + +/** + * Playwright global setup for the CDP config. Unlike the smoke config — whose fixture launches + * Electron per worker — the CDP fixture connects over CDP to a separately-running app. This setup + * provides that app so `npm run test:e2e:cdp` is self-contained (no manual `npm run start:cdp`): + * + * 1. Bootstraps the renderer dev server via {@link bootstrapRendererDevServer}. + * 2. Launches Electron (paranext-core) detached, with the interlinearizer extension loaded via + * `--extensions` and Chromium remote debugging on {@link CDP_PORT}, in an isolated user-data + * dir. + * 3. Waits for the PAPI WebSocket and the CDP debug port to come up. + * 4. Records the PID and user-data dir for {@link globalTeardownCdp}. + * + * The isolated user-data dir means the run never touches a developer's real profile; the feature + * tests self-establish every precondition (they create the E2E project, navigate, and wipe the + * draft at the start of each test), so a fresh profile is sufficient. + * + * If the CDP port is already in use, a warm instance is assumed (a developer's own `npm run + * start:cdp`) and setup is a no-op: no app is launched and nothing is recorded for teardown, so the + * developer's instance is reused and left running. This keeps the manual + * iterate-against-a-warm-instance workflow working through the same config. + * + * @param _config Playwright config object — unused; required by Playwright's global-setup + * interface. + * @returns Resolves once a usable app is available (launched here, or an already-running one). + * @throws {Error} If the app's WebSocket or CDP port do not become ready in time. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export default async function globalSetupCdp(_config: FullConfig): Promise { + // A warm instance already owns the CDP port (a developer's `npm run start:cdp`). Reuse it: don't + // launch a second app (it would collide on the WebSocket singleton and exit) and don't record a + // PID, so teardown leaves the developer's instance running. + if (await isPortInUse(CDP_PORT)) { + console.log( + `CDP port ${CDP_PORT} already in use — reusing the already-running Platform.Bible instance ` + + '(not launching or tearing down an app).', + ); + return; + } + + await bootstrapRendererDevServer(); + + const coreDir = path.resolve(__dirname, '../../paranext-core'); + const extensionDist = path.resolve(__dirname, '../dist'); + + // Resolve the Electron binary from paranext-core's node_modules (its `electron` package's default + // export is the path to the platform binary). + const coreRequire = createRequire(path.resolve(coreDir, 'package.json')); + // eslint-disable-next-line no-type-assertion/no-type-assertion + const electronExecutable = coreRequire('electron') as string; + + // Isolated user-data dir so the singleton lock can't collide with a developer's own instance and + // the run leaves the real profile untouched. + const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'paranext-e2e-cdp-')); + fs.writeFileSync(CDP_USER_DATA_FILE, userDataDir); + + // VSCode/Claude Code set ELECTRON_RUN_AS_NODE=1, which forces the Electron binary to run as plain + // Node.js. Omit it so the Electron child launches a real GUI process. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { ELECTRON_RUN_AS_NODE, ...restEnv } = process.env; + + console.log(`Launching Platform.Bible (CDP) from: ${coreDir}`); + console.log(`Loading extension from: ${extensionDist}`); + console.log(`Remote debugging on port ${CDP_PORT}`); + + // Stream the app's output to a log file rather than discarding it (`stdio: 'ignore'`): when the + // app crashes on startup the only other symptom is an opaque WebSocket-port timeout below. + const appLogFd = fs.openSync(CDP_APP_LOG_FILE, 'w'); + + // Detached: unlike the smoke fixture's Playwright-owned `_electron.launch()`, the CDP fixture + // connects to this process over CDP, so Playwright must not own its lifecycle. Teardown kills the + // whole process tree by the recorded PID. + const appProcess = spawn( + electronExecutable, + [ + `--user-data-dir=${userDataDir}`, + coreDir, + '--extensions', + extensionDist, + `--remote-debugging-port=${CDP_PORT}`, + // GitHub-hosted Linux runners don't ship a root-owned setuid chrome-sandbox binary, so + // Electron's SUID sandbox helper aborts on launch. Skip the OS sandbox on Linux. + ...(process.platform === 'linux' ? ['--no-sandbox'] : []), + ], + { + cwd: coreDir, + env: { ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false' }, + stdio: ['ignore', appLogFd, appLogFd], + detached: true, + }, + ); + appProcess.unref(); + // The child has inherited the fd; close our copy so the file is flushed and released on exit. + fs.closeSync(appLogFd); + + if (appProcess.pid) fs.writeFileSync(CDP_PID_FILE, String(appProcess.pid)); + + console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); + /** + * Rejects the moment {@link appProcess} exits, so a startup crash (e.g. a sandbox + * misconfiguration) fails setup immediately instead of only surfacing after the full + * {@link APP_READY_TIMEOUT} port-wait below elapses. + */ + const earlyExit = new Promise((_resolve, reject) => { + appProcess.once('exit', (code, signal) => { + reject( + new Error( + `Launched Platform.Bible (CDP) process exited early (code=${code}, signal=${signal}) ` + + 'before its WebSocket port came up.', + ), + ); + }); + }); + try { + await Promise.race([waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT), earlyExit]); + } catch (error) { + // The app never came up. Echo its captured output so the failure cause is in the CI log itself, + // not just buried in the uploaded artifact, then re-throw the original error. + dumpAppLog(); + throw error; + } + console.log(`Waiting for CDP debug port ${CDP_PORT}...`); + await waitForPort(CDP_PORT, APP_READY_TIMEOUT); + console.log('Platform.Bible (CDP) is ready.'); +} + +/** + * Print the launched app's captured stdout/stderr to the console. Called when the app fails to open + * its ports so the startup failure's cause appears inline in the CI log. + * + * @returns Nothing; logging-only. + */ +function dumpAppLog(): void { + try { + const log = fs.readFileSync(CDP_APP_LOG_FILE, 'utf-8'); + console.error( + `--- Launched Platform.Bible (CDP) output (${CDP_APP_LOG_FILE}) ---\n${log || '(empty)'}\n--- end app output ---`, + ); + } catch { + console.error(`Could not read app log at ${CDP_APP_LOG_FILE}.`); + } +} diff --git a/e2e-tests/global-setup.ts b/e2e-tests/global-setup.ts index 327a1bc7..c40ba3d7 100644 --- a/e2e-tests/global-setup.ts +++ b/e2e-tests/global-setup.ts @@ -6,8 +6,8 @@ import net from 'net'; import path from 'path'; import fs from 'fs'; -const WEBSOCKET_PORT = 8876; -const RENDERER_PORT = 1212; +export const WEBSOCKET_PORT = 8876; +export const RENDERER_PORT = 1212; /** * Check if a port is already in use. @@ -15,7 +15,7 @@ const RENDERER_PORT = 1212; * @param port Port number to probe. * @returns Resolves to `true` if the port is occupied, `false` if it is free. */ -function isPortInUse(port: number): Promise { +export function isPortInUse(port: number): Promise { return new Promise((resolve) => { const server = net.createServer(); server.once('error', () => { @@ -112,7 +112,7 @@ function waitForHttpOk(url: string, timeout: number): Promise { * @returns Resolves when a TCP connection to the port succeeds. * @throws {Error} If the port does not become available within `timeout` milliseconds. */ -function waitForPort(port: number, timeout: number): Promise { +export function waitForPort(port: number, timeout: number): Promise { const startTime = Date.now(); return new Promise((resolve, reject) => { /** Attempt one TCP connection; retries after 500 ms on failure within the overall timeout. */ @@ -136,24 +136,17 @@ function waitForPort(port: number, timeout: number): Promise { } /** - * Playwright global setup. Runs once before any test worker starts. + * 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). Shared by both the smoke {@link globalSetup} (whose fixture then + * launches Electron) and the CDP setup (which launches Electron itself with remote debugging). * - * 1. Fails fast if port 8876 is already in use (a running Platform.Bible would conflict with the - * Electron instance launched by fixtures). - * 2. Removes stale Electron singleton lock files left behind by crashes. - * 3. Fails fast if the extension dist is missing (directs the developer to run `npm run build`). - * 4. Ensures the paranext-core dev main bundle exists, building it via `npm run prestart` if not. - * 5. Starts the paranext-core webpack renderer dev server on port 1212 if not already running, and - * stores its PID for {@link globalTeardown} to stop it. - * - * @param _config Playwright config object — unused; required by Playwright's global-setup - * interface. * @returns Resolves when the renderer dev server is ready. - * @throws {Error} If port 8876 is already in use. + * @throws {Error} If port 8876 is already in use (a running Platform.Bible would conflict). * @throws {Error} If the extension dist is missing. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export default async function globalSetup(_config: FullConfig): Promise { +export async function bootstrapRendererDevServer(): Promise { const extensionRoot = path.resolve(__dirname, '..'); const coreDir = path.resolve(__dirname, '../../paranext-core'); @@ -248,3 +241,19 @@ export default async function globalSetup(_config: FullConfig): Promise { console.log('Renderer dev server is ready.'); } } + +/** + * Playwright global setup for the smoke config. Runs once before any test worker starts. Bootstraps + * the renderer dev server via {@link bootstrapRendererDevServer}; the smoke fixture + * (`app.fixture.ts`) then launches its own Electron instance per worker. + * + * @param _config Playwright config object — unused; required by Playwright's global-setup + * interface. + * @returns Resolves when the renderer dev server is ready. + * @throws {Error} If port 8876 is already in use. + * @throws {Error} If the extension dist is missing. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export default async function globalSetup(_config: FullConfig): Promise { + await bootstrapRendererDevServer(); +} diff --git a/e2e-tests/global-teardown-cdp.ts b/e2e-tests/global-teardown-cdp.ts new file mode 100644 index 00000000..1b7fb7b7 --- /dev/null +++ b/e2e-tests/global-teardown-cdp.ts @@ -0,0 +1,63 @@ +// Teardown for the self-launching CDP (feature-test) config. +import type { FullConfig } from '@playwright/test'; +import fs from 'fs'; +import { CDP_PID_FILE, CDP_USER_DATA_FILE } from './global-setup-cdp'; +import globalTeardown from './global-teardown'; +import { killProcessTree } from './process-utils'; + +/** + * Playwright global teardown for the CDP config. Kills the Electron instance launched by + * {@link globalSetupCdp} (by the PID recorded in {@link CDP_PID_FILE}), removes its isolated + * user-data dir, then delegates to the shared {@link globalTeardown} to stop the renderer dev server + * and sweep any lingering core processes. + * + * @param config Playwright config object — forwarded to the shared teardown. + * @returns Resolves when the launched app is killed, its user-data dir removed, and shared teardown + * has completed. + */ +export default async function globalTeardownCdp(config: FullConfig): Promise { + // Kill the app we launched (whole process tree) before the shared teardown's generic sweep. + // SIGKILL/`/F` (not SIGTERM) to match the smoke teardown: Electron can ignore SIGTERM, and we + // need it fully dead before removing its user-data dir below. + let appKilled = false; + if (fs.existsSync(CDP_PID_FILE)) { + const pid = parseInt(fs.readFileSync(CDP_PID_FILE, 'utf-8').trim(), 10); + if (Number.isNaN(pid)) { + console.warn(`Invalid PID in ${CDP_PID_FILE}, skipping app kill`); + } else { + console.log(`Stopping self-launched Platform.Bible (CDP) app (PID: ${pid})...`); + appKilled = killProcessTree(pid, 'SIGKILL'); + } + fs.unlinkSync(CDP_PID_FILE); + } + + // Remove the isolated user-data dir created for this run. Give the just-killed Electron a moment + // to release the SingletonLock and flush files before removing, then retry once — the smoke + // teardown removes its dir the same defensive way. + if (fs.existsSync(CDP_USER_DATA_FILE)) { + const userDataDir = fs.readFileSync(CDP_USER_DATA_FILE, 'utf-8').trim(); + if (userDataDir) { + if (appKilled) { + await new Promise((resolve) => { + setTimeout(resolve, 1_000); + }); + } + try { + fs.rmSync(userDataDir, { recursive: true, force: true }); + } catch { + await new Promise((resolve) => { + setTimeout(resolve, 3_000); + }); + try { + fs.rmSync(userDataDir, { recursive: true, force: true }); + } catch (e) { + console.warn(`Could not remove CDP user-data dir ${userDataDir}: ${e}`); + } + } + } + fs.unlinkSync(CDP_USER_DATA_FILE); + } + + // Delegate to the shared teardown to stop the renderer dev server and sweep lingering processes. + await globalTeardown(config); +} diff --git a/e2e-tests/global-teardown.ts b/e2e-tests/global-teardown.ts index 8739e466..4e5902b4 100644 --- a/e2e-tests/global-teardown.ts +++ b/e2e-tests/global-teardown.ts @@ -3,6 +3,7 @@ import type { FullConfig } from '@playwright/test'; import { execSync } from 'child_process'; import path from 'path'; import fs from 'fs'; +import { killProcessTree } from './process-utils'; /** * Playwright global teardown. Runs once after all test workers have finished. @@ -28,15 +29,7 @@ export default async function globalTeardown(_config: FullConfig): Promise fs.unlinkSync(pidFile); } else { console.log(`Stopping renderer dev server (PID: ${pid})...`); - try { - process.kill(-pid, 'SIGTERM'); - } catch { - try { - process.kill(pid, 'SIGTERM'); - } catch { - // Already stopped - } - } + killProcessTree(pid, 'SIGTERM'); fs.unlinkSync(pidFile); } } diff --git a/e2e-tests/playwright-cdp.config.ts b/e2e-tests/playwright-cdp.config.ts index 0e1b8d52..7d373006 100644 --- a/e2e-tests/playwright-cdp.config.ts +++ b/e2e-tests/playwright-cdp.config.ts @@ -2,20 +2,24 @@ import { defineConfig } from '@playwright/test'; /** - * Playwright configuration for running E2E tests against an already-running Platform.Bible instance - * with CDP enabled (port 9223). + * Playwright configuration for the CDP (feature) test tier. These tests connect over CDP (port + * 9223) to a running Platform.Bible instance with the interlinearizer extension loaded. * - * Prerequisites: Platform.Bible running with --remote-debugging-port=9223 and the interlinearizer - * extension loaded. - * - * Use: npx playwright test --config=e2e-tests/playwright-cdp.config.ts + * `globalSetup` launches that instance automatically (with `--remote-debugging-port=9223`) and + * `globalTeardown` shuts it down, so `npm run test:e2e:cdp` is self-contained — no manual `npm run + * start:cdp` first. If the CDP port is already taken, the setup instead reuses the running instance + * and leaves it up, so a developer iterating against a warm `npm run start:cdp` instance can point + * Playwright at it directly (`npx playwright test --config e2e-tests/playwright-cdp.config.ts`). */ export default defineConfig({ testDir: './tests', testIgnore: ['**/smoke/**', '**/_example/**'], fullyParallel: false, workers: 1, - reporter: [['html', { outputFolder: 'playwright-report' }], ['list']], + // Tier-specific report/output folders so a combined `npm run test:e2e` run keeps both tiers' + // reports side by side instead of the cdp run overwriting the smoke run's. `open: 'never'` keeps + // a run from auto-launching a browser in CI. + reporter: [['html', { outputFolder: 'playwright-report/cdp', open: 'never' }], ['list']], timeout: 120_000, expect: { timeout: 10_000 }, use: { @@ -23,6 +27,7 @@ export default defineConfig({ screenshot: 'only-on-failure', video: 'retain-on-failure', }, - outputDir: './test-results', - // NO globalSetup/globalTeardown — app is already running + globalSetup: './global-setup-cdp.ts', + globalTeardown: './global-teardown-cdp.ts', + outputDir: './test-results/cdp', }); diff --git a/e2e-tests/playwright.config.ts b/e2e-tests/playwright.config.ts index e47a532f..a0066752 100644 --- a/e2e-tests/playwright.config.ts +++ b/e2e-tests/playwright.config.ts @@ -20,7 +20,10 @@ export default defineConfig({ forbidOnly: !!process.env.CI, retries: process.env.CI ? 2 : 1, workers: 1, - reporter: [['html', { outputFolder: 'playwright-report' }], ['list']], + // Tier-specific report/output folders so `npm run test:e2e` (smoke then cdp, sequentially) doesn't + // have the second tier's report overwrite the first's. The `open: 'never'` keeps a run from + // auto-launching a browser in CI. + reporter: [['html', { outputFolder: 'playwright-report/smoke', open: 'never' }], ['list']], timeout: 120_000, expect: { timeout: 10_000, @@ -32,7 +35,7 @@ export default defineConfig({ }, globalSetup: './global-setup.ts', globalTeardown: './global-teardown.ts', - outputDir: './test-results', + outputDir: './test-results/smoke', projects: [ { name: 'smoke', diff --git a/e2e-tests/process-utils.ts b/e2e-tests/process-utils.ts new file mode 100644 index 00000000..4d63b26c --- /dev/null +++ b/e2e-tests/process-utils.ts @@ -0,0 +1,47 @@ +// Cross-platform process-tree termination, shared by global-teardown.ts and global-teardown-cdp.ts. +import { execFileSync } from 'child_process'; + +/** + * Forcibly kill a process and all of its descendants, cross-platform. + * + * Both the renderer dev server (`npm run start:renderer`, spawned via a shell) and the + * self-launched Electron app spawn a tree of child processes (webpack workers; + * main/renderer/GPU/utility). Killing only the top PID leaves the children running, which for the + * dev server means orphaned webpack workers and for Electron means a held PAPI WebSocket port that + * poisons the next run's fast-fail port check. + * + * - On Windows there is no process group and `process.kill(-pid)` is meaningless, so shell out to + * `taskkill /T /F`, which terminates the process and its entire descendant tree. `taskkill` + * always force-kills (no graceful-signal equivalent), so `signal` is ignored on this branch. + * - Elsewhere the target was spawned `detached`, so it is its own process-group leader: signal the + * negative PID to signal the whole group in one call, falling back to the bare PID if the group + * is already gone. + * + * @param pid PID of the detached process to kill. + * @param signal POSIX signal to send when not on Windows (ignored on Windows, which always + * force-kills). Defaults to `'SIGTERM'`. + * @returns `true` if a kill was issued, `false` if the process was already gone. + */ +export function killProcessTree(pid: number, signal: NodeJS.Signals = 'SIGTERM'): boolean { + if (process.platform === 'win32') { + try { + execFileSync('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); + return true; + } catch { + // taskkill exits non-zero when the process is already gone — nothing left to kill. + return false; + } + } + try { + process.kill(-pid, signal); + return true; + } catch { + try { + process.kill(pid, signal); + return true; + } catch { + // Already stopped + return false; + } + } +} diff --git a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts index 0b583275..8ec02feb 100644 --- a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts +++ b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts @@ -14,7 +14,7 @@ * This file is excluded from test runs — it's documentation only. */ import { test, expect } from '../../fixtures/cdp.fixture'; -import { waitForAppReady, waitForInterlinearizerReady } from '../../fixtures/helpers'; +import { waitForAppAndInterlinearizerReady } from '../../fixtures/helpers'; /** * Filter out expected/benign console errors from a list of captured error messages. @@ -34,8 +34,7 @@ function filterConsoleErrors(errors: string[]): string[] { test.describe('Example: Open Interlinearizer via menu', () => { test('should open the interlinearizer WebView via menu', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); // Step 1: Click the top-level menu that contains the interlinearizer entry const menuTrigger = mainPage.getByRole('menuitem', { name: /Tools/i }); @@ -51,8 +50,7 @@ test.describe('Example: Open Interlinearizer via menu', () => { }); test('should render without critical console errors', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); const consoleErrors: string[] = []; mainPage.on('console', (msg) => { diff --git a/e2e-tests/tests/features/draft-persistence.spec.ts b/e2e-tests/tests/features/draft-persistence.spec.ts new file mode 100644 index 00000000..5f08c882 --- /dev/null +++ b/e2e-tests/tests/features/draft-persistence.spec.ts @@ -0,0 +1,61 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + closeInterlinearizerTab, + ensureE2eProjectActive, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + navigateToScriptureRef, + waitForAppAndInterlinearizerReady, + wipeDraft, +} from '../../fixtures/helpers'; + +test.describe('Draft persistence', () => { + test('a glossed draft survives closing and reopening the interlinearizer', async ({ + mainPage, + }) => { + // Two full open cycles (plus first-use project creation and book loading on a cold instance) + // legitimately exceed the default 120 s budget. + test.slow(); + await waitForAppAndInterlinearizerReady(mainPage); + await ensureInterlinearizerOpenOnWeb(mainPage); + await ensureE2eProjectActive(mainPage); + await navigateToScriptureRef(mainPage, 'GEN 1:1'); + await wipeDraft(mainPage); + + const frame = getInterlinearizerFrame(mainPage); + const glossInput = frame.getByLabel('Gloss for beginning', { exact: true }).first(); + await expect(glossInput).toBeVisible({ timeout: 30_000 }); + + // Unique per run so a leftover value from a previous run can never false-pass. + const gloss = `e2e-persist-${Date.now()}`; + await glossInput.click(); + await glossInput.fill(gloss); + await glossInput.press('Tab'); + await expect(glossInput).toHaveValue(gloss); + + // The draft auto-saves on a 300 ms debounce after the last keystroke, and there is no UI + // signal that the storage write has landed (the tab's dirty marker tracks draft-vs-saved- + // project, not draft-vs-storage). This fixed wait deliberately covers the debounce so the + // test exercises the debounced-save path; the unmount-flush (close immediately after + // typing) path is intentionally out of scope here. + await mainPage.waitForTimeout(1_000); + + await closeInterlinearizerTab(mainPage); + + await ensureInterlinearizerOpenOnWeb(mainPage); + await navigateToScriptureRef(mainPage, 'GEN 1:1'); + + const reopenedFrame = getInterlinearizerFrame(mainPage); + const reopenedGlossInput = reopenedFrame + .getByLabel('Gloss for beginning', { exact: true }) + .first(); + await expect(reopenedGlossInput).toBeVisible({ timeout: 30_000 }); + await expect(reopenedGlossInput).toHaveValue(gloss); + + // Discard this test's leftover gloss (reload the e2e project into the draft) so the next run + // starts clean instead of triggering the dirty-draft rescue. Rescue must stay off here: the + // close/reopen dropped the active-project WebView state, so the dirty draft would otherwise + // look like developer work. + await ensureE2eProjectActive(mainPage, { rescueDirtyDraft: false }); + }); +}); diff --git a/e2e-tests/tests/features/gloss-roundtrip.spec.ts b/e2e-tests/tests/features/gloss-roundtrip.spec.ts new file mode 100644 index 00000000..93169946 --- /dev/null +++ b/e2e-tests/tests/features/gloss-roundtrip.spec.ts @@ -0,0 +1,39 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + ensureE2eProjectActive, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + navigateToScriptureRef, + waitForAppAndInterlinearizerReady, + wipeDraft, +} from '../../fixtures/helpers'; + +test.describe('Gloss round-trip', () => { + test('typing a gloss on a token renders it in the gloss field', async ({ mainPage }) => { + await waitForAppAndInterlinearizerReady(mainPage); + await ensureInterlinearizerOpenOnWeb(mainPage); + await ensureE2eProjectActive(mainPage); + await navigateToScriptureRef(mainPage, 'GEN 1:1'); + await wipeDraft(mainPage); + + const frame = getInterlinearizerFrame(mainPage); + + // WEB Genesis 1:1: "In the beginning, God created the heavens and the earth." + const glossInput = frame.getByLabel('Gloss for beginning', { exact: true }).first(); + await expect(glossInput).toBeVisible({ timeout: 30_000 }); + // The wipe just cleared all analysis, so the field must start empty. + await expect(glossInput).toHaveValue(''); + + // Unique per run so a leftover value from a previous run can never false-pass. + const gloss = `e2e-gloss-${Date.now()}`; + await glossInput.click(); + await glossInput.fill(gloss); + await glossInput.press('Tab'); + + await expect(glossInput).toHaveValue(gloss); + + // Discard this test's leftover gloss (reload the e2e project into the draft) so the next run + // starts clean instead of triggering the dirty-draft rescue. + await ensureE2eProjectActive(mainPage, { rescueDirtyDraft: false }); + }); +}); diff --git a/e2e-tests/tests/features/project-modals.spec.ts b/e2e-tests/tests/features/project-modals.spec.ts new file mode 100644 index 00000000..e826b39f --- /dev/null +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -0,0 +1,59 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + 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. + */ +const MODAL_TOURS = [ + { + name: 'Select Interlinear Project', + menuItem: /Select Interlinear Project/i, + titleSelector: '#select-project-modal-title', + }, + { + name: 'New Interlinear Project', + menuItem: /New Interlinear Project/i, + titleSelector: '#create-project-modal-title', + }, + { + name: 'Save As', + menuItem: /^Save As/i, + titleSelector: '#save-as-modal-title', + }, +]; + +test.describe('Project modals cancel tour', () => { + MODAL_TOURS.forEach((modal) => { + test(`the ${modal.name} modal opens from the Project menu and cancels cleanly`, async ({ + mainPage, + }) => { + await waitForAppAndInterlinearizerReady(mainPage); + await ensureInterlinearizerOpenOnWeb(mainPage); + + const frame = await openInterlinearizerProjectMenu(mainPage); + await frame.getByRole('menuitem', { name: modal.menuItem }).first().click(); + + const modalTitle = frame.locator(modal.titleSelector); + await expect(modalTitle).toBeVisible({ timeout: 5_000 }); + + await frame.locator('dialog').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 + // overlay is the most common real modal regression. + const projectMenuButton = getInterlinearizerFrame(mainPage) + .locator("button[aria-label='Project']") + .first(); + await expect(projectMenuButton).toBeVisible({ timeout: 5_000 }); + await expect(projectMenuButton).toBeEnabled(); + }); + }); +}); diff --git a/e2e-tests/tests/smoke/extension-launch.spec.ts b/e2e-tests/tests/smoke/extension-launch.spec.ts index 077bce75..7e76c4ef 100644 --- a/e2e-tests/tests/smoke/extension-launch.spec.ts +++ b/e2e-tests/tests/smoke/extension-launch.spec.ts @@ -1,5 +1,5 @@ import { test, expect } from '../../fixtures/app.fixture'; -import { waitForAppReady, waitForInterlinearizerReady } from '../../fixtures/helpers'; +import { waitForAppAndInterlinearizerReady, waitForAppReady } from '../../fixtures/helpers'; test.describe('Launch app and register Interlinearizer', () => { test('should launch Platform.Bible and create at least one window', async ({ electronApp }) => { @@ -19,7 +19,6 @@ test.describe('Launch app and register Interlinearizer', () => { }); test('should register Interlinearizer PAPI commands', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); }); }); diff --git a/e2e-tests/tests/smoke/open-interlinearizer.spec.ts b/e2e-tests/tests/smoke/open-interlinearizer.spec.ts index af440cc8..39b8fac8 100644 --- a/e2e-tests/tests/smoke/open-interlinearizer.spec.ts +++ b/e2e-tests/tests/smoke/open-interlinearizer.spec.ts @@ -1,14 +1,12 @@ import { expect, test } from '../../fixtures/app.fixture'; import { openInterlinearizerFromScriptureEditor, - waitForAppReady, - waitForInterlinearizerReady, + waitForAppAndInterlinearizerReady, } from '../../fixtures/helpers'; test.describe('Open Interlinearizer', () => { test('should open the interlinearizer and see its menus', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); await openInterlinearizerFromScriptureEditor(mainPage); diff --git a/package-lock.json b/package-lock.json index 12c9754d..5d3961fc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3916,17 +3916,17 @@ "license": "MIT" }, "node_modules/@typescript-eslint/eslint-plugin": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.62.1.tgz", - "integrity": "sha512-4EQM77WgVNxj7OkL/5b/D/xZsw00G577+UriYTC7JF5opcF3T2AuoeY7ueLaZgSVjSgCS6yOAJB5bRGLPSJUzA==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.63.0.tgz", + "integrity": "sha512-rvwSgqT+DHpWdzfSzPatRLm02a0GlESt++9iy3hLCDY4BgkaLcl8LBi9Yh7XGFBpwcBE/K3024QuXWTpbz4FfQ==", "dev": true, "license": "MIT", "dependencies": { "@eslint-community/regexpp": "^4.12.2", - "@typescript-eslint/scope-manager": "8.62.1", - "@typescript-eslint/type-utils": "8.62.1", - "@typescript-eslint/utils": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1", + "@typescript-eslint/scope-manager": "8.63.0", + "@typescript-eslint/type-utils": "8.63.0", + "@typescript-eslint/utils": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0", "ignore": "^7.0.5", "natural-compare": "^1.4.0", "ts-api-utils": "^2.5.0" @@ -3939,22 +3939,22 @@ "url": "https://opencollective.com/typescript-eslint" }, "peerDependencies": { - "@typescript-eslint/parser": "^8.62.1", + "@typescript-eslint/parser": "^8.63.0", "eslint": "^8.57.0 || ^9.0.0 || ^10.0.0", "typescript": ">=4.8.4 <6.1.0" } }, "node_modules/@typescript-eslint/parser": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.62.1.tgz", - "integrity": "sha512-sPhE4iHuJDSvoAiec+Ro8JyXw8f0ql13HFR82P99nCm9GwTEKG0KYLvDe6REk8BCXuit6vJAv/Yxg5ABaNS2rA==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.63.0.tgz", + "integrity": "sha512-gwh4gvvlaVDKKxyfxMG+Gnu1u9X0OQBwyGLkbwB65dIzBKnxeRiJlNFqlI3zwVhNXJIs6qV7mlFCn/BIajlVig==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/scope-manager": "8.62.1", - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/typescript-estree": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1", + "@typescript-eslint/scope-manager": "8.63.0", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/typescript-estree": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0", "debug": "^4.4.3" }, "engines": { @@ -3970,14 +3970,14 @@ } }, "node_modules/@typescript-eslint/project-service": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.62.1.tgz", - "integrity": "sha512-yQ3RgY5RkSBpsNS1Bx/JQEcA24FOSdfGktoyprAr5u18390UQdtVcfnEv4nIrIshNnavlVyZBKxQwT1fIAE6cg==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.63.0.tgz", + "integrity": "sha512-e5dh0/UI0ok53AlZ5wRkXCB32z/f2jUZqPR/ygAw5WYaSw8j9EoJWlS7wQjr/dmOaqWjnPIn2m+HhVPCMWGZVQ==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/tsconfig-utils": "^8.62.1", - "@typescript-eslint/types": "^8.62.1", + "@typescript-eslint/tsconfig-utils": "^8.63.0", + "@typescript-eslint/types": "^8.63.0", "debug": "^4.4.3" }, "engines": { @@ -3992,14 +3992,14 @@ } }, "node_modules/@typescript-eslint/scope-manager": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.62.1.tgz", - "integrity": "sha512-r4d249KbQ1SFdpeStvob8Ih6aPPIzfqllPVOtvhve6ZcpuVcYo5/7zUWckKpHE7StASX4kTKZTLf0WQm/wPkcg==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.63.0.tgz", + "integrity": "sha512-uUyfMWCnDSN8bCpcrY8nGP2BLkQ9Xn0GsipcONcpIDWhwhO4ZSyHvyS14U3X75mzxWxL3I2UZIrenTzdzcJO8A==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1" + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -4010,9 +4010,9 @@ } }, "node_modules/@typescript-eslint/tsconfig-utils": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.62.1.tgz", - "integrity": "sha512-xadytJqX9vJVQ2fdQjkcIVigwaOJNWkpjdLt6cEQ+xPnrI1fkp+/jZE/I97k9KUjqtpd25i0HeyZf3T6dutv2g==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.63.0.tgz", + "integrity": "sha512-sUAbkulqBAsncKnbRP3+7CtQFRKicexnj7ZwNC6ddCR7EmrXvjvdCYMJbUIqMd6lwoEriZjwLo08aS5tSjVMHg==", "dev": true, "license": "MIT", "engines": { @@ -4027,15 +4027,15 @@ } }, "node_modules/@typescript-eslint/type-utils": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-8.62.1.tgz", - "integrity": "sha512-aXM5xlqXiTxPibXB93cLAURfT3rlizf7uMXISCXy66Isr/9hISJx3yDsKl0L7lKa51b8JpFuNKby0/O0pEm9jg==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-8.63.0.tgz", + "integrity": "sha512-Nzzh/OGxVCOjObjaj1CQF2RUasyYy2Jfuh+zZ3PjLzG2fYRriAiZLib9UKtO+CpQAS3YHiAS+ckZDclwqI1TPA==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/typescript-estree": "8.62.1", - "@typescript-eslint/utils": "8.62.1", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/typescript-estree": "8.63.0", + "@typescript-eslint/utils": "8.63.0", "debug": "^4.4.3", "ts-api-utils": "^2.5.0" }, @@ -4052,9 +4052,9 @@ } }, "node_modules/@typescript-eslint/types": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.62.1.tgz", - "integrity": "sha512-ooCzJFaf+Hg+uG6fA3NRFGuFjlfNlDhBthbv4ZPU/0elCAFUfnyXUvf/WOpHz/jYwSmvU2GkR2LtyUfy1AxZ1Q==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.63.0.tgz", + "integrity": "sha512-xyLtl9DUBBFrcJS4x2pIqGLH68/tC2uOa4Z7pUteW09D3bXnnXUom4dyPikzWgB7llmIc1zoeI3aoUdC4rPK/Q==", "dev": true, "license": "MIT", "engines": { @@ -4066,16 +4066,16 @@ } }, "node_modules/@typescript-eslint/typescript-estree": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.62.1.tgz", - "integrity": "sha512-xMcW9oP9u7fAMXYs9A65CVmtLQe2r//oXINHfi8HV+oiqhih17sbLdhXr4540YWlgpDKQdY854OL5ZrdCiQsAA==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.63.0.tgz", + "integrity": "sha512-ygBkU+B7ex5UI/gKhaqexWev79uISfIv7XQCRNYO/jmD8rGLPyWLAb3KMRT6nd8Gt9bmUBi9+iX6tBdYfOY81Q==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/project-service": "8.62.1", - "@typescript-eslint/tsconfig-utils": "8.62.1", - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1", + "@typescript-eslint/project-service": "8.63.0", + "@typescript-eslint/tsconfig-utils": "8.63.0", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0", "debug": "^4.4.3", "minimatch": "^10.2.2", "semver": "^7.7.3", @@ -4094,16 +4094,16 @@ } }, "node_modules/@typescript-eslint/utils": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.62.1.tgz", - "integrity": "sha512-sHtbPfuKNZCG+ih8SyjjucqRntSVmp8XgL5u6o9mAhiSn8ds5o/M/XdM0abweme2Tln3szOstOrZ9OXitvPh0g==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.63.0.tgz", + "integrity": "sha512-fUKaeAvrTuQg/Tgt3nliAUSZHJM6DlCcfyEmxCvlX8kieWSStBX+5O5Fnidtc3i2JrH+9c/GL4RY2iasd/GPTA==", "dev": true, "license": "MIT", "dependencies": { "@eslint-community/eslint-utils": "^4.9.1", - "@typescript-eslint/scope-manager": "8.62.1", - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/typescript-estree": "8.62.1" + "@typescript-eslint/scope-manager": "8.63.0", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/typescript-estree": "8.63.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -4118,13 +4118,13 @@ } }, "node_modules/@typescript-eslint/visitor-keys": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.62.1.tgz", - "integrity": "sha512-4g3BLxfdTMy8iZG0MaBkadnlRrCJ74cQiFbyEVMrkwIoqdyaXXQM22cotDvrl4x28wgIZ9rEJRoM+mmhSJpJ1g==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.63.0.tgz", + "integrity": "sha512-UexrHGnGTpbuQHct2ExOc2ZcFbGUS9FOesCxxqdBGcpI1BxYu/LZ6U8Aq6/72XtF/qRBk9nhuGHFJIXXMhPMdw==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.62.1", + "@typescript-eslint/types": "8.63.0", "eslint-visitor-keys": "^5.0.0" }, "engines": { @@ -5488,9 +5488,9 @@ "license": "MIT" }, "node_modules/baseline-browser-mapping": { - "version": "2.10.40", - "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.40.tgz", - "integrity": "sha512-BSSLZ9/Cjjv7Gtj5B68ZzXcXUg8iOf3fme+FCuh8rC/Go+Kmh8cox7M3A8dolou16s64QjLPOSdngh7GxXvkSw==", + "version": "2.10.42", + "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.42.tgz", + "integrity": "sha512-c/jurFrDLyui7o1J86yLkRu4LMsTYcBohveus7/I2Hzdn9KIP2bdJPTue/lR1KH46enoPbD77GKeSYNdyPoD3Q==", "dev": true, "license": "Apache-2.0", "bin": { @@ -5585,9 +5585,9 @@ } }, "node_modules/browserslist": { - "version": "4.28.4", - "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.4.tgz", - "integrity": "sha512-MTc8i/x9jBQd1iMw2CFGS+rwMa07eYjLR0CCTLDACl9xhxy+nIs3KeML/biicXtk9JrZ6dnnTatmc7ErPXIxqw==", + "version": "4.28.5", + "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.5.tgz", + "integrity": "sha512-Cu2E6QejHWzuDMTkuwgpABFgDfZrXLQq5V13YOACZx4mFAG4IwGTbTfHPMr4WtxlHoXSM8FIuRwYYCz5XiabaQ==", "dev": true, "funding": [ { @@ -5605,10 +5605,10 @@ ], "license": "MIT", "dependencies": { - "baseline-browser-mapping": "^2.10.38", - "caniuse-lite": "^1.0.30001799", - "electron-to-chromium": "^1.5.376", - "node-releases": "^2.0.48", + "baseline-browser-mapping": "^2.10.42", + "caniuse-lite": "^1.0.30001800", + "electron-to-chromium": "^1.5.387", + "node-releases": "^2.0.50", "update-browserslist-db": "^1.2.3" }, "bin": { @@ -5804,9 +5804,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001800", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001800.tgz", - "integrity": "sha512-MMHtuAz9Ys840zAY5F4k6fV5GaivZ9sPk+nz0mY+GYVzRBnYkN0mpqkSR92oWRQ19yQWo4HvBV/FnC16AJX8MA==", + "version": "1.0.30001803", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001803.tgz", + "integrity": "sha512-g/uHREV2ZpK9qMalCsWaxmA6ol+DX8GYhuf3T40RKoP+oL7vhRJh8LNt73PCjpnR6l14FzfPrB5Yux4PKm2meg==", "dev": true, "funding": [ { @@ -6970,9 +6970,9 @@ "license": "MIT" }, "node_modules/electron-to-chromium": { - "version": "1.5.382", - "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.382.tgz", - "integrity": "sha512-8ETaWbV6SZOrno+G93Ffd9ENsMtetqdnqj4nlfxFW90Sm5GgnuV28Kf62hqQVD6VUgzm7qFQKsTsAPmeUiU3Ug==", + "version": "1.5.388", + "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.388.tgz", + "integrity": "sha512-Pl/aJaqOOxYxda3vcx1IKSJimwYXHDkEnGn0F+kG2EE68dDtx2uCinaS+Vih8Z91B9t8CSAbiF/HKyWcnXjhzw==", "dev": true, "license": "ISC" }, @@ -7227,9 +7227,9 @@ } }, "node_modules/es-module-lexer": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-2.2.0.tgz", - "integrity": "sha512-3lGxdTXCLfe1MYfTz1y2ksAAUM4NAOP6rPEjxGJVKO7TZ5+tvHCaQWGpC4Y3IXvW3ece0Cz1cIP4FWBxOnGCTQ==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-2.3.0.tgz", + "integrity": "sha512-KLdwQm2NvGLDkQDCGvmiQrhkd0JbMzXthwQAUgWjQuQdBLFa3eiBP5arXZyA+f8x+x7OXgud6bq2rxjGtHV2tw==", "dev": true, "license": "MIT" }, @@ -7506,9 +7506,9 @@ } }, "node_modules/eslint-module-utils": { - "version": "2.13.0", - "resolved": "https://registry.npmjs.org/eslint-module-utils/-/eslint-module-utils-2.13.0.tgz", - "integrity": "sha512-bLohSkT6469rRs8czj0tLTD8vaeIS/whvPRJVjDr7IuoTT1k5DYDERlNycjDj/HkOlvQdYurmfZ/g3fG5bgeLQ==", + "version": "2.14.0", + "resolved": "https://registry.npmjs.org/eslint-module-utils/-/eslint-module-utils-2.14.0.tgz", + "integrity": "sha512-W2WCRZ9Dqntd+2u8jJcVMV2PKulc6RdLgUUoh/yQr3uB6lo/ZOeGx11sv60/8S4QFFKNslAlWhr9u0Ef7ZW6Ig==", "dev": true, "license": "MIT", "dependencies": { @@ -8394,9 +8394,9 @@ "license": "BSD-3-Clause" }, "node_modules/fast-xml-builder": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.0.tgz", - "integrity": "sha512-00aAWieqff+ZJhsXA4g1g7M8k+7AYoMUUHF+/zFb5U6Uv/P0Vl4QZo84/IcufzYalLuEj9928bXN9PbbFzMF0Q==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.1.tgz", + "integrity": "sha512-tPb5TTWfgfVx5BNSi2xV0eLr89POeXXn0dXIsCJ9m1narrWxeIyx6je9d7Rce/3NyXLbvuQmLkxq+RuxMWejvw==", "funding": [ { "type": "github", @@ -9237,9 +9237,9 @@ } }, "node_modules/hono": { - "version": "4.12.27", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.27.tgz", - "integrity": "sha512-1yrb/+w6HWQJrUCLkJ2IF5jNIPvvFkblV5RNOYl6bV+OA6p9GLcMpHFFGTosSvHvcAUibuUukRqhlYI4z32C7Q==", + "version": "4.12.28", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.28.tgz", + "integrity": "sha512-YwUvVpSF7m1yOblFPrU3Hbo8XhPheBoiyfGuII6z19LnOr6JpDnyyp7LFNrfV56wS8tpvtBFGRISHN02pDdLOA==", "dev": true, "license": "MIT", "engines": { @@ -9346,9 +9346,9 @@ } }, "node_modules/iconv-lite": { - "version": "0.7.2", - "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.2.tgz", - "integrity": "sha512-im9DjEDQ55s9fL4EYzOAv0yMqmMBSZp6G0VvFyTMPKWxiSBHUj9NW/qqLmXUwXrrM7AvqSlTCfvqRb0cM8yYqw==", + "version": "0.7.3", + "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.3.tgz", + "integrity": "sha512-IKXpvIzjnC9XTAUbVBcMfGS0EPaIXtW6v+zr+RRp+hqULEpo0owZax6wyRwPOJbWbzjYspQwusTsfVr0ifh4uQ==", "dev": true, "license": "MIT", "dependencies": { @@ -9407,9 +9407,9 @@ } }, "node_modules/immer": { - "version": "11.1.8", - "resolved": "https://registry.npmjs.org/immer/-/immer-11.1.8.tgz", - "integrity": "sha512-/tbkHMW7y10Lx6i1crLjD4/OhNkRG+Fo7byZHtah0547nIeXYcpIXaUh0IAQY6gO5459qpGGYapcEOHtFXkIuA==", + "version": "11.1.11", + "resolved": "https://registry.npmjs.org/immer/-/immer-11.1.11.tgz", + "integrity": "sha512-qzXuyXAkPySAGYkfsAwodDPWT8Zm7/Uo5BNt4BjhMhG5WlWyZZ4wQqnWwdS8kjlQ1Cwu6gjw3A6+0gTQwlyYtw==", "license": "MIT", "funding": { "type": "opencollective", @@ -12075,9 +12075,9 @@ } }, "node_modules/lucide-react": { - "version": "1.22.0", - "resolved": "https://registry.npmjs.org/lucide-react/-/lucide-react-1.22.0.tgz", - "integrity": "sha512-c9o3l0PiNcgOQDW4F31BEYHudE7kgxVt3o30qMl36ZPwTxXlGB4QnLilhERvVM4uh/pl5MDyY1/gzZSYcHDtBg==", + "version": "1.23.0", + "resolved": "https://registry.npmjs.org/lucide-react/-/lucide-react-1.23.0.tgz", + "integrity": "sha512-38BpJcD0JhFosxHApP/BYsBetLpQFRoTRzEzstM/XCc3jsAG7wqaY1lgVwxiUe3xqYE+lNxo2PkCmYwXWrwwIw==", "dev": true, "license": "ISC", "peerDependencies": { @@ -13630,9 +13630,9 @@ "license": "ISC" }, "node_modules/picomatch": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", - "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", + "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", "dev": true, "license": "MIT", "engines": { @@ -15312,9 +15312,9 @@ "license": "ISC" }, "node_modules/shadcn": { - "version": "4.12.0", - "resolved": "https://registry.npmjs.org/shadcn/-/shadcn-4.12.0.tgz", - "integrity": "sha512-o781ieQziCnXH2FKsEqxp1fnbHdbgAPO9inTSPeZ59hQfsZXuMGp3ul8oFSV5KQS4nbUK9b+DrDE6C7OvfKKQQ==", + "version": "4.13.0", + "resolved": "https://registry.npmjs.org/shadcn/-/shadcn-4.13.0.tgz", + "integrity": "sha512-5fuJ4jI/GcPeA/iTL4cJivCZuYQGXz/N3bIzyd+Gd/FM6xUCy2MxGG+LaDQuw2cjNy9zGPSFPTEmI048UwPTZA==", "dev": true, "license": "MIT", "dependencies": { @@ -16454,9 +16454,9 @@ } }, "node_modules/systeminformation": { - "version": "5.31.11", - "resolved": "https://registry.npmjs.org/systeminformation/-/systeminformation-5.31.11.tgz", - "integrity": "sha512-I6O7iaUj23AXRgCPDDnvi3xHvdOLp4+1YMbF+X194lJwY1NeWojgHJPhslVKcmTtrLTguRk3QJK+xEdTiI3P0w==", + "version": "5.31.14", + "resolved": "https://registry.npmjs.org/systeminformation/-/systeminformation-5.31.14.tgz", + "integrity": "sha512-nefRpMCsAI4m71/6JHH//KPaP/d5nTuRVxEtQ7N7SlBrX18DAcC+5Z1JKZYeN9Iw49qMx95BTo/gBMk3Y2H6+g==", "dev": true, "license": "MIT", "os": [ @@ -17475,9 +17475,9 @@ } }, "node_modules/webpack": { - "version": "5.108.3", - "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.108.3.tgz", - "integrity": "sha512-hOpaCHmQVVY66IVTjofnH14IgSdmod2aquSGHGuYig/OIdWge01Hk2Wt988DZcwXumFUT4+FvJY5N+ikl8o/ww==", + "version": "5.108.4", + "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.108.4.tgz", + "integrity": "sha512-yur8LyJoeiWh47dErD+Ok7vlbmDsJ3UbbRPAoxbGJ54WpE2y5yVo5G/inUzujnYgw3tPmBRdn+G7PoxXaYC33w==", "dev": true, "license": "MIT", "dependencies": { @@ -17607,9 +17607,9 @@ } }, "node_modules/webpack-sources": { - "version": "3.5.0", - "resolved": "https://registry.npmjs.org/webpack-sources/-/webpack-sources-3.5.0.tgz", - "integrity": "sha512-HPuy+uuoTCaaoEoI1LQ3JN9+vrPBvEesnnX1jADHy728cHSMlq4wUc4afYqahq2B1mhQVZxCXOkNTnXltr+2vQ==", + "version": "3.5.1", + "resolved": "https://registry.npmjs.org/webpack-sources/-/webpack-sources-3.5.1.tgz", + "integrity": "sha512-jyuiGJdtvY434z5bUZrjz67v76/ePNvFZTp9Mdz29IlH4+GPsgyGjiv0fKI+M7BdkU6ADjulUcKAd3tUK3WlEw==", "dev": true, "license": "MIT", "engines": { @@ -17617,9 +17617,9 @@ } }, "node_modules/webpack/node_modules/enhanced-resolve": { - "version": "5.24.1", - "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.24.1.tgz", - "integrity": "sha512-7DdUaTjmNwMcH2gLr1qycesKII3BK4RLy/mdAb7x10Lq7bR4aNKHt1BR1ZALSv0rPM/hF5wYF0PhGop/rJm8vw==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.24.2.tgz", + "integrity": "sha512-rpsZEGT1jFuve6QlpyRp9ckQ+kN61hvF9BzCPyMdaKTm8UJce96KBn3sorXOFXlzjPrs3Vc4T1NsSroZ3PxlFw==", "dev": true, "license": "MIT", "dependencies": { @@ -18119,9 +18119,9 @@ } }, "node_modules/yocto-spinner": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/yocto-spinner/-/yocto-spinner-1.2.0.tgz", - "integrity": "sha512-Yw0hUB6UA3o4YUgKy3oSe9a4cxoaZ9sBfYDw+JSxo6Id0KoJGoxzPA24qqUXYKBWABs/zDSGTz9kww7t3F0XGw==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/yocto-spinner/-/yocto-spinner-1.2.1.tgz", + "integrity": "sha512-9cbFWLhbiZp+820O4pkHGNncI7+MrUGzBOjw8NMG+ewsY+aG0DdEXnr19Smxao32YOjLZRMdn1UtaxcrXOYOIg==", "dev": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 96373c7c..ee1f3753 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "bump-versions": "ts-node ./lib/bump-versions.ts", "test": "jest", "test:coverage": "jest --coverage", - "test:e2e": "playwright test --config e2e-tests/playwright.config.ts", + "test:e2e": "npm run test:e2e:smoke && npm run test:e2e:cdp", "test:e2e:cdp": "playwright test --config e2e-tests/playwright-cdp.config.ts", "test:e2e:smoke": "playwright test --config e2e-tests/playwright.config.ts --project=smoke", "core:start": "npm --prefix ../paranext-core start", diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 6b28da37..3f8e2838 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -202,14 +202,18 @@ jest.mock('../../components/modals/ProjectModals', () => ({ /** * Minimal ProjectModals stand-in that drives modal state and active-project state through the * same `useWebViewState` hook the real component uses, so tests can assert on state transitions - * without mounting the full modal tree. Accepts (and ignores) the draft-related props the loader - * now passes (`dirty`, `getDraftSnapshot`, `loadFromProject`, `markSynced`). + * without mounting the full modal tree. Accepts (and mostly ignores) the draft-related props the + * loader now passes (`hasUnsavedWork`, `getDraftSnapshot`, `loadFromProject`, `markSynced`); + * `hasUnsavedWork` is surfaced as a `data-*` attribute so tests can assert the loader feeds it + * the combined committed-and-pending unsaved state. * * @param modal - Current modal identifier controlling which stub panel is rendered. * @param setModal - Callback to transition to a different modal state. * @param activeProject - The currently active interlinear project, or undefined when none is * selected. * @param defaultAnalysisLanguage - BCP 47 tag forwarded as the create modal's default language. + * @param hasUnsavedWork - Whether the draft has committed-but-unsaved changes or uncommitted + * in-progress typing; gates the discard confirmation in the real component. * @param useWebViewState - Injected hook used to read and write persisted WebView state; must * support the `'activeProject'` key. * @returns A JSX element containing the stub modal panels keyed by `modal`. @@ -219,13 +223,14 @@ jest.mock('../../components/modals/ProjectModals', () => ({ setModal, activeProject, defaultAnalysisLanguage, + hasUnsavedWork, useWebViewState, }: { modal: string; setModal: (m: string) => void; activeProject: MockProject | undefined; defaultAnalysisLanguage?: string; - dirty: boolean; + hasUnsavedWork: boolean; getDraftSnapshot: () => DraftProject | undefined; loadFromProject: (project: unknown) => void; markSynced: () => void; @@ -241,6 +246,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({ data-testid="project-modals" data-modal={modal} data-default-lang={defaultAnalysisLanguage} + data-has-unsaved-work={hasUnsavedWork} data-active-project-name={activeProject?.name} > {modal === 'select' && ( @@ -1196,6 +1202,46 @@ describe('InterlinearizerLoader', () => { expect(updateWebViewDefinition).toHaveBeenCalledWith({ title: 'Interlinearizer' }); }); + it('reports in-progress typing to ProjectModals as unsaved work so a swap is guarded', async () => { + await act(async () => { + renderLoader(); + }); + + // The persisted draft is clean, so the modal starts with no unsaved work to guard. + expect(screen.getByTestId('project-modals')).toHaveAttribute( + 'data-has-unsaved-work', + 'false', + ); + + // A gloss input begins holding uncommitted text. Even though nothing has committed (the draft + // stays clean), ProjectModals must now treat the draft as having unsaved work so opening or + // creating a project prompts before discarding the in-progress gloss. + act(() => { + capturedInterlinearizerProps?.onPendingEditsChange?.(true); + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-has-unsaved-work', 'true'); + }); + + it('drops the unsaved-work guard once in-progress typing is abandoned', async () => { + await act(async () => { + renderLoader(); + }); + + act(() => { + capturedInterlinearizerProps?.onPendingEditsChange?.(true); + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-has-unsaved-work', 'true'); + + // The edit is reverted or the input unmounts with nothing committed: the guard clears. + act(() => { + capturedInterlinearizerProps?.onPendingEditsChange?.(false); + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute( + 'data-has-unsaved-work', + 'false', + ); + }); + it('logs an error when the saveAnalysis command rejects during Save', async () => { await act(async () => renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_ACTIVE_PROJECT }) }), diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index 2d8e60b0..e28df9e3 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -243,7 +243,7 @@ jest.mock('../../../components/modals/ProjectMetadataModal', () => ({ type ModalsOverrides = Partial<{ activeProject: InterlinearProjectSummary | undefined; defaultAnalysisLanguage: string; - dirty: boolean; + hasUnsavedWork: boolean; getDraftSnapshot: () => DraftProject | undefined; loadFromProject: jest.Mock; newDraft: jest.Mock; @@ -264,7 +264,7 @@ function buildProps(overrides: ModalsOverrides = {}) { return { activeProject: overrides.activeProject, defaultAnalysisLanguage: overrides.defaultAnalysisLanguage, - dirty: overrides.dirty ?? false, + hasUnsavedWork: overrides.hasUnsavedWork ?? false, getDraftSnapshot: overrides.getDraftSnapshot ?? (() => MOCK_DRAFT), loadFromProject: overrides.loadFromProject ?? jest.fn(), newDraft: overrides.newDraft ?? jest.fn(), @@ -584,7 +584,11 @@ describe('ProjectModals', () => { .mocked(papi.commands.sendCommand) .mockResolvedValueOnce(JSON.stringify(MOCK_FULL_PROJECT)); const loadFromProject = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByTestId('select-select')); // The discard confirm overlays the still-mounted select modal (so confirming Open does not @@ -599,7 +603,11 @@ describe('ProjectModals', () => { it('cancels the discard confirm and returns to the select modal', async () => { const loadFromProject = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByTestId('select-select')); await userEvent.click(screen.getByTestId('discard-cancel')); @@ -611,7 +619,9 @@ describe('ProjectModals', () => { it('confirms before creating a project when the draft is dirty', async () => { const newDraft = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByTestId('create-submit')); expect(screen.getByTestId('discard-modal')).toBeInTheDocument(); @@ -652,7 +662,7 @@ describe('ProjectModals', () => { { resolveGet = resolve; }), ); - render(); + render(); await userEvent.click(screen.getByTestId('select-select')); expect(screen.getByTestId('discard-confirm')).toBeEnabled(); diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index a1b78792..5470a9e7 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -521,7 +521,7 @@ function InterlinearizerLoaderInner({ DraftProject | undefined; loadFromProject: (project: OpenableProject) => void; newDraft: (config: NewDraftConfig) => void; @@ -215,10 +217,10 @@ export default function ProjectModals({ * persisted immediately so it shows up in "Select Interlinear Project" right away. * * `newDraft` is called synchronously before the backend round-trip so the editor is ready - * immediately regardless of whether persistence succeeds. This is safe: when dirty is `false` (no - * discard confirmation shown), any data in the draft is either already committed to the active - * project or the draft was empty — nothing is lost. When dirty is `true` the - * {@link DiscardDraftConfirm} dialog has already obtained explicit user consent to discard. + * immediately regardless of whether persistence succeeds. This is safe: when `hasUnsavedWork` is + * `false` (no discard confirmation shown), any data in the draft is either already committed to + * the active project or the draft was empty — nothing is lost. When `hasUnsavedWork` is `true` + * the {@link DiscardDraftConfirm} dialog has already obtained explicit user consent to discard. * * The `interlinearizer.createProject` command sends its own error notification before rethrowing, * so the catch block only needs to log — callers do not need to send a second notification. This @@ -269,16 +271,16 @@ export default function ProjectModals({ /** * Called when the user selects a project in the select modal. Opens it immediately, or defers - * behind the unsaved-changes confirmation when the draft is dirty. + * behind the unsaved-changes confirmation when the draft has unsaved work. * * @param project - The project the user selected. */ const handleSelectProject = useCallback( (project: InterlinearProjectSummary) => { - if (dirty) setPendingReplace({ kind: 'open', project }); + if (hasUnsavedWork) setPendingReplace({ kind: 'open', project }); else openProject(project); }, - [dirty, openProject], + [hasUnsavedWork, openProject], ); /** @@ -306,19 +308,19 @@ export default function ProjectModals({ /** * Called when the New dialog is submitted. Creates and persists the project immediately, or - * defers behind the unsaved-changes confirmation when the draft is dirty. + * defers behind the unsaved-changes confirmation when the draft has unsaved work. * * @param config - The configuration collected by the New dialog. */ const handleCreateDraft = useCallback( async (config: CreateDraftConfig) => { - if (dirty) { + if (hasUnsavedWork) { setPendingReplace({ kind: 'new', config }); return; } await createDraftAndClose(config); }, - [createDraftAndClose, dirty], + [createDraftAndClose, hasUnsavedWork], ); /** diff --git a/user-questions.md b/user-questions.md index 0b9ee51d..46b34e55 100644 --- a/user-questions.md +++ b/user-questions.md @@ -87,6 +87,17 @@ Decisions made during development that we'd like reviewed: two separate menu items (each a single click, no scope step). Current choice: one menu item plus a scope-picker dialog. +11. **In-progress typing guards a project swap.** Switching projects (New / Open) shows the discard + confirm (item 2) whenever the draft has unsaved changes. This now also fires when a gloss field + holds **uncommitted** text — the same eager signal that lights the `●` marker (item 7) — not only + after that text has committed on blur. Previously the guard checked committed changes only, so + opening or creating a project while mid-typing a gloss discarded that in-progress text silently. + The two definitions of "unsaved" (the tab marker vs. the discard guard) are now unified, so the + prompt and the indicator always agree. Is prompting on uncommitted typing the right behavior, or + should a project swap only guard against changes that have actually committed (accepting that + mid-typing text is then lost without warning)? Note this is coupled to item 7: if the marker is + changed to wait until an edit commits, this guard should follow. + ## Suggestion engine: editing a shared analysis, and per-instance analyses The suggestion engine reuses an existing analysis on other matching surface forms by creating a new