From 3ca95f16dfbbe7fae95480e2c2036ca83613da17 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Wed, 2 Sep 2026 12:22:41 +0100 Subject: [PATCH] feat(editor): guard unsaved edits and rework the editor toolbar --- .../public/locales/en-US/translation.toml | 105 +--- .../pdf-text-editor-edge-gestures.spec.ts | 14 +- .../stubbed/pdf-text-editor-empty-run.spec.ts | 203 ++++++ .../stubbed/pdf-text-editor-find-bar.spec.ts | 150 +++++ .../pdf-text-editor-menu-layout.spec.ts | 11 +- .../pdf-text-editor-signed-save.spec.ts | 134 ++++ .../stubbed/pdf-text-editor-topbar.spec.ts | 206 +++++++ .../pdf-text-editor-unsaved-guard.spec.ts | 176 ++++++ .../pdf-text-editor-workbench-files.spec.ts | 42 +- .../tests/stubbed/pdf-text-editor.spec.ts | 8 +- .../tools/pdfTextEditor/PdfTextEditor.tsx | 177 ++++-- .../__tests__/commandRollback.test.ts | 174 ++++++ .../__tests__/pdfiumSaveGuards.test.ts | 113 ++++ .../__tests__/savedBytes.test.ts | 80 +++ .../__tests__/storeApplyRecovery.test.ts | 107 ++++ .../tools/pdfTextEditor/commands/Command.ts | 18 + .../commands/CompositeCommand.ts | 46 +- .../pdfTextEditor/commands/EditTextCommand.ts | 17 + .../components/DiscardChangesModal.tsx | 69 +++ .../components/EditorFileSwitcher.tsx | 139 +++-- .../components/EditorPanelActions.tsx | 148 +++++ .../components/EditorSaveBar.tsx | 113 ---- .../components/EditorSidebar.tsx | 93 +-- .../pdfTextEditor/components/EditorTopBar.css | 87 +++ .../pdfTextEditor/components/EditorTopBar.tsx | 321 ++++++++++ .../pdfTextEditor/components/FindBar.css | 100 +++ .../pdfTextEditor/components/FindBar.tsx | 347 ++++++----- .../pdfTextEditor/components/HelpOverlay.css | 44 ++ .../pdfTextEditor/components/HelpOverlay.tsx | 427 ++++++------- .../pdfTextEditor/components/PageStage.tsx | 50 +- .../pdfTextEditor/components/Toolbar.tsx | 578 ------------------ .../components/toolbar/FormatGroup.tsx | 277 +++++++++ .../components/toolbar/ObjectGroup.tsx | 226 +++++++ .../components/toolbar/toolbarShared.tsx | 22 + .../pdfTextEditor/hooks/useElementWidth.ts | 35 ++ .../tools/pdfTextEditor/pdfium/PdfiumSave.ts | 35 +- .../pdfTextEditor/pdfium/PdfiumTextWriter.ts | 10 + .../pdfTextEditor/store/EditorSession.ts | 59 ++ .../tools/pdfTextEditor/store/EditorStore.ts | 58 +- .../tools/pdfTextEditor/store/HistoryStack.ts | 22 +- .../tools/pdfTextEditor/util/exportPdf.ts | 12 +- .../tools/pdfTextEditor/util/savedBytes.ts | 97 +++ frontend/editor/src/core/ui/KeyCombo.css | 53 ++ .../editor/src/core/ui/KeyCombo.stories.tsx | 68 +++ frontend/editor/src/core/ui/KeyCombo.test.tsx | 70 +++ frontend/editor/src/core/ui/KeyCombo.tsx | 85 +++ frontend/editor/src/core/ui/index.ts | 1 + 47 files changed, 4033 insertions(+), 1394 deletions(-) create mode 100644 frontend/editor/src/core/tests/stubbed/pdf-text-editor-empty-run.spec.ts create mode 100644 frontend/editor/src/core/tests/stubbed/pdf-text-editor-find-bar.spec.ts create mode 100644 frontend/editor/src/core/tests/stubbed/pdf-text-editor-signed-save.spec.ts create mode 100644 frontend/editor/src/core/tests/stubbed/pdf-text-editor-topbar.spec.ts create mode 100644 frontend/editor/src/core/tests/stubbed/pdf-text-editor-unsaved-guard.spec.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/__tests__/commandRollback.test.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/__tests__/pdfiumSaveGuards.test.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/__tests__/savedBytes.test.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/__tests__/storeApplyRecovery.test.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/DiscardChangesModal.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/EditorPanelActions.tsx delete mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/EditorSaveBar.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.css create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.css create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.css delete mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/Toolbar.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/FormatGroup.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/ObjectGroup.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/toolbarShared.tsx create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/hooks/useElementWidth.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/store/EditorSession.ts create mode 100644 frontend/editor/src/core/tools/pdfTextEditor/util/savedBytes.ts create mode 100644 frontend/editor/src/core/ui/KeyCombo.css create mode 100644 frontend/editor/src/core/ui/KeyCombo.stories.tsx create mode 100644 frontend/editor/src/core/ui/KeyCombo.test.tsx create mode 100644 frontend/editor/src/core/ui/KeyCombo.tsx diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 681bca4dd8..003bdca96c 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -6341,7 +6341,6 @@ REVERSE_ORDER = "Flip the document so the last page becomes first and so on." SIDE_STITCH_BOOKLET_SORT = "Arrange pages for side‑stitch booklet printing (optimized for binding on the side)." [pdfTextEditor] -confirmReplaceDirty = "You have unsaved changes. Replace the open document and discard them?" download = "Download" downloadTooltip = "Save and download the edited PDF" save = "Save PDF" @@ -6356,6 +6355,13 @@ freetext = "Annotation text - not page text, so it can't be edited here" stamp = "Stamp annotation - not page text, so it can't be edited here" widget = "Form field - not page text, so it can't be edited here" +[pdfTextEditor.discard] +body = "Opening {{name}} closes the document you are editing. Your unsaved changes will be lost." +cancel = "Cancel" +confirm = "Discard and open" +hint = "Cancel, then Save PDF to keep them in your workspace first." +title = "Discard unsaved changes?" + [pdfTextEditor.drop] hint = "Releases on the editor stage replace any open document." title = "Drop a PDF to open" @@ -6375,10 +6381,8 @@ noMatches = "No matches" previous = "Previous match" replace = "Replace" replaceAll = "Replace all" -replaced = " · {{count}} replaced" +replaced = "{{count}} replaced" replacePlaceholder = "Replace with" -title = "Find & replace" -typeToSearch = "Type to search" wholeWord = "Whole word" [pdfTextEditor.fontPicker] @@ -6422,29 +6426,16 @@ ariaLabel = "Keyboard shortcuts" title = "Keyboard shortcuts" tooltip = "Keyboard shortcuts (?)" -[pdfTextEditor.help.arrangement] -alignDesc = "Align edges L / centre / R / T / mid / B" -alignKey = "Toolbar align" -distributeDesc = "Equal horizontal / vertical spacing (3+)" -distributeKey = "Toolbar distribute" -frontBackDesc = "Bring to front / send to back" -frontBackKey = "Toolbar front/back" -heading = "Object arrangement" -lockDesc = "Lock / unlock selection (session-only)" -lockKey = "Lock button" -orderDesc = "Bring forward / send backward (one step)" -orderKey = "Toolbar ↑ ↓" - [pdfTextEditor.help.clipboard] copyDesc = "Copy selected text" -copyKey = "Ctrl+C" +copyKey = "Ctrl + C" cutDesc = "Cut selected (copy + delete)" -cutKey = "Ctrl+X" +cutKey = "Ctrl + X" heading = "Clipboard" pasteDesc = "Paste clipboard text as new run" -pasteKey = "Ctrl+V" +pasteKey = "Ctrl + V" pastePlainDesc = "Paste as plain text" -pastePlainKey = "Ctrl+Shift+V" +pastePlainKey = "Ctrl + Shift + V" [pdfTextEditor.help.document] escDesc = "Clear selection / close find / close help" @@ -6453,76 +6444,42 @@ heading = "Document" helpDesc = "This help" helpKey = "? / F1" saveDesc = "Save to your workspace" -saveKey = "Ctrl+S" +saveKey = "Ctrl + S" [pdfTextEditor.help.editing] -clickDesc = "Edit text" -clickKey = "Click" deleteDesc = "Remove selected" deleteKey = "Delete" duplicateDesc = "Duplicate selected" -duplicateKey = "Ctrl+D" -groupDesc = "Group selected runs (Group button)" -groupKey = "Ctrl+M" +duplicateKey = "Ctrl + D" +groupDesc = "Group selected runs into one paragraph" +groupKey = "Ctrl + M" heading = "Editing" marqueeDesc = "Marquee multi-select" -marqueeKey = "Ctrl+Shift+drag" +marqueeKey = "Ctrl + Shift + Drag" moveDesc = "Move text run" -moveKey = "Ctrl+Click + drag" +moveKey = "Ctrl + Click + Drag" selectAllDesc = "Select all" -selectAllKey = "Ctrl+A" +selectAllKey = "Ctrl + A" shiftClickDesc = "Add / remove a run from selection" -shiftClickKey = "Ctrl+Click / Shift+Click" +shiftClickKey = "Ctrl + Click / Shift + Click" undoRedoDesc = "Undo / Redo" -undoRedoKey = "Ctrl+Z / Ctrl+Y" -ungroupDesc = "Ungroup paragraph: select it, click Ungroup" -ungroupKey = "-" +undoRedoKey = "Ctrl + Z / Ctrl + Y" [pdfTextEditor.help.find] -enterFindDesc = "Next match" -enterFindKey = "Enter (in find)" -enterReplaceDesc = "Replace one (Shift = Replace All)" -enterReplaceKey = "Enter (in replace)" -heading = "Find & Replace" -nextDesc = "Next match (Shift = previous)" -nextKey = "F3 / Ctrl+G" +heading = "Find & replace" +nextDesc = "Next match (add Shift for previous)" +nextKey = "F3 / Ctrl + G" openDesc = "Open find bar (and replace)" -openKey = "Ctrl+F" - -[pdfTextEditor.help.formatting] -caseDesc = "Change case (upper/lower/title/sentence)" -caseKey = "Toolbar case (Aa)" -colourDesc = "Change fill colour" -colourKey = "Toolbar colour" -fontFamilyDesc = "Swap to base-14 font" -fontFamilyKey = "Toolbar font family" -fontSizeDesc = "Change font size" -fontSizeKey = "Toolbar font size" -heading = "Text formatting" -italicDesc = "Italic" -italicKey = "Toolbar I" - -[pdfTextEditor.help.image] -flipDesc = "Flip horizontally or vertically" -flipKey = "Toolbar flip" -heading = "Image" -moveDesc = "Move image" -moveKey = "Drag" -resizeDesc = "Resize image" -resizeKey = "Corner drag" -rotateDesc = "Rotate 90° clockwise or counter-clockwise" -rotateKey = "Toolbar rotate" +openKey = "Ctrl + F" [pdfTextEditor.help.navigation] firstLastDesc = "First / last page" -firstLastKey = "Ctrl+Home / Ctrl+End" +firstLastKey = "Ctrl + Home / Ctrl + End" heading = "Navigation" -pageDesc = "Next / previous page" -pageKey = "PageDown / PageUp" -toolbarZoomDesc = "Manual zoom + Fit to width" -toolbarZoomKey = "Toolbar zoom" +pageDesc = "Previous / next page" +pageKey = "PageUp / PageDown" zoomDesc = "Zoom in / out" -zoomKey = "Ctrl+Wheel" +zoomKey = "Ctrl + Wheel" [pdfTextEditor.inspector] document = "Document" @@ -6579,6 +6536,7 @@ title = "Saving will change this PDF" [pdfTextEditor.settings] advanced = "Advanced" find = "Find in document" +findTooltip = "Find (Ctrl+F)" view = "View" [pdfTextEditor.sidebar] @@ -6619,6 +6577,7 @@ pickPrompt = "Pick a PDF from the Files panel on the left to begin editing." renderingPreview = "Rendering preview" [pdfTextEditor.toolbar] +addTextTooltip = "Add a text box - then click the page" advancedColour = "Advanced colour" advancedColourTooltip = "Advanced colour (glyph outline)" alignBottom = "Align bottom" @@ -6647,10 +6606,12 @@ flipHorizontal = "Flip horizontal" flipVertical = "Flip vertical" fontColour = "Font colour" fontSize = "Font size" +formatting = "Text formatting" italic = "Italic" italicUnavailable = "This font has no italic version. Load your device fonts or pick another font family." lock = "Lock selection" lockTooltip = "Lock selection - prevents accidental edits" +more = "More actions" order = "Order" outlineColour = "Outline colour" outlineWidth = "Outline width (0 = none)" diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-edge-gestures.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-edge-gestures.spec.ts index 9e457af857..87e24c0acd 100644 --- a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-edge-gestures.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-edge-gestures.spec.ts @@ -155,15 +155,19 @@ test.describe("PDF text editor - edge gestures", () => { expect(await textOf("Open Source")).toBe(before); }); - test("the insert verbs live in the panel, not the canvas strip", async ({ + test("the insert verbs live in the toolbar, not the panel", async ({ page, }) => { await open(page); - const panel = page.locator('[data-sidebar="tool-panel"]'); - await expect(panel.getByTestId("pdf-editor-add-text")).toBeVisible(); - await expect(panel.getByTestId("pdf-editor-add-image")).toBeVisible(); + // Insert is a verb aimed at the page, so it sits above the page - the + // panel is for the properties of whatever is already selected. + const toolbar = page.getByTestId("pdf-editor-toolbar"); + await expect(toolbar.getByTestId("pdf-editor-add-text")).toBeVisible(); + await expect(toolbar.getByTestId("pdf-editor-add-image")).toBeVisible(); await expect( - page.getByTestId("pdf-editor-toolbar").getByTestId("pdf-editor-add-text"), + page + .locator('[data-sidebar="tool-panel"]') + .getByTestId("pdf-editor-add-text"), ).toHaveCount(0); }); }); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-empty-run.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-empty-run.spec.ts new file mode 100644 index 0000000000..2ae0da98af --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-empty-run.spec.ts @@ -0,0 +1,203 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import type { Page } from "@playwright/test"; +import path from "path"; + +/** + * Emptying a text box must actually empty it, in the PDF as well as the model. + * + * `FPDFText_SetText` traps ("unreachable", a raw WASM abort) when handed an + * empty string. An Add-text box is base-14 from the start, so its edits take + * the in-place SetText fast path - and clearing one threw inside PDFium AFTER + * the model had already been set to "". The throw escaped the React handler + * with nothing shown, leaving the model saying empty while the PDFium object + * still held every character. Saving then brought the "deleted" text back. + */ + +const SAMPLE_PDF = path.join( + import.meta.dirname, + "../test-fixtures/sample.pdf", +); +const SECRET = "SHOULDVANISH"; + +async function openEditor(page: Page): Promise { + await page.goto("/pdf-text-editor", { waitUntil: "domcontentloaded" }); + await expect(page.getByTestId("pdf-editor-root")).toBeVisible({ + timeout: 30_000, + }); + await page + .locator('[data-testid="pdf-editor-file-input"]') + .setInputFiles(SAMPLE_PDF); + await expect(page.getByTestId("pdf-editor-page-0")).toBeVisible({ + timeout: 60_000, + }); +} + +/** Drop a new text box on page 0 and return its run id. */ +async function addTextBox(page: Page): Promise { + await page.getByTestId("pdf-editor-add-text").click(); + await page + .getByTestId("pdf-editor-page-0") + .click({ position: { x: 200, y: 600 } }); + await page.waitForTimeout(400); + return page.evaluate(() => { + const runs = ( + window as unknown as { + __editor_store: { + getState: () => { pages: Array<{ runs: Array<{ id: string }> }> }; + }; + } + ).__editor_store.getState().pages[0].runs; + return runs[runs.length - 1].id; + }); +} + +/** Select everything in a run overlay, delete it, and optionally retype. */ +async function clearRun( + page: Page, + runId: string, + retype?: string, +): Promise { + await page.evaluate( + ({ id, text }) => { + const el = document.querySelector( + `[data-testid="pdf-editor-run-${id}"]`, + ); + if (!el) throw new Error(`run ${id} not in DOM`); + el.focus(); + const selection = window.getSelection(); + if (!selection) throw new Error("no Selection api"); + const range = document.createRange(); + range.selectNodeContents(el); + selection.removeAllRanges(); + selection.addRange(range); + document.execCommand("delete", false); + if (text) document.execCommand("insertText", false, text); + }, + { id: runId, text: retype }, + ); + await page.waitForTimeout(500); +} + +async function typeInto(page: Page, runId: string, text: string) { + await page.evaluate( + ({ id, t }) => { + const el = document.querySelector( + `[data-testid="pdf-editor-run-${id}"]`, + ); + if (!el) throw new Error(`run ${id} not in DOM`); + el.focus(); + const selection = window.getSelection(); + if (!selection) throw new Error("no Selection api"); + const range = document.createRange(); + range.selectNodeContents(el); + selection.removeAllRanges(); + selection.addRange(range); + document.execCommand("delete", false); + document.execCommand("insertText", false, t); + }, + { id: runId, t: text }, + ); + await page.waitForTimeout(500); +} + +/** Every run's text on page 0, as the model currently has it. */ +async function pageText(page: Page): Promise { + return page.evaluate(() => + ( + window as unknown as { + __editor_store: { + getState: () => { + pages: Array<{ runs: Array<{ text: string }> }>; + }; + }; + } + ).__editor_store + .getState() + .pages.flatMap((p) => p.runs) + .map((r) => r.text) + .join(" | "), + ); +} + +/** Download the edited PDF and re-open it in the editor. */ +async function saveAndReopen(page: Page): Promise { + const downloadPromise = page.waitForEvent("download"); + await page.getByTestId("pdf-editor-download").click(); + const download = await downloadPromise; + const stream = await download.createReadStream(); + const chunks: Buffer[] = []; + for await (const chunk of stream) chunks.push(chunk as Buffer); + await page.locator('[data-testid="pdf-editor-file-input"]').setInputFiles({ + name: "round.pdf", + mimeType: "application/pdf", + buffer: Buffer.concat(chunks), + }); + await expect( + page.locator('[data-testid^="pdf-editor-run-p0-"]').first(), + ).toBeVisible({ timeout: 30_000 }); + await page.waitForTimeout(600); +} + +test.describe("PDF text editor - emptying a text box", () => { + test("clearing an Add-text box removes the text from the saved PDF", async ({ + page, + }) => { + test.setTimeout(180_000); + await openEditor(page); + const runId = await addTextBox(page); + await typeInto(page, runId, SECRET); + expect(await pageText(page)).toContain(SECRET); + + await clearRun(page, runId); + expect( + await pageText(page), + "the model must show the box as empty", + ).not.toContain(SECRET); + + // The real check: the PDF, not the model. A trap inside the in-place write + // left the object untouched, so the "deleted" text came back on save. + await saveAndReopen(page); + expect( + await pageText(page), + "text deleted before saving must not reappear in the saved PDF", + ).not.toContain(SECRET); + }); + + test("clearing then retyping saves what was typed, not what was cleared", async ({ + page, + }) => { + test.setTimeout(180_000); + await openEditor(page); + const runId = await addTextBox(page); + await typeInto(page, runId, SECRET); + + await clearRun(page, runId, "kept"); + expect(await pageText(page)).toContain("kept"); + + await saveAndReopen(page); + const after = await pageText(page); + expect(after).toContain("kept"); + expect(after).not.toContain(SECRET); + }); + + test("clearing raises no error and stays undoable", async ({ page }) => { + test.setTimeout(180_000); + await openEditor(page); + const runId = await addTextBox(page); + await typeInto(page, runId, SECRET); + // Past the 600ms coalescing window, so the clear is its own undo step + // rather than merging into the typing that preceded it. + await page.waitForTimeout(900); + + await clearRun(page, runId); + // A trap used to reach the store's recovery path, which rebuilds every + // page and throws the whole undo history away. + expect(await page.getByTestId("pdf-editor-error").count()).toBe(0); + + await page.getByTestId("pdf-editor-undo").click(); + await page.waitForTimeout(500); + expect(await pageText(page), "undo must bring the text back").toContain( + SECRET, + ); + }); +}); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-find-bar.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-find-bar.spec.ts new file mode 100644 index 0000000000..8f741b5493 --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-find-bar.spec.ts @@ -0,0 +1,150 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import type { Page } from "@playwright/test"; +import path from "path"; + +/** + * The find bar's layout, the way a code editor builds one. + * + * Two rows fenced off from the page by their own rules, the match count and + * the option toggles inside the search field, and navigation beside it. The + * toggles being inside the field is the part that needs a test: Mantine makes + * an input's section inert by default so clicks fall through to the input, so + * a real button placed there silently stops responding. + */ + +const SAMPLE_PDF = path.join( + import.meta.dirname, + "../test-fixtures/sample.pdf", +); + +async function openFind(page: Page): Promise { + await page.goto("/pdf-text-editor", { waitUntil: "domcontentloaded" }); + await expect(page.getByTestId("pdf-editor-root")).toBeVisible({ + timeout: 30_000, + }); + await page + .locator('[data-testid="pdf-editor-file-input"]') + .setInputFiles(SAMPLE_PDF); + await expect(page.getByTestId("pdf-editor-page-0")).toBeVisible({ + timeout: 60_000, + }); + await page.getByTestId("pdf-editor-open-find").click(); + await expect(page.getByTestId("pdf-editor-find-bar")).toBeVisible(); +} + +test.describe("PDF text editor - find bar", () => { + test("the toolbar's search button opens AND closes the bar", async ({ + page, + }) => { + test.setTimeout(120_000); + await openFind(page); + const search = page.getByTestId("pdf-editor-open-find"); + // It is a toggle, and says so while the bar is up. + await expect(search).toHaveAttribute("aria-pressed", "true"); + + await search.click(); + await expect( + page.getByTestId("pdf-editor-find-bar"), + "re-clicking search must put the find bar away", + ).toBeHidden(); + await expect(search).toHaveAttribute("aria-pressed", "false"); + + await search.click(); + await expect(page.getByTestId("pdf-editor-find-bar")).toBeVisible(); + }); + + test("the close button sits at the bar's far right, clear of the arrows", async ({ + page, + }) => { + test.setTimeout(120_000); + await openFind(page); + const close = await page.getByTestId("pdf-editor-find-close").boundingBox(); + const next = await page.getByTestId("pdf-editor-find-next").boundingBox(); + const bar = await page.getByTestId("pdf-editor-find-bar").boundingBox(); + expect(close).not.toBeNull(); + expect(next).not.toBeNull(); + expect(bar).not.toBeNull(); + // Well clear of the match arrows, and hard against the bar's own edge. + expect(close!.x).toBeGreaterThan(next!.x + next!.width + 40); + expect(bar!.x + bar!.width - (close!.x + close!.width)).toBeLessThan(24); + }); + + test("the option toggles inside the field actually toggle", async ({ + page, + }) => { + test.setTimeout(120_000); + await openFind(page); + + for (const id of [ + "pdf-editor-find-match-case", + "pdf-editor-find-whole-word", + "pdf-editor-find-ignore-accents", + ]) { + const toggle = page.getByTestId(id); + await expect(toggle).toHaveAttribute("aria-pressed", "false"); + await toggle.click(); + await expect( + toggle, + `${id} did not respond to a click - is the input section inert?`, + ).toHaveAttribute("aria-pressed", "true"); + await toggle.click(); + await expect(toggle).toHaveAttribute("aria-pressed", "false"); + } + }); + + test("match case narrows the results it reports", async ({ page }) => { + test.setTimeout(120_000); + await openFind(page); + await page.getByTestId("pdf-editor-find-input").fill("AS"); + + const count = page.getByTestId("pdf-editor-find-count"); + // The fixture's cells are lowercase "as", so a case-blind search finds them. + await expect(count).toContainText(/of/); + await page.getByTestId("pdf-editor-find-match-case").click(); + await expect(count).toContainText(/no matches/i); + }); + + test("the count sits inside the field and navigation beside it", async ({ + page, + }) => { + test.setTimeout(120_000); + await openFind(page); + await page.getByTestId("pdf-editor-find-input").fill("as"); + await expect(page.getByTestId("pdf-editor-find-count")).toContainText(/of/); + + const field = await page.getByTestId("pdf-editor-find-input").boundingBox(); + const count = await page.getByTestId("pdf-editor-find-count").boundingBox(); + const next = await page.getByTestId("pdf-editor-find-next").boundingBox(); + expect(field).not.toBeNull(); + expect(count).not.toBeNull(); + expect(next).not.toBeNull(); + // Count within the field's box; navigation entirely to its right. + expect(count!.x).toBeGreaterThan(field!.x); + expect(count!.x + count!.width).toBeLessThanOrEqual( + field!.x + field!.width + 1, + ); + expect(next!.x).toBeGreaterThan(field!.x + field!.width); + }); + + test("the replace row lines up with the find row", async ({ page }) => { + test.setTimeout(120_000); + await openFind(page); + const find = await page.getByTestId("pdf-editor-find-input").boundingBox(); + const replace = await page + .getByTestId("pdf-editor-replace-input") + .boundingBox(); + expect(find).not.toBeNull(); + expect(replace).not.toBeNull(); + // Both fields end at the same x, so the two rows do not step. + expect( + Math.abs(find!.x + find!.width - (replace!.x + replace!.width)), + ).toBeLessThanOrEqual(2); + }); + + test("Escape in the find field closes the bar", async ({ page }) => { + test.setTimeout(120_000); + await openFind(page); + await page.getByTestId("pdf-editor-find-input").press("Escape"); + await expect(page.getByTestId("pdf-editor-find-bar")).toBeHidden(); + }); +}); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-menu-layout.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-menu-layout.spec.ts index f9b58c0598..f9f6774ff6 100644 --- a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-menu-layout.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-menu-layout.spec.ts @@ -206,8 +206,11 @@ test.describe("PDF text editor - inspector layout", () => { page, }) => { await open(page, 0); - await expect(page.getByTestId("pdf-editor-save")).toBeVisible(); - await expect(page.getByTestId("pdf-editor-download")).toBeVisible(); + // Save finishes the job, so it sits at the foot of the side panel - the + // same place every other tool puts its primary action. + const footer = page.getByTestId("pdf-editor-panel-actions"); + await expect(footer.getByTestId("pdf-editor-save")).toBeVisible(); + await expect(footer.getByTestId("pdf-editor-download")).toBeVisible(); // Zoom sits over the pages it scales, not in the far rail. const zoom = page.getByTestId("pdf-editor-zoom-controls"); await expect(zoom).toBeVisible(); @@ -224,7 +227,7 @@ test.describe("PDF text editor - inspector layout", () => { }) => { await open(page); // Find and the shortcuts sheet are everyday controls, not settings: they - // sit in the panel header, one click from anywhere. + // sit in the toolbar, one click from anywhere. await expect(page.getByTestId("pdf-editor-open-find")).toBeVisible(); await expect(page.getByTestId("pdf-editor-help")).toBeVisible(); @@ -250,7 +253,7 @@ test.describe("PDF text editor - inspector layout", () => { ).toBeVisible(); }); - test("Add text toggles its label and inserts from the panel", async ({ + test("Add text toggles its label and inserts from the toolbar", async ({ page, }) => { await open(page, 0); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-signed-save.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-signed-save.spec.ts new file mode 100644 index 0000000000..3384a9c735 --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-signed-save.spec.ts @@ -0,0 +1,134 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import type { Page } from "@playwright/test"; +import fs from "fs"; +import path from "path"; + +/** + * A signed PDF must come out of the editor still carrying its signed revision. + * + * The tool claims that saving a signed document APPENDS a revision rather than + * rewriting the file, which is the only way the bytes a signature covers stay + * intact. Nothing checked that end to end, so this reads the file the editor + * actually hands back and compares it, byte for byte, with what went in. + */ + +const SIGNED = path.join( + import.meta.dirname, + "../test-fixtures/signed-sample.pdf", +); + +async function openSigned(page: Page): Promise { + await page.goto("/pdf-text-editor", { waitUntil: "domcontentloaded" }); + await expect(page.getByTestId("pdf-editor-root")).toBeVisible({ + timeout: 30_000, + }); + await page + .locator('[data-testid="pdf-editor-file-input"]') + .setInputFiles(SIGNED); + await expect(page.getByTestId("pdf-editor-page-0")).toBeVisible({ + timeout: 60_000, + }); +} + +/** Type into the first run overlay so the save is a real, edited save. */ +async function makeAnEdit(page: Page): Promise { + await page.evaluate(() => { + const el = document.querySelector( + '[data-testid^="pdf-editor-run-"]', + ); + if (!el) throw new Error("no run overlay in the DOM"); + el.focus(); + const selection = window.getSelection(); + if (!selection) throw new Error("no Selection api"); + const range = document.createRange(); + range.selectNodeContents(el); + range.collapse(false); + selection.removeAllRanges(); + selection.addRange(range); + document.execCommand("insertText", false, "Z"); + }); + await expect(page.getByTestId("pdf-editor-dirty-dot")).toBeVisible({ + timeout: 15_000, + }); +} + +test.describe("PDF text editor - signed save stays signed", () => { + test("an edited signed PDF is the original plus an appended revision", async ({ + page, + }) => { + test.setTimeout(180_000); + await openSigned(page); + await makeAnEdit(page); + + const downloadPromise = page.waitForEvent("download"); + await page.getByTestId("pdf-editor-download").click(); + // Signed documents warn before saving; confirm to get the file. + await page.getByTestId("pdf-editor-save-risk-confirm").click(); + const download = await downloadPromise; + const saved = await download.path(); + expect(saved).not.toBeNull(); + + const before = fs.readFileSync(SIGNED); + const after = fs.readFileSync(saved!); + + // An incremental save may only ADD. If the editor had rewritten the file, + // every byte the signature covers would have moved and it would no longer + // verify for the revision it signed. + expect( + after.length, + "the saved file is shorter than the revision it must preserve", + ).toBeGreaterThanOrEqual(before.length); + expect( + after.subarray(0, before.length).equals(before), + "the original signed revision was rewritten, not appended to", + ).toBe(true); + // And it really is an edited save, not a byte-identical passthrough. + expect(after.length).toBeGreaterThan(before.length); + }); + + test("the signature survives as far as PDFium is concerned", async ({ + page, + }) => { + test.setTimeout(180_000); + await openSigned(page); + const before = await page.evaluate(() => + ( + window as unknown as { + __editor_store: { + document: { + module: { FPDF_GetSignatureCount: (doc: number) => number }; + docPtr: number; + }; + }; + } + ).__editor_store.document.module.FPDF_GetSignatureCount( + ( + window as unknown as { + __editor_store: { document: { docPtr: number } }; + } + ).__editor_store.document.docPtr, + ), + ); + expect(before, "fixture must actually carry a signature").toBeGreaterThan( + 0, + ); + }); + + test("an unedited signed PDF saves byte-identical", async ({ page }) => { + test.setTimeout(180_000); + await openSigned(page); + + const downloadPromise = page.waitForEvent("download"); + await page.getByTestId("pdf-editor-download").click(); + await page.getByTestId("pdf-editor-save-risk-confirm").click(); + const download = await downloadPromise; + const saved = await download.path(); + + const before = fs.readFileSync(SIGNED); + const after = fs.readFileSync(saved!); + expect( + after.equals(before), + "an untouched document must come back exactly as it went in", + ).toBe(true); + }); +}); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-topbar.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-topbar.spec.ts new file mode 100644 index 0000000000..59647140fa --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-topbar.spec.ts @@ -0,0 +1,206 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import type { Page } from "@playwright/test"; +import path from "path"; + +/** + * The editor's actions live in the canvas top bar, not the side panel. + * + * Insert, Find, help and Save used to sit in the tool panel, which put the two + * most-used verbs at the bottom of a scrolling column on the far side of the + * screen. These pin them where a document editor's toolbar has always been. + */ + +const SAMPLE_PDF = path.join( + import.meta.dirname, + "../test-fixtures/sample.pdf", +); + +async function openEditor(page: Page): Promise { + await page.goto("/pdf-text-editor", { waitUntil: "domcontentloaded" }); + await expect(page.getByTestId("pdf-editor-root")).toBeVisible({ + timeout: 30_000, + }); + await page + .locator('[data-testid="pdf-editor-file-input"]') + .setInputFiles(SAMPLE_PDF); + await expect(page.getByTestId("pdf-editor-page-0")).toBeVisible({ + timeout: 60_000, + }); +} + +test.describe("PDF text editor - top bar", () => { + test("carries the document actions that used to live in the panel", async ({ + page, + }) => { + test.setTimeout(120_000); + await openEditor(page); + + const toolbar = page.getByTestId("pdf-editor-toolbar"); + await expect(toolbar).toBeVisible(); + for (const id of [ + "pdf-editor-undo", + "pdf-editor-redo", + "pdf-editor-add-text", + "pdf-editor-add-image", + "pdf-editor-open-find", + "pdf-editor-help", + ]) { + await expect( + toolbar.locator(`[data-testid="${id}"]`), + `${id} must be in the top bar`, + ).toBeVisible(); + } + }); + + test("names the open document, and marks it unsaved once edited", async ({ + page, + }) => { + test.setTimeout(120_000); + await openEditor(page); + + await expect(page.getByTestId("pdf-editor-filename")).toContainText( + "sample.pdf", + ); + expect(await page.getByTestId("pdf-editor-dirty-dot").count()).toBe(0); + + // A real keystroke through the contenteditable overlay, not a store poke. + await page.evaluate(() => { + const el = document.querySelector( + '[data-testid^="pdf-editor-run-"]', + ); + if (!el) throw new Error("no run overlay in the DOM"); + el.focus(); + const selection = window.getSelection(); + if (!selection) throw new Error("no Selection api"); + const range = document.createRange(); + range.selectNodeContents(el); + range.collapse(false); + selection.removeAllRanges(); + selection.addRange(range); + document.execCommand("insertText", false, "X"); + }); + + await expect(page.getByTestId("pdf-editor-dirty-dot")).toBeVisible({ + timeout: 15_000, + }); + }); + + test("each control is stated exactly once", async ({ page }) => { + test.setTimeout(120_000); + await openEditor(page); + + // The page verbs live in the toolbar, Save lives in the panel footer, and + // neither is repeated in the other. + for (const id of [ + "pdf-editor-add-text", + "pdf-editor-open-find", + "pdf-editor-help", + "pdf-editor-save", + "pdf-editor-filename", + ]) { + expect( + await page.locator(`[data-testid="${id}"]`).count(), + `${id} must exist exactly once`, + ).toBe(1); + } + }); + + test("Save sits in the panel footer, like every other tool", async ({ + page, + }) => { + test.setTimeout(120_000); + await openEditor(page); + + const footer = page.getByTestId("pdf-editor-panel-actions"); + await expect(footer).toBeVisible(); + await expect( + footer.locator('[data-testid="pdf-editor-save"]'), + ).toBeVisible(); + await expect( + footer.locator('[data-testid="pdf-editor-download"]'), + ).toBeVisible(); + // ...and nowhere else. A primary action in two places is a primary action + // the user has to choose between. + expect( + await page + .getByTestId("pdf-editor-toolbar") + .locator('[data-testid="pdf-editor-save"]') + .count(), + ).toBe(0); + }); + + test("a phone-width layout also gets find and help in the panel", async ({ + page, + }) => { + test.setTimeout(120_000); + await page.setViewportSize({ width: 480, height: 900 }); + await openEditor(page); + + // Here the panel covers the canvas outright, so the toolbar's own find and + // shortcuts buttons are not merely elsewhere - they are off screen. + await expect(page.getByTestId("pdf-editor-panel-actions")).toBeVisible({ + timeout: 15_000, + }); + await expect(page.getByTestId("pdf-editor-save")).toBeVisible(); + await expect(page.getByTestId("pdf-editor-open-find-panel")).toBeVisible(); + await expect(page.getByTestId("pdf-editor-help-panel")).toBeVisible(); + }); + + test("a narrow bar folds insert, find and help into one menu", async ({ + page, + }) => { + test.setTimeout(120_000); + // Wide enough that the canvas still shows, narrow enough that the toolbar + // cannot hold the contextual formatting group AND four standalone buttons. + await page.setViewportSize({ width: 1150, height: 900 }); + await openEditor(page); + + const bar = page.getByTestId("pdf-editor-toolbar"); + await expect(bar).toHaveAttribute("data-compact", "true"); + // The individual buttons are gone; nothing is merely scrolled off-screen. + expect(await page.getByTestId("pdf-editor-add-text").count()).toBe(0); + expect(await page.getByTestId("pdf-editor-open-find").count()).toBe(0); + + // Undo/redo never folds away, and Save is not on the bar to fold. + await expect(bar.locator('[data-testid="pdf-editor-undo"]')).toBeVisible(); + await expect(page.getByTestId("pdf-editor-panel-actions")).toBeVisible(); + + await page.getByTestId("pdf-editor-overflow-menu").click(); + for (const id of [ + "pdf-editor-add-text", + "pdf-editor-add-image", + "pdf-editor-open-find", + "pdf-editor-help", + ]) { + await expect( + page.getByTestId(id), + `${id} must be reachable from the overflow menu`, + ).toBeVisible(); + } + }); + + test("a wide bar keeps every control on show", async ({ page }) => { + test.setTimeout(120_000); + await openEditor(page); + await expect(page.getByTestId("pdf-editor-toolbar")).toHaveAttribute( + "data-compact", + "false", + ); + expect(await page.getByTestId("pdf-editor-overflow-menu").count()).toBe(0); + }); + + test("Find opens beside the pages, not in the panel", async ({ page }) => { + test.setTimeout(120_000); + await openEditor(page); + + await page.getByTestId("pdf-editor-open-find").click(); + const findBar = page.getByTestId("pdf-editor-find-bar"); + await expect(findBar).toBeVisible(); + // Docked under the toolbar, above the page stack. + const bar = await findBar.boundingBox(); + const stage = await page.getByTestId("pdf-editor-stage").boundingBox(); + expect(bar).not.toBeNull(); + expect(stage).not.toBeNull(); + expect(bar!.y).toBeLessThan(stage!.y); + }); +}); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-unsaved-guard.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-unsaved-guard.spec.ts new file mode 100644 index 0000000000..4d0cb4cced --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-unsaved-guard.spec.ts @@ -0,0 +1,176 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import type { Page } from "@playwright/test"; +import path from "path"; +import { uploadFiles } from "@app/tests/helpers/ui-helpers"; + +/** + * Opening a second document must not silently bin the first one's edits. + * + * Every route in disposes the open PDFium document and its whole undo history, + * so a stray click in the file switcher used to throw away everything the user + * had typed with no warning at all. + */ + +const SAMPLE_PDF = path.join( + import.meta.dirname, + "../test-fixtures/sample.pdf", +); +const PARAGRAPH_PDF = path.join( + import.meta.dirname, + "../test-fixtures/paragraph-sample.pdf", +); + +async function openEditorWithTwoFiles(page: Page): Promise { + await page.goto("/pdf-text-editor", { waitUntil: "domcontentloaded" }); + await expect(page.getByTestId("pdf-editor-root")).toBeVisible({ + timeout: 30_000, + }); + await uploadFiles(page, [SAMPLE_PDF, PARAGRAPH_PDF]); + await expect(page.getByTestId("pdf-editor-page-0")).toBeVisible({ + timeout: 60_000, + }); +} + +/** Type into the first run overlay, the way a user would. */ +async function makeAnEdit(page: Page): Promise { + await page.evaluate(() => { + const el = document.querySelector( + '[data-testid^="pdf-editor-run-"]', + ); + if (!el) throw new Error("no run overlay in the DOM"); + el.focus(); + const selection = window.getSelection(); + if (!selection) throw new Error("no Selection api"); + const range = document.createRange(); + range.selectNodeContents(el); + range.collapse(false); + selection.removeAllRanges(); + selection.addRange(range); + document.execCommand("insertText", false, "GUARDED"); + }); + await expect(page.getByTestId("pdf-editor-dirty-dot")).toBeVisible({ + timeout: 15_000, + }); +} + +async function pickOtherFile(page: Page, opened: string): Promise { + const other = opened === "sample.pdf" ? "paragraph-sample.pdf" : "sample.pdf"; + await page.getByTestId("pdf-editor-file-switcher").click(); + await page + .getByTestId("pdf-editor-file-switch") + .filter({ hasText: new RegExp(`^${other}$`) }) + .first() + .click(); + return other; +} + +test.describe("PDF text editor - unsaved-changes guard", () => { + test("switching files with unsaved edits asks first", async ({ page }) => { + test.setTimeout(180_000); + await openEditorWithTwoFiles(page); + const opened = ( + await page.getByTestId("pdf-editor-filename").innerText() + ).trim(); + await makeAnEdit(page); + + const other = await pickOtherFile(page, opened); + + // The modal root stays mounted, so assert on what the user actually sees. + await expect( + page.getByTestId("pdf-editor-discard-confirm"), + "switching files must not discard edits without asking", + ).toBeVisible({ timeout: 10_000 }); + // The prompt names the file it would open, so the answer is informed. + await expect( + page + .getByTestId("pdf-editor-discard-modal") + .locator(".mantine-Modal-body"), + ).toContainText(other); + }); + + test("cancelling keeps the document, its edits and its history", async ({ + page, + }) => { + test.setTimeout(180_000); + await openEditorWithTwoFiles(page); + const opened = ( + await page.getByTestId("pdf-editor-filename").innerText() + ).trim(); + await makeAnEdit(page); + + await pickOtherFile(page, opened); + await page.getByTestId("pdf-editor-discard-cancel").click(); + + await expect(page.getByTestId("pdf-editor-discard-confirm")).toBeHidden(); + await expect(page.getByTestId("pdf-editor-filename")).toContainText(opened); + await expect(page.getByTestId("pdf-editor-dirty-dot")).toBeVisible(); + // The edit itself is still in the model, not just the dirty flag. + const stillThere = await page.evaluate(() => + ( + window as unknown as { + __editor_store: { + getState: () => { + pages: Array<{ runs: Array<{ text: string }> }>; + }; + }; + } + ).__editor_store + .getState() + .pages.flatMap((p) => p.runs) + .some((r) => r.text.includes("GUARDED")), + ); + expect(stillThere).toBe(true); + }); + + test("confirming opens the other file", async ({ page }) => { + test.setTimeout(180_000); + await openEditorWithTwoFiles(page); + const opened = ( + await page.getByTestId("pdf-editor-filename").innerText() + ).trim(); + await makeAnEdit(page); + + const other = await pickOtherFile(page, opened); + await page.getByTestId("pdf-editor-discard-confirm").click(); + + await expect(page.getByTestId("pdf-editor-filename")).toHaveText(other, { + timeout: 60_000, + }); + await expect(page.getByTestId("pdf-editor-page-0")).toBeVisible({ + timeout: 60_000, + }); + }); + + test("a clean document switches with no prompt at all", async ({ page }) => { + test.setTimeout(180_000); + await openEditorWithTwoFiles(page); + const opened = ( + await page.getByTestId("pdf-editor-filename").innerText() + ).trim(); + + const other = await pickOtherFile(page, opened); + + await expect(page.getByTestId("pdf-editor-filename")).toHaveText(other, { + timeout: 60_000, + }); + await expect(page.getByTestId("pdf-editor-discard-confirm")).toBeHidden(); + }); + + test("opening a file from disk over unsaved edits asks too", async ({ + page, + }) => { + test.setTimeout(180_000); + await openEditorWithTwoFiles(page); + await makeAnEdit(page); + + // The same input the drag-and-drop handler feeds. + await page + .locator('[data-testid="pdf-editor-file-input"]') + .setInputFiles(PARAGRAPH_PDF); + + await expect( + page.getByTestId("pdf-editor-discard-confirm"), + "a dropped file must not discard edits without asking", + ).toBeVisible({ timeout: 10_000 }); + }); +}); diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-workbench-files.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-workbench-files.spec.ts index 70bff3eabd..f4d6a7bf12 100644 --- a/frontend/editor/src/core/tests/stubbed/pdf-text-editor-workbench-files.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor-workbench-files.spec.ts @@ -61,19 +61,44 @@ test.describe("PDF text editor - workbench file selection", () => { }) => { test.setTimeout(180_000); await openEditorWithTwoFiles(page); - const opened = ( - await page.getByTestId("pdf-editor-filename").innerText() - ).trim(); + // The document's own text identifies it, and unlike the toolbar's filename + // chip it survives the editor's workbench view being replaced. + const textOf = () => + page.evaluate(() => + ( + window as unknown as { + __editor_store: { + getState: () => { + pages: Array<{ runs: Array<{ text: string }> }>; + }; + }; + } + ).__editor_store + .getState() + .pages.flatMap((p) => p.runs) + .map((r) => r.text) + .join(" ") + .slice(0, 120), + ); + const opened = await textOf(); + expect( + opened.length, + "the fixture must have readable text", + ).toBeGreaterThan(0); // Mounting Active Files trims the selection to its last entry to honour the // tool's one-file limit; the editor must not follow that onto another file. await activeFilesTab(page).click(); - await page.waitForTimeout(2000); + // The canvas unmounting starts the store's dispose grace, after which the + // panel re-opens the same file - so poll rather than sample one frame. + await expect + .poll(async () => await textOf(), { timeout: 30_000 }) + .not.toBe(""); - await expect( - page.getByTestId("pdf-editor-filename"), + expect( + await textOf(), "the editor swapped the open document out from under the user", - ).toHaveText(opened); + ).toBe(opened); }); test("the editor lists the workbench files and opens the one picked", async ({ @@ -94,6 +119,8 @@ test.describe("PDF text editor - workbench file selection", () => { const other = opened === "sample.pdf" ? "paragraph-sample.pdf" : "sample.pdf"; + // The switcher is the toolbar's file-title menu: open it, then pick. + await page.getByTestId("pdf-editor-file-switcher").click(); await page .getByTestId("pdf-editor-file-switch") .filter({ hasText: new RegExp(`^${other}$`) }) @@ -108,6 +135,7 @@ test.describe("PDF text editor - workbench file selection", () => { timeout: 60_000, }); // The picked entry is the one marked current. + await page.getByTestId("pdf-editor-file-switcher").click(); await expect( page.locator( '[data-testid="pdf-editor-file-switch"][data-current="true"]', diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-editor.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-editor.spec.ts index 64b9a7b310..769577c024 100644 --- a/frontend/editor/src/core/tests/stubbed/pdf-text-editor.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/pdf-text-editor.spec.ts @@ -3738,11 +3738,11 @@ test.describe("PDF text editor - dirty state", () => { await gotoEditor(page); await loadSamplePdf(page); - // Save state lives beside the top-bar filename; the sidebar no longer - // repeats it. Clean on load. + // Save state is a dot beside the top-bar filename - a word would squeeze + // the name it sits next to. Clean on load. const filename = page.getByTestId("pdf-editor-filename"); await expect(filename).toBeVisible(); - await expect(filename).not.toContainText("unsaved"); + await expect(page.getByTestId("pdf-editor-dirty-dot")).toHaveCount(0); const firstRunTestId = await page .locator('[data-testid^="pdf-editor-run-p0-"]') @@ -3750,7 +3750,7 @@ test.describe("PDF text editor - dirty state", () => { .getAttribute("data-testid"); await typeIntoRun(page, firstRunTestId!, "X"); - await expect(filename).toContainText("unsaved"); + await expect(page.getByTestId("pdf-editor-dirty-dot")).toBeVisible(); }); }); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/PdfTextEditor.tsx b/frontend/editor/src/core/tools/pdfTextEditor/PdfTextEditor.tsx index 521feefc64..38d2033b06 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/PdfTextEditor.tsx +++ b/frontend/editor/src/core/tools/pdfTextEditor/PdfTextEditor.tsx @@ -3,11 +3,13 @@ import { Alert, Stack } from "@mantine/core"; import { useTranslation } from "react-i18next"; import DescriptionIcon from "@mui/icons-material/DescriptionOutlined"; import { downloadFile } from "@app/services/downloadService"; -import { useFileContext } from "@app/contexts/FileContext"; +import { useFileContext, useFileSelection } from "@app/contexts/FileContext"; import { createStirlingFilesAndStubs } from "@app/services/fileStubHelpers"; import type { FileId } from "@app/types/file"; import type { BaseToolProps } from "@app/types/tool"; import { useEditorStore } from "@app/tools/pdfTextEditor/hooks/useEditorStore"; +import { setEditorSession } from "@app/tools/pdfTextEditor/store/EditorSession"; +import { useIsMobile } from "@app/hooks/useIsMobile"; import { useDocumentLoader, ensureAllPagesRead, @@ -19,11 +21,11 @@ import { useEditorTestGlobal } from "@app/tools/pdfTextEditor/hooks/useEditorTes import { useSelectionActions } from "@app/tools/pdfTextEditor/hooks/useSelectionActions"; import { useEditorKeyboardShortcuts } from "@app/tools/pdfTextEditor/hooks/useEditorKeyboardShortcuts"; import { useEditorClipboard } from "@app/tools/pdfTextEditor/hooks/useEditorClipboard"; -import { FindBar } from "@app/tools/pdfTextEditor/components/FindBar"; -import { HelpOverlay } from "@app/tools/pdfTextEditor/components/HelpOverlay"; import { SaveRiskModal } from "@app/tools/pdfTextEditor/components/SaveRiskModal"; +import { DiscardChangesModal } from "@app/tools/pdfTextEditor/components/DiscardChangesModal"; +import { HelpOverlay } from "@app/tools/pdfTextEditor/components/HelpOverlay"; import { PasswordPromptModal } from "@app/tools/pdfTextEditor/components/PasswordPromptModal"; -import { EditorSaveBar } from "@app/tools/pdfTextEditor/components/EditorSaveBar"; +import { EditorPanelActions } from "@app/tools/pdfTextEditor/components/EditorPanelActions"; import { EditorSidebar } from "@app/tools/pdfTextEditor/components/EditorSidebar"; import { EditorFileInputs } from "@app/tools/pdfTextEditor/components/EditorFileInputs"; import { PageStage } from "@app/tools/pdfTextEditor/components/PageStage"; @@ -49,14 +51,13 @@ const INSERTED_IMAGE_RATIO = 0.4; export default function PdfTextEditor(_props: BaseToolProps) { const { t } = useTranslation(); + const isMobile = useIsMobile(); const { store, state } = useEditorStore(); const load = useDocumentLoader(store); const [selection, setSelection] = useState( store.selection.value, ); - const [findOpen, setFindOpen] = useState(false); - const [helpOpen, setHelpOpen] = useState(false); const [openedFileName, setOpenedFileName] = useState(null); // Set only when the document came from the workbench; a drag-dropped // file has no fileId and can only be downloaded. Mirrored into state so the @@ -68,6 +69,7 @@ export default function PdfTextEditor(_props: BaseToolProps) { setSourceFileId(id); }, []); const { addFiles, consumeFiles, selectors } = useFileContext(); + const { setSelectedFiles } = useFileSelection(); // Saving replaces the workbench file, so for a moment the selection points at // a file the editor has not adopted yet. Auto-load must sit that out. const [applying, setApplying] = useState(false); @@ -370,21 +372,24 @@ export default function PdfTextEditor(_props: BaseToolProps) { [store], ); - const handleFindNext = useCallback((reverse: boolean) => { - setFindOpen(true); - const button = document.querySelector( - reverse - ? '[data-testid="pdf-editor-find-prev"]' - : '[data-testid="pdf-editor-find-next"]', - ); - button?.click(); - }, []); + const handleFindNext = useCallback( + (reverse: boolean) => { + store.setFindOpen(true); + const button = document.querySelector( + reverse + ? '[data-testid="pdf-editor-find-prev"]' + : '[data-testid="pdf-editor-find-next"]', + ); + button?.click(); + }, + [store], + ); const handleEscape = useCallback(() => { store.selection.clear(); store.setMode("select"); - setHelpOpen(false); - setFindOpen(false); + store.setHelpOpen(false); + store.setFindOpen(false); }, [store]); const handleUngroupSelection = useCallback(() => { @@ -455,8 +460,11 @@ export default function PdfTextEditor(_props: BaseToolProps) { .pages.flatMap((p) => p.runs.map((r) => r.id)); if (ids.length > 0) store.selection.selectMany(ids); }, [store]), - onToggleHelp: useCallback(() => setHelpOpen((v) => !v), []), - onOpenFind: useCallback(() => setFindOpen(true), []), + onToggleHelp: useCallback( + () => store.setHelpOpen(!store.getState().helpOpen), + [store], + ), + onOpenFind: useCallback(() => store.setFindOpen(true), [store]), onFindNext: handleFindNext, onEscape: handleEscape, onMergeSelection: handleMergeSelection, @@ -477,8 +485,19 @@ export default function PdfTextEditor(_props: BaseToolProps) { .find((r) => r.id === selection.runIds[0]); return !!run && (run.paragraphLineCount ?? 0) > 1; })(); - const onPickPdf = useCallback( - (file: File) => { + // Opening any document disposes the one in memory - edits, undo history and + // all - so both routes in go through `requestOpen` first. + const openDocument = useCallback( + (file: File, fromDisk: boolean) => { + if (!fromDisk) { + // Deferred until the open is agreed: moving the workbench selection + // for an open the user then cancels leaves the rest of the app + // pointing at a file the editor is not editing. + const fileId = (file as File & { fileId?: FileId }).fileId; + if (fileId != null) setSelectedFiles([fileId]); + openWorkbenchFile(file); + return; + } setOpenedFileName(file.name); // Dropped/picked from disk: no workbench file to replace yet, but claim // it so a later workbench arrival cannot auto-open over these edits. @@ -486,9 +505,70 @@ export default function PdfTextEditor(_props: BaseToolProps) { setSourceFile(null); void load(file); }, - [adoptFile, load, setSourceFile], + [adoptFile, load, openWorkbenchFile, setSelectedFiles, setSourceFile], ); + // A document waiting on the user's answer to "discard your changes?". + const [pendingOpen, setPendingOpen] = useState<{ + file: File; + fromDisk: boolean; + } | null>(null); + + const requestOpen = useCallback( + (file: File, fromDisk: boolean) => { + // Read the store, not the render's copy: a keystroke that dirtied the + // document in the same tick must still be caught. + if (store.getState().dirty) { + setPendingOpen({ file, fromDisk }); + return; + } + openDocument(file, fromDisk); + }, + [openDocument, store], + ); + + const onPickPdf = useCallback( + (file: File) => requestOpen(file, true), + [requestOpen], + ); + const onPickWorkbenchFile = useCallback( + (file: File) => requestOpen(file, false), + [requestOpen], + ); + const confirmPendingOpen = useCallback(() => { + const pending = pendingOpen; + setPendingOpen(null); + if (pending) openDocument(pending.file, pending.fromDisk); + }, [openDocument, pendingOpen]); + + const openImagePicker = useCallback(() => { + document + .querySelector('[data-testid="pdf-editor-image-input"]') + ?.click(); + }, []); + + // Publish what the canvas top bar cannot reach on its own. Kept in an effect + // so the canvas always sees the CURRENT handlers, and retracted on unmount so + // a stale Save can never fire against a panel that is gone. + useEffect(() => { + setEditorSession({ + fileName: openedFileName, + fileId: sourceFileId, + save: handleSave, + download: handleDownload, + pickFile: onPickWorkbenchFile, + pickImage: openImagePicker, + }); + return () => setEditorSession(null); + }, [ + openedFileName, + sourceFileId, + handleSave, + handleDownload, + onPickWorkbenchFile, + openImagePicker, + ]); + const handleSubmitPassword = useCallback( (password: string) => { const file = store.pendingPasswordFile; @@ -515,14 +595,13 @@ export default function PdfTextEditor(_props: BaseToolProps) { )} - {findOpen && state.hasDocument && ( - setFindOpen(false)} - /> - )} - setHelpOpen(false)} /> + {/* Declared here, not on the canvas: the canvas unmounts whenever the + workbench shows something else, and a modal that only exists while + the page stack is on screen cannot be opened from the panel. */} + store.setHelpOpen(false)} + /> + setPendingOpen(null)} + /> store.setGroupingMode(mode)} onSetWidthMode={(m) => store.setWidthMode(m)} onSetShowRulers={(show) => store.setShowRulers(show)} - onOpenFind={() => setFindOpen(true)} - onShowHelp={() => setHelpOpen(true)} - addTextArmed={state.mode === "addText"} - onToggleAddText={() => - store.setMode( - store.getState().mode === "addText" ? "select" : "addText", - ) - } - onPickImage={() => - document - .querySelector( - '[data-testid="pdf-editor-image-input"]', - ) - ?.click() - } /> + {/* Every tool pins its primary action to the bottom of this panel, so + the editor's Save lives there too, at every width. */} {state.hasDocument && ( - { + if (store.getState().findOpen) { + store.setFindOpen(false); + return; + } + pinWorkbench(); + store.setFindOpen(true); + }} + onShowHelp={() => store.setHelpOpen(true)} /> )} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/__tests__/commandRollback.test.ts b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/commandRollback.test.ts new file mode 100644 index 0000000000..e5d75fdc26 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/commandRollback.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, it } from "vitest"; +import { CompositeCommand } from "@app/tools/pdfTextEditor/commands/CompositeCommand"; +import { + RolledBackError, + type Command, +} from "@app/tools/pdfTextEditor/commands/Command"; +import { + HistoryStack, + HistoryStepError, +} from "@app/tools/pdfTextEditor/store/HistoryStack"; +import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; + +const doc = {} as EditorDocument; + +/** A command that appends to `log` so the exact call order is assertable. */ +function tracked( + name: string, + log: string[], + fail?: { apply?: boolean; revert?: boolean }, +): Command { + return { + type: name, + apply: () => { + if (fail?.apply) throw new Error(`${name} apply blew up`); + log.push(`+${name}`); + }, + revert: () => { + if (fail?.revert) throw new Error(`${name} revert blew up`); + log.push(`-${name}`); + }, + } as unknown as Command; +} + +describe("CompositeCommand rollback", () => { + it("undoes the children that ran when a later apply throws", () => { + const log: string[] = []; + const composite = new CompositeCommand([ + tracked("a", log), + tracked("b", log), + tracked("c", log, { apply: true }), + tracked("d", log), + ]); + expect(() => composite.apply(doc)).toThrow(RolledBackError); + // b and a reverted in reverse order; d never ran. + expect(log).toEqual(["+a", "+b", "-b", "-a"]); + }); + + it("re-applies the children that ran when a revert throws", () => { + const log: string[] = []; + const composite = new CompositeCommand([ + tracked("a", log), + tracked("b", log, { revert: true }), + tracked("c", log), + ]); + // revert() runs c then b; b throws, so c is put back. + expect(() => composite.revert(doc)).toThrow(RolledBackError); + expect(log).toEqual(["-c", "+c"]); + }); + + it("reports an unrecoverable failure when the rollback itself throws", () => { + const log: string[] = []; + const composite = new CompositeCommand([ + // Applies fine, but cannot be undone - so the group is genuinely stuck. + tracked("a", log, { revert: true }), + tracked("b", log, { apply: true }), + ]); + let thrown: unknown; + try { + composite.apply(doc); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(Error); + expect(thrown).not.toBeInstanceOf(RolledBackError); + }); + + it("leaves nothing applied when the FIRST child throws", () => { + const log: string[] = []; + const composite = new CompositeCommand([ + tracked("a", log, { apply: true }), + tracked("b", log), + ]); + expect(() => composite.apply(doc)).toThrow(RolledBackError); + expect(log).toEqual([]); + }); +}); + +describe("HistoryStack forward-apply failures", () => { + it("surfaces a failed execute as a HistoryStepError", () => { + const h = new HistoryStack(); + const log: string[] = []; + expect(() => h.execute(tracked("a", log, { apply: true }), doc)).toThrow( + HistoryStepError, + ); + }); + + it("does not record a command whose apply threw", () => { + const h = new HistoryStack(); + const log: string[] = []; + try { + h.execute(tracked("a", log, { apply: true }), doc); + } catch { + /* expected */ + } + // Recording it would make the next undo revert changes never made. + expect(h.size()).toEqual({ undo: 0, redo: 0 }); + expect(h.canUndo).toBe(false); + }); + + it("keeps the previous history intact after a failed execute", () => { + const h = new HistoryStack(); + const log: string[] = []; + h.execute(tracked("a", log), doc); + try { + h.execute(tracked("b", log, { apply: true }), doc); + } catch { + /* expected */ + } + expect(h.size()).toEqual({ undo: 1, redo: 0 }); + h.undo(doc); + expect(log).toEqual(["+a", "-a"]); + }); + + it("marks a rolled-back composite as leaving the document intact", () => { + const h = new HistoryStack(); + const log: string[] = []; + const composite = new CompositeCommand([ + tracked("a", log), + tracked("b", log, { apply: true }), + ]); + let thrown: unknown; + try { + h.execute(composite, doc); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(HistoryStepError); + expect((thrown as HistoryStepError).documentIntact).toBe(true); + // The cause is unwrapped, so the UI shows what actually failed. + expect((thrown as HistoryStepError).cause).toBeInstanceOf(Error); + expect(String((thrown as HistoryStepError).cause)).toMatch(/b apply/); + }); + + it("marks a bare command's failure as leaving the document unknown", () => { + const h = new HistoryStack(); + const log: string[] = []; + let thrown: unknown; + try { + h.execute(tracked("a", log, { apply: true }), doc); + } catch (err) { + thrown = err; + } + expect((thrown as HistoryStepError).documentIntact).toBe(false); + }); + + it("a failed execute does not coalesce the next edit into it", () => { + const h = new HistoryStack(); + const log: string[] = []; + const keyed = (name: string, fail?: boolean): Command => + ({ + ...tracked(name, log, fail ? { apply: true } : undefined), + coalesceKey: () => "same", + }) as unknown as Command; + h.execute(keyed("a"), doc); + try { + h.execute(keyed("b", true), doc); + } catch { + /* expected */ + } + h.execute(keyed("c"), doc); + // Two separate undo steps: the failure ended the burst. + expect(h.size()).toEqual({ undo: 2, redo: 0 }); + }); +}); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/__tests__/pdfiumSaveGuards.test.ts b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/pdfiumSaveGuards.test.ts new file mode 100644 index 0000000000..0c04cc8574 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/pdfiumSaveGuards.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it, vi } from "vitest"; +import { PdfiumSave } from "@app/tools/pdfTextEditor/pdfium/PdfiumSave"; +import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; + +const MIN_PDF = `%PDF-1.7\n${"x".repeat(400)}\n%%EOF\n`; + +interface FakeOptions { + /** What PDFiumExt_SaveAsCopy returns (PDFium's FPDF_BOOL). */ + saveResult?: number; + /** What FPDF_SaveAsCopy returns; omit the key to drop the entry point. */ + flaggedResult?: boolean | null; + /** Bytes the writer hands back. */ + output?: string; +} + +/** + * A PDFium module stub that writes `output` into a fake wasm heap. + * + * Big enough to be honest about the save path - the malloc/heap dance is where + * a size of 0 would otherwise slip through as "saved fine". + */ +function fakeDoc(opts: FakeOptions = {}) { + const text = opts.output ?? MIN_PDF; + const bytes = new Uint8Array(text.length); + for (let i = 0; i < text.length; i++) bytes[i] = text.charCodeAt(i); + const memory = { buffer: new ArrayBuffer(64 * 1024) }; + const heap = new Uint8Array(memory.buffer); + const BASE = 1024; + + const saveAsCopy = vi.fn(() => { + heap.set(bytes, BASE); + return opts.saveResult ?? 1; + }); + const flagged = vi.fn(() => { + heap.set(bytes, BASE); + return opts.flaggedResult ?? true; + }); + + const module: Record = { + PDFiumExt_OpenFileWriter: () => 7, + PDFiumExt_CloseFileWriter: vi.fn(), + PDFiumExt_SaveAsCopy: saveAsCopy, + PDFiumExt_GetFileWriterSize: () => bytes.length, + PDFiumExt_GetFileWriterData: (_w: number, out: number, size: number) => { + heap.copyWithin(out, BASE, BASE + size); + }, + pdfium: { + wasmExports: { malloc: () => 8192, free: vi.fn(), memory }, + }, + }; + if (opts.flaggedResult !== null) module.FPDF_SaveAsCopy = flagged; + + const doc = { + module, + docPtr: 42, + loadedPages: () => [], + } as unknown as EditorDocument; + return { doc, saveAsCopy, flagged }; +} + +describe("PdfiumSave failure detection", () => { + it("returns the bytes when PDFium reports success", () => { + const { doc } = fakeDoc(); + expect(PdfiumSave.serialize(doc).length).toBe(MIN_PDF.length); + }); + + it("throws when PDFium's save returns false", () => { + const { doc } = fakeDoc({ saveResult: 0 }); + // Ignoring this return code is what let a failed save overwrite the user's + // file with whatever the writer happened to hold. + expect(() => PdfiumSave.serialize(doc)).toThrow(/could not serialize/i); + }); + + it("throws when the flagged incremental save returns false", () => { + const { doc } = fakeDoc({ flaggedResult: false }); + expect(() => PdfiumSave.serialize(doc, { incremental: true })).toThrow( + /could not serialize/i, + ); + }); + + it("throws when the writer hands back an empty buffer", () => { + const { doc } = fakeDoc({ output: "" }); + expect(() => PdfiumSave.serialize(doc)).toThrow(/too short/i); + }); + + it("throws when the writer hands back something that is not a PDF", () => { + const { doc } = fakeDoc({ output: "N".repeat(2048) }); + expect(() => PdfiumSave.serialize(doc)).toThrow(/PDF header/i); + }); + + it("uses the flagged entry point for an incremental save", () => { + const { doc, flagged, saveAsCopy } = fakeDoc(); + PdfiumSave.serialize(doc, { incremental: true }); + expect(flagged).toHaveBeenCalledWith(42, 7, 1); + expect(saveAsCopy).not.toHaveBeenCalled(); + }); + + it("refuses to silently full-rewrite when the flagged entry point is gone", () => { + const { doc, saveAsCopy } = fakeDoc({ flaggedResult: null }); + expect(() => PdfiumSave.serialize(doc, { incremental: true })).toThrow( + /FPDF_SaveAsCopy/, + ); + // The whole point: no rewrite of the signed revision happened. + expect(saveAsCopy).not.toHaveBeenCalled(); + }); + + it("still uses the plain entry point when incremental was not asked for", () => { + const { doc, saveAsCopy, flagged } = fakeDoc(); + PdfiumSave.serialize(doc); + expect(saveAsCopy).toHaveBeenCalledWith(42, 7); + expect(flagged).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/__tests__/savedBytes.test.ts b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/savedBytes.test.ts new file mode 100644 index 0000000000..2c38e50612 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/savedBytes.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; +import { + assertIncrementalAppend, + assertSavedPdf, +} from "@app/tools/pdfTextEditor/util/savedBytes"; + +function ascii(text: string): Uint8Array { + const out = new Uint8Array(text.length); + for (let i = 0; i < text.length; i++) out[i] = text.charCodeAt(i); + return out; +} + +/** A byte string long enough to clear the minimum-length check. */ +function pdf(body = "x".repeat(400), tail = "\n%%EOF\n"): Uint8Array { + return ascii(`%PDF-1.7\n${body}${tail}`); +} + +describe("assertSavedPdf", () => { + it("accepts a plausible PDF", () => { + expect(() => assertSavedPdf(pdf())).not.toThrow(); + }); + + it("rejects the empty buffer a failed writer hands back", () => { + expect(() => assertSavedPdf(new Uint8Array(0))).toThrow(/too short/i); + }); + + it("rejects a truncated write", () => { + expect(() => assertSavedPdf(ascii("%PDF-1.7\n%%EOF\n"))).toThrow( + /too short/i, + ); + }); + + it("rejects data that is not a PDF at all", () => { + expect(() => assertSavedPdf(ascii("N".repeat(2048)))).toThrow( + /PDF header/i, + ); + }); + + it("rejects a PDF with no end-of-file marker", () => { + expect(() => assertSavedPdf(pdf("x".repeat(400), "\n"))).toThrow( + /end-of-file/i, + ); + }); + + it("rejects an EOF marker buried far from the end", () => { + // %%EOF followed by 8k of padding: the real terminator is missing. + expect(() => assertSavedPdf(pdf("%%EOF" + "\0".repeat(8192), ""))).toThrow( + /end-of-file/i, + ); + }); +}); + +describe("assertIncrementalAppend", () => { + const original = pdf(); + + it("accepts bytes that only appended a revision", () => { + const appended = new Uint8Array(original.length + 32); + appended.set(original); + appended.set(ascii("\n1 0 obj\n<<>>\nendobj\n%%EOF\n"), original.length); + expect(() => assertIncrementalAppend(appended, original)).not.toThrow(); + }); + + it("accepts a byte-identical save", () => { + expect(() => assertIncrementalAppend(original, original)).not.toThrow(); + }); + + it("rejects a save shorter than the revision it must preserve", () => { + expect(() => + assertIncrementalAppend(original.slice(0, original.length - 1), original), + ).toThrow(/shorter/i); + }); + + it("rejects a full rewrite that changed the signed bytes", () => { + const rewritten = new Uint8Array(original); + rewritten[100] = rewritten[100] ^ 0xff; + expect(() => assertIncrementalAppend(rewritten, original)).toThrow( + /byte 100/, + ); + }); +}); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/__tests__/storeApplyRecovery.test.ts b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/storeApplyRecovery.test.ts new file mode 100644 index 0000000000..7568bc3d13 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/__tests__/storeApplyRecovery.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it, vi } from "vitest"; +import { EditorStore } from "@app/tools/pdfTextEditor/store/EditorStore"; +import { CompositeCommand } from "@app/tools/pdfTextEditor/commands/CompositeCommand"; +import type { Command } from "@app/tools/pdfTextEditor/commands/Command"; +import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; + +function makeCmd(overrides: Partial = {}): Command { + return { + type: "test", + apply: vi.fn(), + revert: vi.fn(), + ...overrides, + } as unknown as Command; +} + +/** Minimal document stub: no pages, so repopulate is a no-op. */ +function makeDoc(): EditorDocument { + return { + pageCount: 0, + loadedPages: () => [], + dispose: () => {}, + } as unknown as EditorDocument; +} + +async function makeStore(): Promise { + const store = new EditorStore(); + await store.setDocument(makeDoc()); + return store; +} + +describe("EditorStore recovery from a failed apply", () => { + it("does not throw out of dispatch when a command's apply fails", async () => { + const store = await makeStore(); + expect(() => + store.dispatch( + makeCmd({ + apply: () => { + throw new Error("pdfium said no"); + }, + }), + ), + ).not.toThrow(); + }); + + it("surfaces the underlying failure, not the history wrapper", async () => { + const store = await makeStore(); + store.dispatch( + makeCmd({ + apply: () => { + throw new Error("pdfium said no"); + }, + }), + ); + expect(store.getState().error).toBe("pdfium said no"); + }); + + it("throws the history away when the document may be half-changed", async () => { + const store = await makeStore(); + store.dispatch(makeCmd()); + expect(store.history.canUndo).toBe(true); + store.dispatch( + makeCmd({ + apply: () => { + throw new Error("boom"); + }, + }), + ); + // A bare command cannot say what it did, so nothing already on the stack + // can be trusted to describe the page any more. + expect(store.history.canUndo).toBe(false); + expect(store.getState().dirty).toBe(true); + }); + + it("keeps the history when a composite rolled itself back", async () => { + const store = await makeStore(); + store.dispatch(makeCmd()); + const before = store.history.peekUndo(); + store.dispatch( + new CompositeCommand([ + makeCmd(), + makeCmd({ + apply: () => { + throw new Error("second child failed"); + }, + }), + ]), + ); + // The group put the document back, so the earlier edit is still undoable. + expect(store.history.peekUndo()).toBe(before); + expect(store.getState().error).toBe("second child failed"); + }); + + it("a failed edit does not mark a clean document dirty when it rolled back", async () => { + const store = await makeStore(); + store.dispatch( + new CompositeCommand([ + makeCmd(), + makeCmd({ + apply: () => { + throw new Error("nope"); + }, + }), + ]), + ); + expect(store.getState().dirty).toBe(false); + }); +}); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/commands/Command.ts b/frontend/editor/src/core/tools/pdfTextEditor/commands/Command.ts index ee05506068..941fc68028 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/commands/Command.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/commands/Command.ts @@ -16,3 +16,21 @@ export interface Command { // previous undo step however long ago that step ran. coalesceIgnoresTimeWindow?(previous: Command | null): boolean; } + +/** + * A command failed to apply/revert but the document was put back as it was. + * + * Only `CompositeCommand` can promise this - it knows which children ran and + * can undo exactly those. The store reads it to decide whether the run model + * still describes the page (surface the error) or has to be rebuilt from + * PDFium (throw the history away). + */ +export class RolledBackError extends Error { + readonly cause: unknown; + + constructor(cause: unknown) { + super(cause instanceof Error ? cause.message : String(cause)); + this.name = "RolledBackError"; + this.cause = cause; + } +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/commands/CompositeCommand.ts b/frontend/editor/src/core/tools/pdfTextEditor/commands/CompositeCommand.ts index 8433c689f8..e87509ac70 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/commands/CompositeCommand.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/commands/CompositeCommand.ts @@ -1,4 +1,7 @@ -import type { Command } from "@app/tools/pdfTextEditor/commands/Command"; +import { + RolledBackError, + type Command, +} from "@app/tools/pdfTextEditor/commands/Command"; import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; /** Groups several already-applied commands into one undo/redo step. */ @@ -21,12 +24,47 @@ export class CompositeCommand implements Command { } apply(doc: EditorDocument): void { - for (const cmd of this.commands) cmd.apply(doc); + this.run(doc, this.commands, "apply"); } revert(doc: EditorDocument): void { - for (let i = this.commands.length - 1; i >= 0; i--) { - this.commands[i].revert(doc); + const reversed = [...this.commands].reverse(); + this.run(doc, reversed, "revert"); + } + + /** + * Run every child in order, undoing the ones that already ran if one throws. + * + * A group is ONE undo step, so half of it landing is worse than none of it: + * the model would describe a page that no longer matches and the user's next + * undo would revert children that never applied. Rolling back keeps the + * document exactly as it was found, which is what `RolledBackError` promises. + */ + private run( + doc: EditorDocument, + order: Command[], + phase: "apply" | "revert", + ): void { + const done: Command[] = []; + for (const cmd of order) { + try { + if (phase === "apply") cmd.apply(doc); + else cmd.revert(doc); + } catch (err) { + for (let i = done.length - 1; i >= 0; i--) { + try { + if (phase === "apply") done[i].revert(doc); + else done[i].apply(doc); + } catch { + // The rollback itself failed, so the document is genuinely + // half-changed - report the original failure unwrapped so the + // caller rebuilds instead of trusting the model. + throw err; + } + } + throw new RolledBackError(err); + } + done.push(cmd); } } diff --git a/frontend/editor/src/core/tools/pdfTextEditor/commands/EditTextCommand.ts b/frontend/editor/src/core/tools/pdfTextEditor/commands/EditTextCommand.ts index 1033176dad..964acd6cfe 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/commands/EditTextCommand.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/commands/EditTextCommand.ts @@ -482,6 +482,23 @@ export class EditTextCommand implements Command { // emits land at the same baselines we just established. run.paragraphLineHeight = lineHeight; } + } else { + // Nothing was emitted, which means the run is now empty. Every object it + // owned was removed just above, so its pointers have to go too: left + // behind, the NEXT edit removes and reads freed objects, and whatever it + // emits never reaches the page. + run.pdfiumObjPtr = 0; + run.paragraphMemberPtrs = []; + run.paragraphMemberContainers = []; + run.paragraphMemberFs = []; + run.paragraphLeafPtrs = []; + run.paragraphLeafContainers = []; + // The box has no ink left, so its old width is not geometry any more - + // and the overlay treats run.bounds.width as a floor, so leaving it would + // make whatever is typed next sit in a box the size of what was deleted. + // The emit below only ever GROWS bounds, which rebuilds this from the + // fresh span. + run.bounds = { ...run.bounds, width: 0 }; } run.mergedFromPtrs = []; diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/DiscardChangesModal.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/DiscardChangesModal.tsx new file mode 100644 index 0000000000..5469035f0a --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/DiscardChangesModal.tsx @@ -0,0 +1,69 @@ +import { Group, Modal, Stack, Text } from "@mantine/core"; +import { useTranslation } from "react-i18next"; +import { Button } from "@app/ui/Button"; + +interface Props { + /** Name of the document that would be opened, or null when closed. */ + incomingFileName: string | null; + onConfirm: () => void; + onCancel: () => void; +} + +/** + * Last stop before the editor throws away unsaved edits. + * + * Opening another document disposes the one in memory along with its whole + * undo history, so every route into that - the file switcher, a drop onto the + * canvas - has to pass through here first. Cancel is the default action: the + * user reaching for a file list has not asked to lose anything. + */ +export function DiscardChangesModal({ + incomingFileName, + onConfirm, + onCancel, +}: Props) { + const { t } = useTranslation(); + return ( + + + + {t( + "pdfTextEditor.discard.body", + "Opening {{name}} closes the document you are editing. Your unsaved changes will be lost.", + { name: incomingFileName ?? "" }, + )} + + + {t( + "pdfTextEditor.discard.hint", + "Cancel, then Save PDF to keep them in your workspace first.", + )} + + + + + + + + ); +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/EditorFileSwitcher.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorFileSwitcher.tsx index b782700913..c5a6e76907 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/components/EditorFileSwitcher.tsx +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorFileSwitcher.tsx @@ -1,66 +1,117 @@ -import { Stack, Text } from "@mantine/core"; +import { Menu, Text, Tooltip } from "@mantine/core"; import { useTranslation } from "react-i18next"; import DescriptionIcon from "@mui/icons-material/DescriptionOutlined"; +import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; +import CheckIcon from "@mui/icons-material/Check"; import { Button } from "@app/ui/Button"; -import { useAllFiles, useFileSelection } from "@app/contexts/FileContext"; +import { useAllFiles } from "@app/contexts/FileContext"; import type { FileId } from "@app/types/file"; interface Props { /** Workbench file the editor currently holds, when it came from one. */ currentFileId: FileId | null; - /** Open the picked file; the editor never follows the selection on its own. */ + /** Name shown on the chip - the open document, workbench file or not. */ + currentFileName: string; + /** Unsaved-changes marker, shown as a dot beside the name. */ + dirty: boolean; + // Ask the editor to open the picked file. It may refuse - a dirty document + // gets a confirmation first - so the workbench selection is NOT moved here; + // the editor syncs it once the open actually goes ahead. onPick: (file: File) => void; } /** - * Switch which workbench file the editor is editing. + * Which document you are editing, and how to edit a different one. * - * The editor owns the whole canvas, so the workbench's own Active Files grid is - * a view away; without this the user can open the tool with several files - * loaded and have no way to say which one to edit. Picking here sets the - * workbench selection rather than loading directly, so the rest of the app - * agrees about which file is being worked on. + * Sits at the top-left of the toolbar because that is where a document editor + * puts the document's name; picking from it is the same gesture as any other + * app's file title menu. With fewer than two PDFs in the workbench there is + * nothing to switch to, so it collapses to a plain name. */ -export function EditorFileSwitcher({ currentFileId, onPick }: Props) { +export function EditorFileSwitcher({ + currentFileId, + currentFileName, + dirty, + onPick, +}: Props) { const { t } = useTranslation(); const { files } = useAllFiles(); - const { setSelectedFiles } = useFileSelection(); const pdfs = files.filter((f) => /\.pdf$/i.test(f.name)); - if (pdfs.length < 2) return null; + const label = ( + + + {currentFileName} + {dirty && ( + + )} + + ); + + if (pdfs.length < 2) { + return ( + + + {label} + + + ); + } return ( - - - {t("pdfTextEditor.sidebar.document", "Document")} - - {pdfs.map((file) => { - const fileId = (file as File & { fileId?: FileId }).fileId; - const current = fileId != null && fileId === currentFileId; - return ( - - ); - })} - + + + + + + + {t("pdfTextEditor.sidebar.document", "Document")} + + {pdfs.map((file) => { + const fileId = (file as File & { fileId?: FileId }).fileId; + const current = fileId != null && fileId === currentFileId; + return ( + + ) : ( + + ) + } + disabled={fileId == null} + data-testid="pdf-editor-file-switch" + data-current={current ? "true" : "false"} + onClick={() => { + if (fileId == null) return; + onPick(file); + }} + > + {file.name} + + ); + })} + + ); } diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/EditorPanelActions.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorPanelActions.tsx new file mode 100644 index 0000000000..f50159c3b8 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorPanelActions.tsx @@ -0,0 +1,148 @@ +import { Box, Group, Tooltip } from "@mantine/core"; +import { useTranslation } from "react-i18next"; +import { Button } from "@app/ui/Button"; +import DownloadIcon from "@mui/icons-material/FileDownloadOutlined"; +import SearchIcon from "@mui/icons-material/SearchOutlined"; +import HelpIcon from "@mui/icons-material/HelpOutlineOutlined"; +import { EditorFileSwitcher } from "@app/tools/pdfTextEditor/components/EditorFileSwitcher"; +import type { FileId } from "@app/types/file"; + +interface Props { + /** Phones also lose the toolbar behind this panel, so it carries its extras. */ + compact: boolean; + openedFileName: string | null; + dirty: boolean; + /** Workbench file currently open, so the switcher can mark it. */ + currentFileId: FileId | null; + /** Open a different workbench file. */ + onPickFile: (file: File) => void; + onSave: () => void; + onDownload: () => void; + onOpenFind: () => void; + onShowHelp: () => void; +} + +/** + * Where the editor finishes, in the place every other tool finishes. + * + * Every tool in the app pins its primary action to the bottom of this panel + * (see `createToolFlow`'s executeButton and its sticky `executeFooter`), so the + * editor's Save belongs here too - a user who has learnt where "the button that + * does the thing" lives should not have to learn a second answer for one tool. + * + * It is here at every width, and it is the ONLY Save: a primary action that + * appears in two places, or moves between them, is worse than one that is + * always in the same place. Find, help and the file switcher ride along only on + * phones, where the panel covers the canvas and the toolbar carrying them is + * off screen entirely. + */ +export function EditorPanelActions({ + compact, + openedFileName, + dirty, + currentFileId, + onPickFile, + onSave, + onDownload, + onOpenFind, + onShowHelp, +}: Props) { + const { t } = useTranslation(); + return ( + + {compact && openedFileName && ( + + + + )} + + + + + + - - - - diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.css b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.css new file mode 100644 index 0000000000..155de0cfe9 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.css @@ -0,0 +1,87 @@ +/* The editor's top bar: a fixed identity block on the left, a scrolling band of + controls in the middle, and the finishing actions pinned right. */ + +.pdf-editor-topbar { + display: flex; + align-items: center; + gap: 0.5rem; + padding: 0.375rem 0.5rem; + border-bottom: 0.0625rem solid var(--c-border-subtle); + background: var(--c-bg-raised); + /* The bar itself never scrolls: the Save cluster has to stay reachable no + matter how many contextual controls the selection adds. */ + overflow: hidden; + flex: 0 0 auto; + /* Queried by width below. The canvas can be narrow while the WINDOW is wide + (the tool panel takes its share), so a viewport media query would keep + labels the bar has no room for. */ + container-type: inline-size; + container-name: pdfeditortoolbar; +} + +.pdf-editor-topbar__lead, +.pdf-editor-topbar__trail { + display: flex; + align-items: center; + gap: 0.25rem; + flex: 0 0 auto; +} + +/* Only the middle band scrolls, and only when the contextual groups overflow. */ +.pdf-editor-topbar__band { + display: flex; + align-items: center; + gap: 0.25rem; + flex: 1 1 auto; + min-width: 0; + overflow-x: auto; + overflow-y: hidden; + scrollbar-width: thin; + /* Room for the thin scrollbar so it never sits on top of the buttons. */ + padding-bottom: 0.125rem; +} + +.pdf-editor-topbar__file { + display: flex; + align-items: center; + gap: 0.375rem; + min-width: 0; + max-width: 15rem; +} + +.pdf-editor-topbar__filename { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: 0.8125rem; + font-weight: 500; +} + +/* Unsaved marker: a dot, not the word, so it never squeezes the name. */ +.pdf-editor-topbar__dirty { + flex: 0 0 auto; + width: 0.4375rem; + height: 0.4375rem; + border-radius: 50%; + background: var(--c-warning); +} + +/* Squeezed: drop the words the icons already say, in order of expendability - + the file name (the tooltip still has it), then the Save label. */ +@container pdfeditortoolbar (max-width: 62rem) { + .pdf-editor-topbar__filename { + display: none; + } +} + +@container pdfeditortoolbar (max-width: 52rem) { + .pdf-editor-topbar__save-label { + display: none; + } +} + +@media (max-width: 64rem) { + .pdf-editor-topbar__file { + max-width: 8rem; + } +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.tsx new file mode 100644 index 0000000000..a3e41938f6 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/EditorTopBar.tsx @@ -0,0 +1,321 @@ +import { useRef } from "react"; +import { Group, Menu, Popover, Text, Tooltip } from "@mantine/core"; +import { useTranslation } from "react-i18next"; +import UndoIcon from "@mui/icons-material/Undo"; +import RedoIcon from "@mui/icons-material/Redo"; +import TextFieldsIcon from "@mui/icons-material/TextFieldsOutlined"; +import ImageIcon from "@mui/icons-material/ImageOutlined"; +import SearchIcon from "@mui/icons-material/SearchOutlined"; +import HelpIcon from "@mui/icons-material/HelpOutlineOutlined"; +import MoreVertIcon from "@mui/icons-material/MoreVert"; +import TextFormatIcon from "@mui/icons-material/TextFormat"; +import { Button } from "@app/ui/Button"; +import { EditorFileSwitcher } from "@app/tools/pdfTextEditor/components/EditorFileSwitcher"; +import { FormatGroup } from "@app/tools/pdfTextEditor/components/toolbar/FormatGroup"; +import { ObjectGroup } from "@app/tools/pdfTextEditor/components/toolbar/ObjectGroup"; +import { + NO_SHRINK, + ToolbarSeparator, + type Controller, +} from "@app/tools/pdfTextEditor/components/toolbar/toolbarShared"; +import { useEditorSession } from "@app/tools/pdfTextEditor/store/EditorSession"; +import { useElementWidth } from "@app/tools/pdfTextEditor/hooks/useElementWidth"; +import "@app/tools/pdfTextEditor/components/EditorTopBar.css"; + +// Below this the insert/find/help controls stop being buttons and become one +// menu. Chosen so the contextual formatting group - the reason a selection is +// made at all - still gets room beside them rather than scrolling out of view. +const COMPACT_BELOW_PX = 900; + +interface EditorTopBarProps { + controller: Controller; + /** True while the next page click drops a new text box. */ + addTextArmed: boolean; + onToggleAddText: () => void; + /** True while the find bar is showing, so the button can read as pressed. */ + findOpen: boolean; + onToggleFind: () => void; + onShowHelp: () => void; + /** False before a document is open: only the identity block makes sense. */ + hasDocument: boolean; + /** Unsaved-changes marker for the file chip. */ + dirty: boolean; +} + +/** + * Everything you do TO the document, in the place editors have always put it. + * + * The side panel used to carry the file you are editing, Insert, Find, help and + * Save, which meant the two most common verbs in the tool lived at the bottom + * of a scrolling panel on the far side of the screen from the page. They read + * left to right the way a document editor's toolbar does: what file this is, + * undo/redo, what you can add, what you can find, then the selection's own + * formatting. + * + * Saving is deliberately NOT here. Every other tool pins its primary action to + * the bottom of the side panel, and one tool answering "where is the button" + * differently from the rest is worse than the extra travel - see + * EditorPanelActions. + * + * Narrow bars fold insert, find and help into one overflow menu rather than + * scrolling them out of sight - see COMPACT_BELOW_PX. + */ +export function EditorTopBar({ + controller, + addTextArmed, + onToggleAddText, + findOpen, + onToggleFind, + onShowHelp, + hasDocument, + dirty, +}: EditorTopBarProps) { + const { t } = useTranslation(); + const session = useEditorSession(); + const hasSelection = controller.selectionCount > 0; + const barRef = useRef(null); + const barWidth = useElementWidth(barRef); + // null until the first measurement: start roomy so the bar does not flash + // through its compact form on mount. + const compact = barWidth !== null && barWidth < COMPACT_BELOW_PX; + + const addTextLabel = addTextArmed + ? t("pdfTextEditor.sidebar.clickPageToAddText", "Click page to add text") + : t("pdfTextEditor.sidebar.addText", "Add text"); + const addImageLabel = t("pdfTextEditor.sidebar.addImage", "Add image"); + const findLabel = t("pdfTextEditor.settings.find", "Find in document"); + const helpLabel = t("pdfTextEditor.help.ariaLabel", "Keyboard shortcuts"); + + return ( +
+
+ {session?.fileName ? ( + + ) : ( + + {t("pdfTextEditor.sidebar.noFile", "No file loaded")} + + )} +
+ + {hasDocument && ( +
+ + + + +
+ )} + +
+ {hasDocument && !compact && ( + +
+
+ ); +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.css b/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.css new file mode 100644 index 0000000000..c12ae18f03 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.css @@ -0,0 +1,100 @@ +/* Find & replace, laid out the way a code editor's is: a docked strip under + the toolbar, two rows, and hairlines that fence it off from the page. */ + +.pdf-editor-findbar { + display: flex; + align-items: flex-start; + gap: 0.5rem; + padding: 0.4375rem 0.625rem; + background: var(--c-bg-raised); + /* Both edges drawn: without the bottom rule the bar reads as part of the + page it is searching. The rule spans the full width; the controls do not. */ + border-bottom: 0.0625rem solid var(--c-border-subtle); + flex: 0 0 auto; +} + +/* One grid for BOTH rows, so the find and replace fields end at the same x + whatever the buttons beside them are called. Two flex rows could only agree + by hardcoding a width, which the first translation longer than "Replace all" + would break. */ +.pdf-editor-findbar__grid { + flex: 1 1 auto; + min-width: 0; + display: grid; + grid-template-columns: minmax(0, 1fr) auto auto; + align-items: center; + column-gap: 0.25rem; + row-gap: 0.3125rem; + /* Capped, not full-bleed: a search box stretched across a 1600px canvas + stops reading as a control. */ + max-width: 44rem; +} + +/* The toggles + match count that live inside the find field. */ +.pdf-editor-findbar__inline { + display: flex; + align-items: center; + gap: 0.125rem; + padding-right: 0.25rem; +} + +.pdf-editor-findbar__count { + font-size: 0.6875rem; + color: var(--c-text-muted); + white-space: nowrap; + padding-right: 0.25rem; + font-variant-numeric: tabular-nums; +} + +/* Square, quiet toggles - "Aa", "ab", "a" - pressed state filled. */ +.pdf-editor-findbar__toggle { + display: inline-flex; + align-items: center; + justify-content: center; + width: 1.375rem; + height: 1.375rem; + padding: 0; + border: 0; + border-radius: 0.25rem; + background: transparent; + color: var(--c-text-muted); + font-size: 0.6875rem; + font-weight: 600; + line-height: 1; + cursor: pointer; +} + +.pdf-editor-findbar__toggle--underlined { + text-decoration: underline; + text-underline-offset: 0.125rem; +} + +.pdf-editor-findbar__toggle:hover { + background: var(--c-hover); + color: var(--c-text); +} + +.pdf-editor-findbar__toggle[aria-pressed="true"] { + background: var(--c-primary); + color: var(--c-text-on-primary); +} + +/* Vertical hairline between each field and its buttons. */ +.pdf-editor-findbar__divider { + width: 0.0625rem; + align-self: stretch; + margin: 0.125rem 0.1875rem; + background: var(--c-border-strong); +} + +.pdf-editor-findbar__actions { + display: flex; + align-items: center; + gap: 0.25rem; +} + +/* Pinned to the bar's own right edge, clear of the capped control grid. */ +.pdf-editor-findbar__close { + flex: 0 0 auto; + margin-left: auto; +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.tsx index 4971464710..c4e1c42f6a 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.tsx +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/FindBar.tsx @@ -1,8 +1,11 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { Group, Stack, Text, TextInput, Tooltip } from "@mantine/core"; +import { TextInput, Tooltip } from "@mantine/core"; import { Button } from "@app/ui/Button"; import { useTranslation } from "react-i18next"; import CloseIcon from "@mui/icons-material/Close"; +import KeyboardArrowUpIcon from "@mui/icons-material/KeyboardArrowUp"; +import KeyboardArrowDownIcon from "@mui/icons-material/KeyboardArrowDown"; +import "@app/tools/pdfTextEditor/components/FindBar.css"; import { EditTextCommand } from "@app/tools/pdfTextEditor/commands/EditTextCommand"; import { CompositeCommand } from "@app/tools/pdfTextEditor/commands/CompositeCommand"; import { @@ -194,158 +197,212 @@ export function FindBar({ store, pages, onClose }: FindBarProps) { setReplaceCount(n); }, [query, replace, matches, store]); + const countLabel = + matches.length === 0 + ? query + ? t("pdfTextEditor.find.noMatches", "No matches") + : "" + : t("pdfTextEditor.find.count", "{{current}} of {{total}}", { + current: activeIndex + 1, + total: matches.length, + }); + + // "Aa" and "ab" would read as the same two letters at 11px, so whole-word + // borrows the underline every code editor draws under its own version. + const toggles: Array<{ + on: boolean; + set: () => void; + label: string; + glyph: string; + underline?: boolean; + testId: string; + }> = [ + { + on: matchCase, + set: () => setMatchCase((v) => !v), + label: t("pdfTextEditor.find.matchCase", "Match case"), + glyph: "Aa", + testId: "pdf-editor-find-match-case", + }, + { + on: wholeWord, + set: () => setWholeWord((v) => !v), + label: t("pdfTextEditor.find.wholeWord", "Whole word"), + glyph: "ab", + underline: true, + testId: "pdf-editor-find-whole-word", + }, + { + on: ignoreAccents, + set: () => setIgnoreAccents((v) => !v), + label: t("pdfTextEditor.find.ignoreAccents", "Ignore accents"), + glyph: "\u00e1", + testId: "pdf-editor-find-ignore-accents", + }, + ]; + return ( - - - - {t("pdfTextEditor.find.title", "Find & replace")} - +
+
+ setQuery(e.currentTarget.value)} + placeholder={t("pdfTextEditor.find.findPlaceholder", "Find")} + aria-label={t("pdfTextEditor.find.findPlaceholder", "Find")} + data-testid="pdf-editor-find-input" + size="xs" + // The count and the toggles live INSIDE the field, the way every code + // editor puts them - the row stays one line however many are added. + rightSectionWidth={132} + // Mantine makes a section inert by default so clicks reach the input. + // These are real buttons, so they have to take their own clicks. + rightSectionPointerEvents="all" + rightSection={ + + + {countLabel} + + {toggles.map((toggle) => ( + + + + ))} + + } + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + if (e.shiftKey) prev(); + else next(); + } else if (e.key === "Escape") { + e.preventDefault(); + onClose(); + } + }} + /> + +
+ +
+ + setReplace(e.currentTarget.value)} + placeholder={t( + "pdfTextEditor.find.replacePlaceholder", + "Replace with", + )} + aria-label={t( + "pdfTextEditor.find.replacePlaceholder", + "Replace with", + )} + data-testid="pdf-editor-replace-input" + size="xs" + rightSectionWidth={132} + rightSection={ + replaceCount !== null ? ( + + + {t("pdfTextEditor.find.replaced", "{{count}} replaced", { + count: replaceCount, + })} + + + ) : null + } + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + if (e.shiftKey) doReplaceAll(); + else doReplaceOne(); + } else if (e.key === "Escape") { + e.preventDefault(); + onClose(); + } + }} + /> + +
+ + +
+
+ + {/* Dismiss sits at the far corner of the bar, not beside the match + arrows: next to them it read as a third way to navigate. */} + - - - - - - - - - - - {matches.length === 0 - ? query - ? t("pdfTextEditor.find.noMatches", "No matches") - : t("pdfTextEditor.find.typeToSearch", "Type to search") - : t("pdfTextEditor.find.count", "{{current}} of {{total}}", { - current: activeIndex + 1, - total: matches.length, - })} - {replaceCount !== null - ? t("pdfTextEditor.find.replaced", " · {{count}} replaced", { - count: replaceCount, - }) - : ""} - - - - - - - + +
); } diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.css b/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.css new file mode 100644 index 0000000000..d1131bd53a --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.css @@ -0,0 +1,44 @@ +/* Shortcut reference: one card per section, one row per shortcut, hairlines + between rows so a long list stays scannable. */ + +.pdf-editor-help__section-title { + font-size: 0.9375rem; + font-weight: 600; + margin: 0 0 0.5rem; +} + +.pdf-editor-help__section-hint { + font-size: 0.8125rem; + color: var(--c-text-muted); + margin: -0.25rem 0 0.5rem; +} + +.pdf-editor-help__card { + border: 0.0625rem solid var(--c-border-subtle); + border-radius: 0.5rem; + background: var(--c-surface-raised); + overflow: hidden; +} + +.pdf-editor-help__row { + display: flex; + align-items: center; + justify-content: space-between; + gap: 1rem; + padding: 0.625rem 0.875rem; +} + +.pdf-editor-help__row + .pdf-editor-help__row { + border-top: 0.0625rem solid var(--c-border-subtle); +} + +.pdf-editor-help__label { + font-size: 0.875rem; + min-width: 0; +} + +/* The keys column never shrinks - a wrapped description is fine, a wrapped + shortcut is not. */ +.pdf-editor-help__keys { + flex: 0 0 auto; +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.tsx index 7901291f64..27efec5460 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.tsx +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/HelpOverlay.tsx @@ -1,287 +1,216 @@ -import { Modal, Stack, Text, Group, Kbd, Divider } from "@mantine/core"; +import { Modal, Stack, Text } from "@mantine/core"; import { useTranslation } from "react-i18next"; +import { KeyCombo } from "@app/ui/KeyCombo"; +import "@app/tools/pdfTextEditor/components/HelpOverlay.css"; interface HelpOverlayProps { opened: boolean; onClose: () => void; } -/** Keyboard + click reference for the PDF text/image editor. */ +interface Shortcut { + /** Keys exactly as pressed - see the spacing rule below. */ + keys: string; + description: string; +} + +interface Section { + heading: string; + items: Shortcut[]; +} + +/** + * Keyboard shortcuts for the PDF text/image editor - and only those. + * + * Nothing here describes a toolbar control, a mouse gesture, or what a key does + * inside a field it is already obvious in. A shortcut list people trust is one + * they can scan for a key they did not know about; padding it with "Font family + * - swap to another font" or "Drag - move image" turns it into a feature tour + * and buries the handful of things it exists to tell you. + * + * Every `keys` string is written as separate keys joined by " + ", never as a + * run-on like "Ctrl+Click": KeyCombo splits on that separator to give each key + * its own tile, so the spacing is what keeps the whole list consistent. Where a + * shortcut has alternatives, separate them with " / ". + */ export function HelpOverlay({ opened, onClose }: HelpOverlayProps) { const { t } = useTranslation(); - const SHORTCUTS: Array<{ - heading: string; - items: Array<[string, string]>; - }> = [ + const SHORTCUTS: Section[] = [ { heading: t("pdfTextEditor.help.editing.heading", "Editing"), items: [ - [ - t("pdfTextEditor.help.editing.clickKey", "Click"), - t("pdfTextEditor.help.editing.clickDesc", "Edit text"), - ], - [ - t( + { + keys: t( "pdfTextEditor.help.editing.shiftClickKey", - "Ctrl+Click / Shift+Click", + "Ctrl + Click / Shift + Click", ), - t( + description: t( "pdfTextEditor.help.editing.shiftClickDesc", "Add / remove a run from selection", ), - ], - [ - t("pdfTextEditor.help.editing.marqueeKey", "Ctrl+Shift+drag"), - t("pdfTextEditor.help.editing.marqueeDesc", "Marquee multi-select"), - ], - [ - t("pdfTextEditor.help.editing.groupKey", "Ctrl+M"), - t( + }, + { + keys: t( + "pdfTextEditor.help.editing.marqueeKey", + "Ctrl + Shift + Drag", + ), + description: t( + "pdfTextEditor.help.editing.marqueeDesc", + "Marquee multi-select", + ), + }, + { + keys: t("pdfTextEditor.help.editing.moveKey", "Ctrl + Click + Drag"), + description: t( + "pdfTextEditor.help.editing.moveDesc", + "Move text run", + ), + }, + { + keys: t("pdfTextEditor.help.editing.selectAllKey", "Ctrl + A"), + description: t( + "pdfTextEditor.help.editing.selectAllDesc", + "Select all", + ), + }, + { + keys: t("pdfTextEditor.help.editing.groupKey", "Ctrl + M"), + description: t( "pdfTextEditor.help.editing.groupDesc", - "Group selected runs (Group button)", + "Group selected runs into one paragraph", ), - ], - [ - t("pdfTextEditor.help.editing.ungroupKey", "-"), - t( - "pdfTextEditor.help.editing.ungroupDesc", - "Ungroup paragraph: select it, click Ungroup", + }, + { + keys: t("pdfTextEditor.help.editing.duplicateKey", "Ctrl + D"), + description: t( + "pdfTextEditor.help.editing.duplicateDesc", + "Duplicate selected", ), - ], - [ - t("pdfTextEditor.help.editing.moveKey", "Ctrl+Click + drag"), - t("pdfTextEditor.help.editing.moveDesc", "Move text run"), - ], - [ - t("pdfTextEditor.help.editing.deleteKey", "Delete"), - t("pdfTextEditor.help.editing.deleteDesc", "Remove selected"), - ], - [ - t("pdfTextEditor.help.editing.duplicateKey", "Ctrl+D"), - t("pdfTextEditor.help.editing.duplicateDesc", "Duplicate selected"), - ], - [ - t("pdfTextEditor.help.editing.selectAllKey", "Ctrl+A"), - t("pdfTextEditor.help.editing.selectAllDesc", "Select all"), - ], - [ - t("pdfTextEditor.help.editing.undoRedoKey", "Ctrl+Z / Ctrl+Y"), - t("pdfTextEditor.help.editing.undoRedoDesc", "Undo / Redo"), - ], + }, + { + keys: t("pdfTextEditor.help.editing.deleteKey", "Delete"), + description: t( + "pdfTextEditor.help.editing.deleteDesc", + "Remove selected", + ), + }, + { + keys: t( + "pdfTextEditor.help.editing.undoRedoKey", + "Ctrl + Z / Ctrl + Y", + ), + description: t( + "pdfTextEditor.help.editing.undoRedoDesc", + "Undo / Redo", + ), + }, ], }, { heading: t("pdfTextEditor.help.clipboard.heading", "Clipboard"), items: [ - [ - t("pdfTextEditor.help.clipboard.copyKey", "Ctrl+C"), - t("pdfTextEditor.help.clipboard.copyDesc", "Copy selected text"), - ], - [ - t("pdfTextEditor.help.clipboard.cutKey", "Ctrl+X"), - t( + { + keys: t("pdfTextEditor.help.clipboard.copyKey", "Ctrl + C"), + description: t( + "pdfTextEditor.help.clipboard.copyDesc", + "Copy selected text", + ), + }, + { + keys: t("pdfTextEditor.help.clipboard.cutKey", "Ctrl + X"), + description: t( "pdfTextEditor.help.clipboard.cutDesc", "Cut selected (copy + delete)", ), - ], - [ - t("pdfTextEditor.help.clipboard.pasteKey", "Ctrl+V"), - t( + }, + { + keys: t("pdfTextEditor.help.clipboard.pasteKey", "Ctrl + V"), + description: t( "pdfTextEditor.help.clipboard.pasteDesc", "Paste clipboard text as new run", ), - ], - [ - t("pdfTextEditor.help.clipboard.pastePlainKey", "Ctrl+Shift+V"), - t( + }, + { + keys: t( + "pdfTextEditor.help.clipboard.pastePlainKey", + "Ctrl + Shift + V", + ), + description: t( "pdfTextEditor.help.clipboard.pastePlainDesc", "Paste as plain text", ), - ], + }, ], }, { - heading: t("pdfTextEditor.help.find.heading", "Find & Replace"), + heading: t("pdfTextEditor.help.find.heading", "Find & replace"), items: [ - [ - t("pdfTextEditor.help.find.openKey", "Ctrl+F"), - t("pdfTextEditor.help.find.openDesc", "Open find bar (and replace)"), - ], - [ - t("pdfTextEditor.help.find.nextKey", "F3 / Ctrl+G"), - t( + { + keys: t("pdfTextEditor.help.find.openKey", "Ctrl + F"), + description: t( + "pdfTextEditor.help.find.openDesc", + "Open find bar (and replace)", + ), + }, + { + keys: t("pdfTextEditor.help.find.nextKey", "F3 / Ctrl + G"), + description: t( "pdfTextEditor.help.find.nextDesc", - "Next match (Shift = previous)", + "Next match (add Shift for previous)", ), - ], - [ - t("pdfTextEditor.help.find.enterFindKey", "Enter (in find)"), - t("pdfTextEditor.help.find.enterFindDesc", "Next match"), - ], - [ - t("pdfTextEditor.help.find.enterReplaceKey", "Enter (in replace)"), - t( - "pdfTextEditor.help.find.enterReplaceDesc", - "Replace one (Shift = Replace All)", - ), - ], - ], - }, - { - heading: t( - "pdfTextEditor.help.arrangement.heading", - "Object arrangement", - ), - items: [ - [ - t("pdfTextEditor.help.arrangement.orderKey", "Toolbar ↑ ↓"), - t( - "pdfTextEditor.help.arrangement.orderDesc", - "Bring forward / send backward (one step)", - ), - ], - [ - t( - "pdfTextEditor.help.arrangement.frontBackKey", - "Toolbar front/back", - ), - t( - "pdfTextEditor.help.arrangement.frontBackDesc", - "Bring to front / send to back", - ), - ], - [ - t("pdfTextEditor.help.arrangement.alignKey", "Toolbar align"), - t( - "pdfTextEditor.help.arrangement.alignDesc", - "Align edges L / centre / R / T / mid / B", - ), - ], - [ - t( - "pdfTextEditor.help.arrangement.distributeKey", - "Toolbar distribute", - ), - t( - "pdfTextEditor.help.arrangement.distributeDesc", - "Equal horizontal / vertical spacing (3+)", - ), - ], - [ - t("pdfTextEditor.help.arrangement.lockKey", "Lock button"), - t( - "pdfTextEditor.help.arrangement.lockDesc", - "Lock / unlock selection (session-only)", - ), - ], - ], - }, - { - heading: t("pdfTextEditor.help.image.heading", "Image"), - items: [ - [ - t("pdfTextEditor.help.image.rotateKey", "Toolbar rotate"), - t( - "pdfTextEditor.help.image.rotateDesc", - "Rotate 90° clockwise or counter-clockwise", - ), - ], - [ - t("pdfTextEditor.help.image.flipKey", "Toolbar flip"), - t( - "pdfTextEditor.help.image.flipDesc", - "Flip horizontally or vertically", - ), - ], - [ - t("pdfTextEditor.help.image.resizeKey", "Corner drag"), - t("pdfTextEditor.help.image.resizeDesc", "Resize image"), - ], - [ - t("pdfTextEditor.help.image.moveKey", "Drag"), - t("pdfTextEditor.help.image.moveDesc", "Move image"), - ], - ], - }, - { - heading: t("pdfTextEditor.help.formatting.heading", "Text formatting"), - items: [ - [ - t("pdfTextEditor.help.formatting.italicKey", "Toolbar I"), - t("pdfTextEditor.help.formatting.italicDesc", "Italic"), - ], - [ - t("pdfTextEditor.help.formatting.fontSizeKey", "Toolbar font size"), - t("pdfTextEditor.help.formatting.fontSizeDesc", "Change font size"), - ], - [ - t("pdfTextEditor.help.formatting.colourKey", "Toolbar colour"), - t("pdfTextEditor.help.formatting.colourDesc", "Change fill colour"), - ], - [ - t( - "pdfTextEditor.help.formatting.fontFamilyKey", - "Toolbar font family", - ), - t( - "pdfTextEditor.help.formatting.fontFamilyDesc", - "Swap to base-14 font", - ), - ], - [ - t("pdfTextEditor.help.formatting.caseKey", "Toolbar case (Aa)"), - t( - "pdfTextEditor.help.formatting.caseDesc", - "Change case (upper/lower/title/sentence)", - ), - ], + }, ], }, { heading: t("pdfTextEditor.help.navigation.heading", "Navigation"), items: [ - [ - t("pdfTextEditor.help.navigation.pageKey", "PageDown / PageUp"), - t("pdfTextEditor.help.navigation.pageDesc", "Next / previous page"), - ], - [ - t( + { + keys: t("pdfTextEditor.help.navigation.pageKey", "PageUp / PageDown"), + description: t( + "pdfTextEditor.help.navigation.pageDesc", + "Previous / next page", + ), + }, + { + keys: t( "pdfTextEditor.help.navigation.firstLastKey", - "Ctrl+Home / Ctrl+End", + "Ctrl + Home / Ctrl + End", ), - t("pdfTextEditor.help.navigation.firstLastDesc", "First / last page"), - ], - [ - t("pdfTextEditor.help.navigation.zoomKey", "Ctrl+Wheel"), - t("pdfTextEditor.help.navigation.zoomDesc", "Zoom in / out"), - ], - [ - t("pdfTextEditor.help.navigation.toolbarZoomKey", "Toolbar zoom"), - t( - "pdfTextEditor.help.navigation.toolbarZoomDesc", - "Manual zoom + Fit to width", + description: t( + "pdfTextEditor.help.navigation.firstLastDesc", + "First / last page", ), - ], + }, + { + keys: t("pdfTextEditor.help.navigation.zoomKey", "Ctrl + Wheel"), + description: t( + "pdfTextEditor.help.navigation.zoomDesc", + "Zoom in / out", + ), + }, ], }, { heading: t("pdfTextEditor.help.document.heading", "Document"), items: [ - [ - t("pdfTextEditor.help.document.saveKey", "Ctrl+S"), - t("pdfTextEditor.help.document.saveDesc", "Save to your workspace"), - ], - [ - t("pdfTextEditor.help.document.helpKey", "? / F1"), - t("pdfTextEditor.help.document.helpDesc", "This help"), - ], - [ - t("pdfTextEditor.help.document.escKey", "Esc"), - t( + { + keys: t("pdfTextEditor.help.document.saveKey", "Ctrl + S"), + description: t( + "pdfTextEditor.help.document.saveDesc", + "Save to your workspace", + ), + }, + { + keys: t("pdfTextEditor.help.document.helpKey", "? / F1"), + description: t("pdfTextEditor.help.document.helpDesc", "This help"), + }, + { + keys: t("pdfTextEditor.help.document.escKey", "Esc"), + description: t( "pdfTextEditor.help.document.escDesc", "Clear selection / close find / close help", ), - ], + }, ], }, ]; @@ -291,29 +220,31 @@ export function HelpOverlay({ opened, onClose }: HelpOverlayProps) { opened={opened} onClose={onClose} title={t("pdfTextEditor.help.title", "Keyboard shortcuts")} - size="md" + size="lg" data-testid="pdf-editor-help-overlay" > - - {SHORTCUTS.map((section, sectionIdx) => ( - - {sectionIdx > 0 && } - + + {SHORTCUTS.map((section) => ( +
+

{section.heading} - - {section.items.map(([keys, desc]) => ( - - - {desc} - - {keys} - - ))} - +

+
+ {section.items.map((item) => ( +
+ + {item.description} + + + + +
+ ))} +
+
))} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/PageStage.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/PageStage.tsx index 5b2dc61c69..932efa38d8 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/components/PageStage.tsx +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/PageStage.tsx @@ -11,7 +11,8 @@ import { import { useTranslation } from "react-i18next"; import { useEditorStore } from "@app/tools/pdfTextEditor/hooks/useEditorStore"; import { ensurePageRead } from "@app/tools/pdfTextEditor/hooks/useDocumentLoader"; -import { Toolbar } from "@app/tools/pdfTextEditor/components/Toolbar"; +import { EditorTopBar } from "@app/tools/pdfTextEditor/components/EditorTopBar"; +import { FindBar } from "@app/tools/pdfTextEditor/components/FindBar"; import { useToolbarController } from "@app/tools/pdfTextEditor/hooks/useToolbarController"; import { ZoomPill } from "@app/tools/pdfTextEditor/components/ZoomPill"; import { MarqueeSelector } from "@app/tools/pdfTextEditor/components/MarqueeSelector"; @@ -69,12 +70,30 @@ export function PageStage() { // inspector derives from the same controller, so both surfaces read one // source of truth. const controller = useToolbarController(store, state, selection); - const toolbar = { controller }; + // One element, rendered in both the empty and loaded branches. Find/help + // open from here, so their panels are rendered here too rather than in the + // side panel the user is not looking at. + const topBar = ( + + store.setMode( + store.getState().mode === "addText" ? "select" : "addText", + ) + } + findOpen={state.findOpen} + onToggleFind={() => store.setFindOpen(!store.getState().findOpen)} + onShowHelp={() => store.setHelpOpen(true)} + /> + ); if (!state.hasDocument && !state.loading) { return ( - + {topBar}
- + {topBar} + {state.findOpen && state.hasDocument && ( + store.setFindOpen(false)} + /> + )} f.type === "application/pdf" || /\.pdf$/i.test(f.name), ); if (!pdf) return; - // Replacing the open document discards in-progress edits - confirm - // first when dirty, so an accidental drop can't silently lose work. - if ( - store.getState().dirty && - !window.confirm( - t( - "pdfTextEditor.confirmReplaceDirty", - "You have unsaved changes. Replace the open document and discard them?", - ), - ) - ) { - return; - } + // Replacing the open document discards in-progress edits. The panel + // owns that confirmation (DiscardChangesModal) for every route in, + // so the drop just hands the file over. const input = document.querySelector( '[data-testid="pdf-editor-file-input"]', ); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/Toolbar.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/Toolbar.tsx deleted file mode 100644 index a9cbb18a63..0000000000 --- a/frontend/editor/src/core/tools/pdfTextEditor/components/Toolbar.tsx +++ /dev/null @@ -1,578 +0,0 @@ -import { useState } from "react"; -import { - ColorInput, - Group, - Menu, - NumberInput, - Popover, - Text, - Tooltip, -} from "@mantine/core"; -import { Button } from "@app/ui/Button"; -import UndoIcon from "@mui/icons-material/Undo"; -import RedoIcon from "@mui/icons-material/Redo"; -import DeleteIcon from "@mui/icons-material/DeleteOutlined"; -import FormatItalicIcon from "@mui/icons-material/FormatItalic"; -import TuneIcon from "@mui/icons-material/TuneOutlined"; -import LockIcon from "@mui/icons-material/LockOutlined"; -import LockOpenIcon from "@mui/icons-material/LockOpenOutlined"; -import TextFieldsIcon from "@mui/icons-material/TextFields"; -import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; -import LayersIcon from "@mui/icons-material/LayersOutlined"; -import FlipToFrontIcon from "@mui/icons-material/FlipToFrontOutlined"; -import FlipToBackIcon from "@mui/icons-material/FlipToBackOutlined"; -import ArrowUpwardIcon from "@mui/icons-material/ArrowUpward"; -import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; -import VerticalAlignTopIcon from "@mui/icons-material/VerticalAlignTop"; -import VerticalAlignBottomIcon from "@mui/icons-material/VerticalAlignBottom"; -import VerticalAlignCenterIcon from "@mui/icons-material/VerticalAlignCenter"; -import AlignHorizontalLeftIcon from "@mui/icons-material/AlignHorizontalLeftOutlined"; -import AlignHorizontalCenterIcon from "@mui/icons-material/AlignHorizontalCenterOutlined"; -import AlignHorizontalRightIcon from "@mui/icons-material/AlignHorizontalRightOutlined"; -import LinearScaleIcon from "@mui/icons-material/LinearScaleOutlined"; -import { useTranslation } from "react-i18next"; -import { parseCssColor, toCssHex } from "@app/tools/pdfTextEditor/model/Color"; -import { familyOf } from "@app/tools/pdfTextEditor/util/fontFamily"; -import { FontFamilySelect } from "@app/tools/pdfTextEditor/components/FontFamilySelect"; -import type { useToolbarController } from "@app/tools/pdfTextEditor/hooks/useToolbarController"; - -type Controller = ReturnType; - -/** - * The canvas toolbar: undo/redo, plus formatting for the current selection. - * - * Character formatting sits here rather than in the side panel because that is - * where every document editor puts it. The group is *contextual* - it appears - * with a selection instead of standing permanently greyed - which is what - * keeps the strip to a single row. - */ -interface ToolbarProps { - controller: Controller; -} - -function ToolbarSeparator() { - return ( - - | - - ); -} - -/** Toolbar children keep their natural width; the strip scrolls if pressed. */ -const NO_SHRINK = { flexShrink: 0 } as const; - -export function Toolbar({ controller }: ToolbarProps) { - const { t } = useTranslation(); - const hasSelection = controller.selectionCount > 0; - return ( - - - - - - {t("pdfTextEditor.toolbar.order", "Order")} - } - onClick={() => onChangeZOrder("to-front")} - data-testid="pdf-editor-z-to-front" - > - {t("pdfTextEditor.toolbar.bringToFront", "Bring to front")} - - } - onClick={() => onChangeZOrder("forward")} - data-testid="pdf-editor-z-forward" - > - {t("pdfTextEditor.toolbar.bringForward", "Bring forward")} - - } - onClick={() => onChangeZOrder("backward")} - data-testid="pdf-editor-z-backward" - > - {t("pdfTextEditor.toolbar.sendBackward", "Send backward")} - - } - onClick={() => onChangeZOrder("to-back")} - data-testid="pdf-editor-z-to-back" - > - {t("pdfTextEditor.toolbar.sendToBack", "Send to back")} - - - - {t("pdfTextEditor.toolbar.alignLabel", "Align · needs 2+ objects")} - - } - disabled={hAlignDisabled} - onClick={() => onAlign("left")} - data-testid="pdf-editor-align-left" - > - {t("pdfTextEditor.toolbar.alignLeft", "Align left")} - - } - disabled={hAlignDisabled} - onClick={() => onAlign("center-h")} - data-testid="pdf-editor-align-center-h" - > - {t("pdfTextEditor.toolbar.alignCentre", "Align centre")} - - } - disabled={hAlignDisabled} - onClick={() => onAlign("right")} - data-testid="pdf-editor-align-right" - > - {t("pdfTextEditor.toolbar.alignRight", "Align right")} - - } - disabled={alignDisabled} - onClick={() => onAlign("top")} - data-testid="pdf-editor-align-top" - > - {t("pdfTextEditor.toolbar.alignTop", "Align top")} - - } - disabled={alignDisabled} - onClick={() => onAlign("middle-v")} - data-testid="pdf-editor-align-middle-v" - > - {t("pdfTextEditor.toolbar.alignMiddle", "Align middle")} - - } - disabled={alignDisabled} - onClick={() => onAlign("bottom")} - data-testid="pdf-editor-align-bottom" - > - {t("pdfTextEditor.toolbar.alignBottom", "Align bottom")} - - - - {t( - "pdfTextEditor.toolbar.distributeLabel", - "Distribute · needs 3+ objects", - )} - - } - disabled={distributeDisabled} - onClick={() => onDistribute("horizontal")} - data-testid="pdf-editor-distribute-h" - > - {t( - "pdfTextEditor.toolbar.distributeHorizontally", - "Distribute horizontally", - )} - - - } - disabled={distributeDisabled} - onClick={() => onDistribute("vertical")} - data-testid="pdf-editor-distribute-v" - > - {t( - "pdfTextEditor.toolbar.distributeVertically", - "Distribute vertically", - )} - - - - - ); -} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/FormatGroup.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/FormatGroup.tsx new file mode 100644 index 0000000000..3fb7f29757 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/FormatGroup.tsx @@ -0,0 +1,277 @@ +import { useState } from "react"; +import { + ColorInput, + Group, + Menu, + NumberInput, + Popover, + Tooltip, +} from "@mantine/core"; +import { useTranslation } from "react-i18next"; +import { Button } from "@app/ui/Button"; +import FormatItalicIcon from "@mui/icons-material/FormatItalic"; +import TuneIcon from "@mui/icons-material/TuneOutlined"; +import TextFieldsIcon from "@mui/icons-material/TextFields"; +import { parseCssColor, toCssHex } from "@app/tools/pdfTextEditor/model/Color"; +import { familyOf } from "@app/tools/pdfTextEditor/util/fontFamily"; +import { FontFamilySelect } from "@app/tools/pdfTextEditor/components/FontFamilySelect"; +import { + NO_SHRINK, + type Controller, +} from "@app/tools/pdfTextEditor/components/toolbar/toolbarShared"; + +/** Character formatting. Text runs only - absent for a pure image selection. */ +export function FormatGroup({ controller }: { controller: Controller }) { + const { t } = useTranslation(); + const { + state, + hasRunSelection, + onChangeFontFamily, + onChangeFontSize, + onChangeFill, + onChangeOutline, + onToggleItalic, + onChangeCase, + } = controller; + // Mantine only closes the fill picker's dropdown on blur, and it is + // portalled over the page - so we drive it and close it on a commit. + const [fillPickerOpen, setFillPickerOpen] = useState(false); + if (!hasRunSelection) return null; + + const fillHex = state.fill ? toCssHex(state.fill) : "#000000"; + const outlineHex = state.stroke ? toCssHex(state.stroke) : "#000000"; + const outlineWidth = state.strokeWidth ?? 0; + const fontFamily = state.fontFamily ? familyOf(state.fontFamily) : null; + + return ( + <> + + { + const next = typeof value === "number" ? value : Number(value); + if (Number.isFinite(next) && next > 0) onChangeFontSize(next); + }} + aria-label={t("pdfTextEditor.toolbar.fontSize", "Font size")} + data-testid="pdf-editor-font-size" + style={NO_SHRINK} + /> + { + if (!next) return; + const rgb = parseCssColor(next); + if (!rgb) return; + // Re-emitting the applied colour must not cost a second undo step. + // Mixed fills (state.fill null) still apply - that unifies them. + if ( + state.fill && + rgb.r === state.fill.r && + rgb.g === state.fill.g && + rgb.b === state.fill.b + ) { + return; + } + onChangeFill(next); + }} + // Drag end, swatch click or a committed hex - the deliberate pick. + // Deferred a frame: a saturation-square pick delivers its value on the + // NEXT animation frame (use-move), so closing now unmounts the picker + // first and Firefox loses the pick against a detached 0x0 node. + onChangeEnd={() => + window.requestAnimationFrame(() => setFillPickerOpen(false)) + } + onFocus={() => setFillPickerOpen(true)} + onClick={() => setFillPickerOpen(true)} + onBlur={() => setFillPickerOpen(false)} + onKeyDown={(event) => { + // The dropdown's own Escape handler never sees the key - focus stays + // in the input - so dismiss it here. + if (event.key === "Escape" || event.key === "Enter") { + setFillPickerOpen(false); + } + }} + popoverProps={{ + opened: fillPickerOpen, + onChange: setFillPickerOpen, + transitionProps: { transition: "fade", duration: 0 }, + }} + aria-label={t("pdfTextEditor.toolbar.fontColour", "Font colour")} + data-testid="pdf-editor-colour" + style={NO_SHRINK} + /> + + + + + + + {t("pdfTextEditor.toolbar.order", "Order")} + } + onClick={() => onChangeZOrder("to-front")} + data-testid="pdf-editor-z-to-front" + > + {t("pdfTextEditor.toolbar.bringToFront", "Bring to front")} + + } + onClick={() => onChangeZOrder("forward")} + data-testid="pdf-editor-z-forward" + > + {t("pdfTextEditor.toolbar.bringForward", "Bring forward")} + + } + onClick={() => onChangeZOrder("backward")} + data-testid="pdf-editor-z-backward" + > + {t("pdfTextEditor.toolbar.sendBackward", "Send backward")} + + } + onClick={() => onChangeZOrder("to-back")} + data-testid="pdf-editor-z-to-back" + > + {t("pdfTextEditor.toolbar.sendToBack", "Send to back")} + + + + {t("pdfTextEditor.toolbar.alignLabel", "Align · needs 2+ objects")} + + } + disabled={hAlignDisabled} + onClick={() => onAlign("left")} + data-testid="pdf-editor-align-left" + > + {t("pdfTextEditor.toolbar.alignLeft", "Align left")} + + } + disabled={hAlignDisabled} + onClick={() => onAlign("center-h")} + data-testid="pdf-editor-align-center-h" + > + {t("pdfTextEditor.toolbar.alignCentre", "Align centre")} + + } + disabled={hAlignDisabled} + onClick={() => onAlign("right")} + data-testid="pdf-editor-align-right" + > + {t("pdfTextEditor.toolbar.alignRight", "Align right")} + + } + disabled={alignDisabled} + onClick={() => onAlign("top")} + data-testid="pdf-editor-align-top" + > + {t("pdfTextEditor.toolbar.alignTop", "Align top")} + + } + disabled={alignDisabled} + onClick={() => onAlign("middle-v")} + data-testid="pdf-editor-align-middle-v" + > + {t("pdfTextEditor.toolbar.alignMiddle", "Align middle")} + + } + disabled={alignDisabled} + onClick={() => onAlign("bottom")} + data-testid="pdf-editor-align-bottom" + > + {t("pdfTextEditor.toolbar.alignBottom", "Align bottom")} + + + + {t( + "pdfTextEditor.toolbar.distributeLabel", + "Distribute · needs 3+ objects", + )} + + } + disabled={distributeDisabled} + onClick={() => onDistribute("horizontal")} + data-testid="pdf-editor-distribute-h" + > + {t( + "pdfTextEditor.toolbar.distributeHorizontally", + "Distribute horizontally", + )} + + + } + disabled={distributeDisabled} + onClick={() => onDistribute("vertical")} + data-testid="pdf-editor-distribute-v" + > + {t( + "pdfTextEditor.toolbar.distributeVertically", + "Distribute vertically", + )} + + + + + ); +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/toolbarShared.tsx b/frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/toolbarShared.tsx new file mode 100644 index 0000000000..f1639d84ae --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/components/toolbar/toolbarShared.tsx @@ -0,0 +1,22 @@ +import type { useToolbarController } from "@app/tools/pdfTextEditor/hooks/useToolbarController"; + +export type Controller = ReturnType; + +/** Toolbar children keep their natural width; the strip scrolls if pressed. */ +export const NO_SHRINK = { flexShrink: 0 } as const; + +/** Hairline between two groups of toolbar controls. */ +export function ToolbarSeparator() { + return ( + + ); +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/hooks/useElementWidth.ts b/frontend/editor/src/core/tools/pdfTextEditor/hooks/useElementWidth.ts new file mode 100644 index 0000000000..f1e973fe76 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/hooks/useElementWidth.ts @@ -0,0 +1,35 @@ +import { useEffect, useState, type RefObject } from "react"; + +/** + * The element's own width in pixels, tracked as it resizes. + * + * A CSS container query can hide a control that does not fit, but it cannot + * move one into a menu - that is a change of markup, not of style. So the one + * place the toolbar needs to make a structural decision has to measure. + * + * Returns null until the first observation, so a caller can render its roomy + * layout rather than flashing the compact one on mount. + */ +export function useElementWidth( + ref: RefObject, +): number | null { + const [width, setWidth] = useState(null); + + useEffect(() => { + const el = ref.current; + if (!el) return; + if (typeof ResizeObserver === "undefined") { + // No observer (jsdom, very old engines): measure once and stay there. + setWidth(el.getBoundingClientRect().width); + return; + } + const observer = new ResizeObserver((entries) => { + const entry = entries[0]; + if (entry) setWidth(entry.contentRect.width); + }); + observer.observe(el); + return () => observer.disconnect(); + }, [ref]); + + return width; +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumSave.ts b/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumSave.ts index 4f1c308eef..d292d4e971 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumSave.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumSave.ts @@ -1,4 +1,5 @@ import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; +import { assertSavedPdf } from "@app/tools/pdfTextEditor/util/savedBytes"; /** `FPDF_SaveAsCopy` flags. */ const FPDF_INCREMENTAL = 1; @@ -40,15 +41,34 @@ export class PdfiumSave { ); } + // The writer the shim hands back is the FPDF_FILEWRITE the flagged + // entry point expects, so incremental mode needs no extra plumbing. + const withFlags = (m as unknown as SaveFlagsModule).FPDF_SaveAsCopy; + // Falling back to the unflagged entry point would REWRITE a document the + // caller asked to append to - the one thing incremental mode exists to + // prevent - so refuse rather than quietly break its signatures. + if (options.incremental && typeof withFlags !== "function") { + throw new Error( + "This PDF needs a signature-preserving incremental save, but the " + + "PDFium build in use has no FPDF_SaveAsCopy entry point; save " + + "aborted rather than rewriting the signed revision.", + ); + } + const writerPtr = m.PDFiumExt_OpenFileWriter(); try { - // The writer the shim hands back is the FPDF_FILEWRITE the flagged - // entry point expects, so incremental mode needs no extra plumbing. - const withFlags = (m as unknown as SaveFlagsModule).FPDF_SaveAsCopy; - if (options.incremental && typeof withFlags === "function") { - withFlags(doc.docPtr, writerPtr, FPDF_INCREMENTAL); - } else { - m.PDFiumExt_SaveAsCopy(doc.docPtr, writerPtr); + // PDFium reports a failed save by return code, not by throwing. Ignoring + // it meant a failure produced an empty/partial buffer that the editor + // then wrote over the user's file. + const ok = + options.incremental && typeof withFlags === "function" + ? withFlags(doc.docPtr, writerPtr, FPDF_INCREMENTAL) + : m.PDFiumExt_SaveAsCopy(doc.docPtr, writerPtr); + if (!ok) { + throw new Error( + "PDFium could not serialize the edited document; save aborted so " + + "the existing file is left untouched.", + ); } const size = m.PDFiumExt_GetFileWriterSize(writerPtr); const outBuf = m.pdfium.wasmExports.malloc(size); @@ -62,6 +82,7 @@ export class PdfiumSave { size, ); view.set(heap); + assertSavedPdf(view); return view; } finally { m.pdfium.wasmExports.free(outBuf); diff --git a/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumTextWriter.ts b/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumTextWriter.ts index ed11bd073b..03f4bb1c24 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumTextWriter.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/pdfium/PdfiumTextWriter.ts @@ -23,10 +23,20 @@ export class PdfiumTextWriter { */ static commitRunText(doc: EditorDocument, page: Page, run: TextRun): boolean { if (!run.pdfiumObjPtr) return false; + // A text object with no characters is not a thing PDFium will build: + // FPDFText_SetText traps ("unreachable") on an empty string rather than + // returning false, which used to escape as a raw WASM error the moment a + // user selected all the text in a box and pressed Delete. Emptying a run + // is the overlay path's job - it paints the cover rect and emits nothing. + if (run.text.length === 0) return false; const m = doc.module; const ptr = writeUtf16(m, run.text); try { m.FPDFText_SetText(run.pdfiumObjPtr, ptr); + } catch { + // Any other trap means the fast path is unusable, which is exactly what + // false already means; the caller restores and re-emits. + return false; } finally { m.pdfium.wasmExports.free(ptr); } diff --git a/frontend/editor/src/core/tools/pdfTextEditor/store/EditorSession.ts b/frontend/editor/src/core/tools/pdfTextEditor/store/EditorSession.ts new file mode 100644 index 0000000000..ec22a2cad7 --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/store/EditorSession.ts @@ -0,0 +1,59 @@ +import { useSyncExternalStore } from "react"; +import type { FileId } from "@app/types/file"; + +/** + * What the editor's canvas top bar needs from the tool panel. + * + * The two surfaces are siblings, not ancestor and descendant: the panel is the + * tool, and the canvas is a workbench view registered by id and mounted + * somewhere else entirely, so no React context reaches from one to the other. + * They already share the EditorStore singleton for document state; this is the + * same seam for the things only the panel can do - anything touching the + * workbench's file list has to run where the FileContext is. + */ +export interface EditorSession { + /** Name of the open document, or null before one is opened. */ + fileName: string | null; + /** Workbench file the document came from; null for a file opened from disk. */ + fileId: FileId | null; + /** Apply the edit back to the workbench file. */ + save: () => void; + /** Apply, then hand the user a copy. */ + download: () => void; + /** Open a different workbench PDF (asks first when there are unsaved edits). */ + pickFile: (file: File) => void; + /** Open the OS file picker for a replacement image. */ + pickImage: () => void; +} + +let current: EditorSession | null = null; +const listeners = new Set<() => void>(); + +function emit(): void { + for (const listener of Array.from(listeners)) listener(); +} + +/** Publish (or, with null, retract) the panel's session. */ +export function setEditorSession(session: EditorSession | null): void { + current = session; + emit(); +} + +function subscribe(listener: () => void): () => void { + listeners.add(listener); + return () => listeners.delete(listener); +} + +function getSnapshot(): EditorSession | null { + return current; +} + +/** + * The live session, or null while the panel is not mounted. + * + * A null session means the canvas is on its own - it must degrade to view-only + * controls rather than render a Save button that cannot save. + */ +export function useEditorSession(): EditorSession | null { + return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); +} diff --git a/frontend/editor/src/core/tools/pdfTextEditor/store/EditorStore.ts b/frontend/editor/src/core/tools/pdfTextEditor/store/EditorStore.ts index 05edd8a1e8..28c1cdf872 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/store/EditorStore.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/store/EditorStore.ts @@ -1,5 +1,8 @@ import { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; -import { HistoryStack } from "@app/tools/pdfTextEditor/store/HistoryStack"; +import { + HistoryStack, + HistoryStepError, +} from "@app/tools/pdfTextEditor/store/HistoryStack"; import { Selection } from "@app/tools/pdfTextEditor/store/Selection"; import { pageGuides } from "@app/tools/pdfTextEditor/util/guides"; import { PdfiumTextReader } from "@app/tools/pdfTextEditor/pdfium/PdfiumTextReader"; @@ -78,6 +81,11 @@ export interface EditorViewState { widthMode: WidthMode; /** Show per-page rulers and alignment guides. */ showRulers: boolean; + // Find and shortcuts are opened from the canvas top bar but rendered beside + // the pages, and the panel's keyboard shortcuts drive both - so the flags + // live on the store both surfaces already share. + findOpen: boolean; + helpOpen: boolean; } const POSITION_REFRESH_MS = 600; @@ -101,6 +109,8 @@ const INITIAL: EditorViewState = { groupingMode: "auto", widthMode: "grow", showRulers: false, + findOpen: false, + helpOpen: false, }; // Single observable store for the editor's React layer. Components never reach @@ -203,6 +213,16 @@ export class EditorStore { this.patch({ showRulers }); } + setFindOpen(findOpen: boolean): void { + if (this.state.findOpen === findOpen) return; + this.patch({ findOpen }); + } + + setHelpOpen(helpOpen: boolean): void { + if (this.state.helpOpen === helpOpen) return; + this.patch({ helpOpen }); + } + get groupingMode(): GroupingMode { return this.state.groupingMode; } @@ -301,7 +321,14 @@ export class EditorStore { /** Apply a command via the history stack, re-snapshot, and notify. */ dispatch(cmd: Command): void { if (!this.doc) return; - this.history.execute(cmd, this.doc); + try { + this.history.execute(cmd, this.doc); + } catch (err) { + // A forward edit can break the document exactly like an undo can, so it + // gets the same recovery rather than throwing out of the click handler. + this.recoverFromBrokenStep(err); + return; + } this.resnapshot(); this.patch({ dirty: this.isDirty() }); this.schedulePositionRefresh(); @@ -391,8 +418,8 @@ export class EditorStore { if (!this.doc) return; try { this.history.undo(this.doc); - } catch { - this.recoverFromBrokenStep(); + } catch (err) { + this.recoverFromBrokenStep(err); return; } this.resnapshot(); @@ -403,8 +430,8 @@ export class EditorStore { if (!this.doc) return; try { this.history.redo(this.doc); - } catch { - this.recoverFromBrokenStep(); + } catch (err) { + this.recoverFromBrokenStep(err); return; } this.resnapshot(); @@ -412,10 +439,23 @@ export class EditorStore { } // A half-applied command leaves the run model describing objects that no - // longer match the page, so rebuild it from PDFium rather than guess. - private recoverFromBrokenStep(): void { + // longer match the page, so rebuild it from PDFium rather than guess. A + // command that rolled itself back changed nothing, so it keeps its history. + private recoverFromBrokenStep(err?: unknown): void { const doc = this.doc; if (!doc) return; + // The wrapper says which phase broke; the cause says what actually failed, + // and that is the half worth putting in front of the user. + const reason = err instanceof HistoryStepError ? err.cause : err; + const message = reason instanceof Error ? reason.message : null; + if (err instanceof HistoryStepError && err.documentIntact) { + this.resnapshot(); + this.patch({ + dirty: this.isDirty(), + error: message ?? this.state.error, + }); + return; + } this.repopulateAllPages(doc, this.state.groupingMode); // Rebuilt runs get fresh ids, so no existing history entry can apply. this.history.clear(); @@ -423,7 +463,7 @@ export class EditorStore { this.bakedDirty = true; this.selection.clear(); this.resnapshot(); - this.patch({ dirty: true }); + this.patch({ dirty: true, error: message ?? this.state.error }); } /** Drop every page's run model and read it back from the document. */ diff --git a/frontend/editor/src/core/tools/pdfTextEditor/store/HistoryStack.ts b/frontend/editor/src/core/tools/pdfTextEditor/store/HistoryStack.ts index a403e4a56e..152cc4813c 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/store/HistoryStack.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/store/HistoryStack.ts @@ -1,4 +1,7 @@ -import type { Command } from "@app/tools/pdfTextEditor/commands/Command"; +import { + RolledBackError, + type Command, +} from "@app/tools/pdfTextEditor/commands/Command"; import { CompositeCommand } from "@app/tools/pdfTextEditor/commands/CompositeCommand"; import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; @@ -12,12 +15,16 @@ const COALESCE_WINDOW_MS = 600; export class HistoryStepError extends Error { readonly phase: "apply" | "revert"; readonly cause: unknown; + // True when the failing command put the document back as it was, so the run + // model still describes the page and only THIS step is lost. + readonly documentIntact: boolean; constructor(phase: "apply" | "revert", cause: unknown) { super(`Command failed to ${phase}`); this.name = "HistoryStepError"; this.phase = phase; - this.cause = cause; + this.documentIntact = cause instanceof RolledBackError; + this.cause = cause instanceof RolledBackError ? cause.cause : cause; } } @@ -59,7 +66,16 @@ export class HistoryStack { // Read the clock BEFORE apply: the window is meant to measure the user's // idle time between edits. const startedAt = Date.now(); - cmd.apply(doc); + try { + cmd.apply(doc); + } catch (err) { + // Same contract undo/redo already honoured: a command that threw is NOT + // recorded, because a history entry whose apply half-ran cannot be + // reverted. Pushing first would have made the next undo run a revert + // against changes that were never made. + this.lastCoalesceKey = null; + throw new HistoryStepError("apply", err); + } const key = cmd.coalesceKey?.() ?? null; const top = this.undoStack[this.undoStack.length - 1]; // The command a merge would join. Unwrap a group to its most recent diff --git a/frontend/editor/src/core/tools/pdfTextEditor/util/exportPdf.ts b/frontend/editor/src/core/tools/pdfTextEditor/util/exportPdf.ts index b5b35c39aa..694c72379d 100644 --- a/frontend/editor/src/core/tools/pdfTextEditor/util/exportPdf.ts +++ b/frontend/editor/src/core/tools/pdfTextEditor/util/exportPdf.ts @@ -1,6 +1,7 @@ import type { EditorDocument } from "@app/tools/pdfTextEditor/model/EditorDocument"; import { preserveShadings } from "@app/tools/pdfTextEditor/pdfdoc/passes/preserveShadings"; import { PdfiumSave } from "@app/tools/pdfTextEditor/pdfium/PdfiumSave"; +import { assertIncrementalAppend } from "@app/tools/pdfTextEditor/util/savedBytes"; /** Serialize the editor document to a Blob plus the download filename. */ export async function exportToBlob( @@ -32,12 +33,21 @@ export async function exportToBlob( const repaired = await preserveShadings(bytes, doc.openedBytes, { pages: regenerated, }); - if (repaired) bytes = repaired; + // The repair appends its own revision, so it may only ever grow the file + // - a shorter result would mean it rewrote what it was handed. + if (repaired && (!incremental || repaired.length >= bytes.length)) { + bytes = repaired; + } } catch { /* the unrepaired save is still a correct save */ } } + // Checked LAST, on the bytes that will actually be written: the incremental + // path is the tool's signature-preserving promise, and a promise nothing + // verifies is just a comment. + if (incremental) assertIncrementalAppend(bytes, doc.openedBytes); + return { blob: pdfBlob(bytes), filename: exportName(sourceName) }; } diff --git a/frontend/editor/src/core/tools/pdfTextEditor/util/savedBytes.ts b/frontend/editor/src/core/tools/pdfTextEditor/util/savedBytes.ts new file mode 100644 index 0000000000..5bf4032a4e --- /dev/null +++ b/frontend/editor/src/core/tools/pdfTextEditor/util/savedBytes.ts @@ -0,0 +1,97 @@ +/** + * Checks the bytes a save produced before anything writes them anywhere. + * + * PDFium's save path reports failure by return code, and a writer that gave + * back nothing still hands over a zero-length buffer. Without these the editor + * would replace the user's workbench file with that buffer and clear the dirty + * flag - a total loss with no error anywhere. + */ + +const PDF_HEADER = "%PDF-"; +// A PDF cannot be meaningfully smaller than this; the smallest valid file is +// a few hundred bytes. Anything under it is a truncated or empty write. +const MIN_PDF_BYTES = 256; +// The spec puts %%EOF in the last 1024 bytes; allow slack for trailing padding. +const EOF_SCAN_BYTES = 4096; + +function startsWith(bytes: Uint8Array, ascii: string): boolean { + if (bytes.length < ascii.length) return false; + for (let i = 0; i < ascii.length; i++) { + if (bytes[i] !== ascii.charCodeAt(i)) return false; + } + return true; +} + +/** True when `%%EOF` appears within the last `EOF_SCAN_BYTES` bytes. */ +function hasTrailingEof(bytes: Uint8Array): boolean { + const marker = "%%EOF"; + const from = Math.max(0, bytes.length - EOF_SCAN_BYTES); + for (let i = bytes.length - marker.length; i >= from; i--) { + let match = true; + for (let j = 0; j < marker.length; j++) { + if (bytes[i + j] !== marker.charCodeAt(j)) { + match = false; + break; + } + } + if (match) return true; + } + return false; +} + +/** + * Throw unless `bytes` look like a whole PDF. + * + * Cheap structural checks only - header, plausible length, terminating marker. + * They are not a validator; they are the difference between "the save failed + * quietly" and "the save failed loudly". + */ +export function assertSavedPdf(bytes: Uint8Array): void { + if (bytes.length < MIN_PDF_BYTES) { + throw new Error( + `Save produced ${bytes.length} bytes, which is too short to be a PDF; ` + + "the file was left untouched.", + ); + } + if (!startsWith(bytes, PDF_HEADER)) { + throw new Error( + "Save produced data that does not start with a PDF header; " + + "the file was left untouched.", + ); + } + if (!hasTrailingEof(bytes)) { + throw new Error( + "Save produced a PDF with no end-of-file marker, so it is truncated; " + + "the file was left untouched.", + ); + } +} + +/** + * Throw unless `saved` is `original` plus an appended revision. + * + * This is what makes the signature-preserving claim checkable rather than + * hopeful: an incremental save may only ADD bytes, so every byte a signature + * covers has to still be there, unchanged, at the same offset. A full rewrite + * that slipped through the incremental path fails here instead of silently + * shipping a document whose signatures no longer verify. + */ +export function assertIncrementalAppend( + saved: Uint8Array, + original: Uint8Array, +): void { + if (saved.length < original.length) { + throw new Error( + "Incremental save is shorter than the file it appends to, so the signed " + + "revision was rewritten; the file was left untouched.", + ); + } + for (let i = 0; i < original.length; i++) { + if (saved[i] !== original[i]) { + throw new Error( + `Incremental save changed byte ${i} of the original revision, so its ` + + "signatures would no longer verify; the file was left untouched.", + ); + } + } +} diff --git a/frontend/editor/src/core/ui/KeyCombo.css b/frontend/editor/src/core/ui/KeyCombo.css new file mode 100644 index 0000000000..3ed1f586d2 --- /dev/null +++ b/frontend/editor/src/core/ui/KeyCombo.css @@ -0,0 +1,53 @@ +/* Keyboard shortcut rendered as one tile per key, the way chat and design apps + show their shortcut lists. */ + +.sui-keycombo { + display: inline-flex; + align-items: center; + gap: 0.25rem; + flex-wrap: wrap; + justify-content: flex-end; +} + +.sui-keycap { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 1.625rem; + height: 1.625rem; + padding: 0 0.4375rem; + border-radius: 0.3125rem; + border: 0.0625rem solid var(--c-border-subtle); + /* Raised against the row it sits on, so the tiles read as physical keys + rather than as inline code. */ + background: var(--c-bg-raised); + box-shadow: 0 0.0625rem 0 var(--c-border-subtle); + color: var(--c-text); + font-family: inherit; + font-size: 0.75rem; + font-weight: 600; + line-height: 1; + white-space: nowrap; +} + +/* A word key ("Click", "Drag") needs room to read as a word. */ +.sui-keycap--word { + font-weight: 500; + letter-spacing: 0.01em; +} + +/* Separator between alternative combos ("Ctrl + Z / Ctrl + Y"). */ +.sui-keycombo__or { + padding: 0 0.125rem; + color: var(--c-text-muted); + font-size: 0.75rem; + line-height: 1; +} + +/* The "+" between two keys of one combo. Muted, because the tiles carry the + meaning and a bright plus competes with them. */ +.sui-keycombo__plus { + color: var(--c-text-muted); + font-size: 0.6875rem; + line-height: 1; +} diff --git a/frontend/editor/src/core/ui/KeyCombo.stories.tsx b/frontend/editor/src/core/ui/KeyCombo.stories.tsx new file mode 100644 index 0000000000..d50c09bbff --- /dev/null +++ b/frontend/editor/src/core/ui/KeyCombo.stories.tsx @@ -0,0 +1,68 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { KeyCombo } from "@app/ui/KeyCombo"; + +const meta: Meta = { + title: "Primitives/KeyCombo", + component: KeyCombo, + tags: ["autodocs"], + parameters: { layout: "padded" }, + args: { combo: "Ctrl + Shift + V" }, +}; +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; + +export const WithPlus: Story = { + args: { combo: "Ctrl + S", withPlus: true }, +}; + +/** Alternatives are separated by "/", never crammed into one cap. */ +export const Alternatives: Story = { + args: { combo: "F3 / Ctrl + G" }, +}; + +/** Mouse gestures are keys too - they just happen to be words. */ +export const Gestures: Story = { + args: { combo: "Ctrl + Click + Drag" }, +}; + +export const InContext_ShortcutList: Story = { + render: () => ( +
+ {[ + ["Edit text", "Click"], + ["Add / remove a run from selection", "Ctrl + Click / Shift + Click"], + ["Marquee multi-select", "Ctrl + Shift + Drag"], + ["Undo / Redo", "Ctrl + Z / Ctrl + Y"], + ["Save to your workspace", "Ctrl + S"], + ].map(([label, combo], index) => ( +
+ {label} + +
+ ))} +
+ ), +}; diff --git a/frontend/editor/src/core/ui/KeyCombo.test.tsx b/frontend/editor/src/core/ui/KeyCombo.test.tsx new file mode 100644 index 0000000000..4ea7eabfce --- /dev/null +++ b/frontend/editor/src/core/ui/KeyCombo.test.tsx @@ -0,0 +1,70 @@ +import { render } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { KeyCombo } from "@app/ui/KeyCombo"; + +/** Every the combo rendered, in order. */ +function keycaps(container: HTMLElement): string[] { + return Array.from(container.querySelectorAll("kbd")).map( + (el) => el.textContent ?? "", + ); +} + +describe("KeyCombo", () => { + it("gives each key of a combo its own cap", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["Ctrl", "Shift", "V"]); + }); + + it("splits an unspaced combo the same way", () => { + // Both spellings have to land identically, or a list written by two people + // renders as a mix of tiles and run-on strings. + const { container } = render(); + expect(keycaps(container)).toEqual(["Ctrl", "Shift", "V"]); + }); + + it("keeps alternatives apart with a separator, not a cap", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["F3", "Ctrl", "G"]); + expect(container.textContent).toContain("/"); + }); + + it("treats a word key as one cap", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["Ctrl", "Click", "Drag"]); + }); + + it("renders a lone key with no separators", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["Esc"]); + }); + + it("survives a bare '+' being the key itself", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["+"]); + }); + + it("survives a bare '/' being the key itself", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["/"]); + }); + + it("adds a plus between keys only when asked", () => { + const { container } = render(); + expect(keycaps(container)).toEqual(["Ctrl", "S"]); + expect(container.textContent).toContain("+"); + }); + + it("marks a long key so it can be styled as a word", () => { + const { container } = render(); + expect(container.querySelector("kbd")?.className).toContain( + "sui-keycap--word", + ); + }); + + it("does not mark a short key as a word", () => { + const { container } = render(); + expect(container.querySelector("kbd")?.className).not.toContain( + "sui-keycap--word", + ); + }); +}); diff --git a/frontend/editor/src/core/ui/KeyCombo.tsx b/frontend/editor/src/core/ui/KeyCombo.tsx new file mode 100644 index 0000000000..b9e0a34cbe --- /dev/null +++ b/frontend/editor/src/core/ui/KeyCombo.tsx @@ -0,0 +1,85 @@ +import { Fragment } from "react"; +import "@app/ui/KeyCombo.css"; + +export interface KeyComboProps { + // The shortcut as written by a human: "Ctrl + Z", "Ctrl+Shift+V", + // "F3 / Ctrl + G", "Ctrl + Click + drag". Both spaced and unspaced "+" + // parse the same, so no caller has to remember which style this expects. + combo: string; + /** Show a "+" between the keys of one combo. Off by default. */ + withPlus?: boolean; + className?: string; +} + +/** Alternative separator: " / " between whole combos, e.g. "F3 / Ctrl+G". */ +const ALTERNATIVE = /\s*\/\s*/; +/** Key separator inside one combo: "+" with or without spaces around it. */ +const KEY = /\s*\+\s*/; + +// A lone "+" IS a key (Discord's "add reaction"), so a combo that is nothing +// but separators has to survive the split. +function splitKeys(combo: string): string[] { + const trimmed = combo.trim(); + if (trimmed === "+" || trimmed === "") return [trimmed]; + return trimmed + .split(KEY) + .map((k) => k.trim()) + .filter((k) => k.length > 0); +} + +/** True for keys that read as words rather than as a single glyph or name. */ +function isWord(key: string): boolean { + return /\s/.test(key) || key.length > 5; +} + +/** + * A keyboard shortcut drawn as one tile per key. + * + * Every key gets its own cap - "Ctrl" and "Z" are two things you press, so + * they are two tiles, which is what every app that shows a shortcut list does. + * Rendering the whole string in one cap ("Ctrl+Shift+V") reads as a single + * mystery key and makes lists of shortcuts impossible to scan. + */ +export function KeyCombo({ + combo, + withPlus = false, + className, +}: KeyComboProps) { + const alternatives = combo.split(ALTERNATIVE).filter((c) => c.trim() !== ""); + // "/" is also a key on its own ("Cmd + /"), so a string that splits to + // nothing was a bare separator and is shown verbatim. + const groups = alternatives.length > 0 ? alternatives : [combo]; + + return ( + + {groups.map((group, groupIndex) => ( + + {groupIndex > 0 && ( + + / + + )} + {splitKeys(group).map((key, keyIndex) => ( + + {withPlus && keyIndex > 0 && ( + + + + + )} + + {key} + + + ))} + + ))} + + ); +} diff --git a/frontend/editor/src/core/ui/index.ts b/frontend/editor/src/core/ui/index.ts index c621319f0e..7a8645701e 100644 --- a/frontend/editor/src/core/ui/index.ts +++ b/frontend/editor/src/core/ui/index.ts @@ -15,6 +15,7 @@ export * from "@app/ui/NavSurface"; export * from "@app/ui/Surface"; export * from "@app/ui/PanelHeader"; export * from "@app/ui/CodeBlock"; +export * from "@app/ui/KeyCombo"; export * from "@app/ui/SectionDivider"; export * from "@app/ui/CarouselDots"; export * from "@app/ui/Card";