From e25f1808528eea38d93897c5ff996230f054d137 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Sun, 30 Aug 2026 10:45:16 +0100 Subject: [PATCH] Re-arm the stamp tool after each placement when place multiple is on --- .../public/locales/en-GB/translation.toml | 6 - .../public/locales/en-US/translation.toml | 6 + .../viewer/SignatureAPIBridge.test.tsx | 178 ++++++++++++++++++ .../components/viewer/SignatureAPIBridge.tsx | 32 +++- .../viewer/signaturePlacement.test.ts | 81 ++++++++ .../components/viewer/signaturePlacement.ts | 17 ++ 6 files changed, 313 insertions(+), 7 deletions(-) create mode 100644 frontend/editor/src/core/components/viewer/SignatureAPIBridge.test.tsx diff --git a/frontend/editor/public/locales/en-GB/translation.toml b/frontend/editor/public/locales/en-GB/translation.toml index 94d9efa11f..d2b2aa38e3 100644 --- a/frontend/editor/public/locales/en-GB/translation.toml +++ b/frontend/editor/public/locales/en-GB/translation.toml @@ -226,8 +226,6 @@ title = "How to add images" move = "Move Image" pause = "Pause placement" place = "Place Image" -placeMultiple = "Stay in placement mode after each placement" -placeMultipleDesc = "Place several images in a row instead of exiting after the first." resume = "Resume placement" [addImage.results] @@ -444,8 +442,6 @@ title = "How to add text" move = "Move Text" pause = "Pause placement" place = "Place Text" -placeMultiple = "Stay in placement mode after each placement" -placeMultipleDesc = "Place several text stamps in a row instead of exiting after the first." resume = "Resume placement" [addText.results] @@ -10313,8 +10309,6 @@ title = "Draw on your phone" move = "Move Signature" pause = "Pause placement" place = "Place Signature" -placeMultiple = "Stay in placement mode after each placement" -placeMultipleDesc = "Place several signatures in a row instead of exiting after the first." resume = "Resume placement" [sign.results] diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index baee16cd02..d3af84d904 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -226,6 +226,8 @@ title = "How to add images" move = "Move Image" pause = "Pause placement" place = "Place Image" +placeMultiple = "Stay in placement mode after each placement" +placeMultipleDesc = "Place several images in a row instead of exiting after the first." resume = "Resume placement" [addImage.results] @@ -442,6 +444,8 @@ title = "How to add text" move = "Move Text" pause = "Pause placement" place = "Place Text" +placeMultiple = "Stay in placement mode after each placement" +placeMultipleDesc = "Place several text stamps in a row instead of exiting after the first." resume = "Resume placement" [addText.results] @@ -10671,6 +10675,8 @@ title = "Draw on your phone" move = "Move Signature" pause = "Pause placement" place = "Place Signature" +placeMultiple = "Stay in placement mode after each placement" +placeMultipleDesc = "Place several signatures in a row instead of exiting after the first." resume = "Resume placement" [sign.results] diff --git a/frontend/editor/src/core/components/viewer/SignatureAPIBridge.test.tsx b/frontend/editor/src/core/components/viewer/SignatureAPIBridge.test.tsx new file mode 100644 index 0000000000..f315e4f245 --- /dev/null +++ b/frontend/editor/src/core/components/viewer/SignatureAPIBridge.test.tsx @@ -0,0 +1,178 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, waitFor, act } from "@testing-library/react"; +import { PdfAnnotationSubtype } from "@embedpdf/models"; + +const mocks = vi.hoisted(() => ({ + annotationApi: null as unknown, + signature: {} as Record, +})); + +vi.mock("@embedpdf/plugin-annotation/react", () => ({ + useAnnotationCapability: () => ({ provides: mocks.annotationApi }), +})); +vi.mock("@app/contexts/SignatureContext", () => ({ + useSignature: () => mocks.signature, +})); +vi.mock("@app/contexts/ViewerContext", () => ({ + useViewer: () => ({ + getZoomState: () => ({ currentZoom: 1 }), + registerImmediateZoomUpdate: () => () => {}, + }), +})); +vi.mock("@app/components/viewer/hooks/useDocumentReady", () => ({ + useDocumentReady: () => true, +})); + +import { SignatureAPIBridge } from "@app/components/viewer/SignatureAPIBridge"; + +const SIGNATURE_DATA = "data:image/png;base64,iVBORw0KGgo="; + +type AnnotationEvent = { + type: string; + annotation: { id: string; type: number }; + ctx?: unknown; +}; + +const settle = () => new Promise((resolve) => setTimeout(resolve, 30)); + +/** + * Stand-in for @embedpdf/plugin-annotation's capability. `placeStamp` mirrors + * the real onCommit ordering (dist/index.js): the create event is emitted + * synchronously, and only then does deactivateToolAfterCreate disarm the tool. + */ +function makeAnnotationApi() { + const listeners = new Set<(event: AnnotationEvent) => void>(); + let activeTool: { id: string } | null = null; + let placedCount = 0; + + return { + setActiveTool: vi.fn((id: string | null) => { + activeTool = id ? { id } : null; + }), + getActiveTool: vi.fn(() => activeTool), + setToolDefaults: vi.fn(), + onAnnotationEvent: vi.fn((cb: (event: AnnotationEvent) => void) => { + listeners.add(cb); + return () => listeners.delete(cb); + }), + getSelectedAnnotation: vi.fn(() => null), + deleteAnnotation: vi.fn(), + + /** A pointer placement. Throws if the tool is disarmed, as the real one is a no-op then. */ + placeStamp(id: string) { + if (activeTool?.id !== "stamp") { + throw new Error( + `Cannot place "${id}": stamp tool not armed (active: ${activeTool?.id ?? "none"})`, + ); + } + placedCount += 1; + listeners.forEach((cb) => + cb({ + type: "create", + annotation: { id, type: PdfAnnotationSubtype.STAMP }, + ctx: { pointer: true }, + }), + ); + activeTool = null; + }, + + /** A paste / undo-redo restore: a create event with no pointer context. */ + restoreStamp(id: string) { + listeners.forEach((cb) => + cb({ + type: "create", + annotation: { id, type: PdfAnnotationSubtype.STAMP }, + }), + ); + }, + + activeToolId: () => activeTool?.id ?? null, + placedCount: () => placedCount, + }; +} + +type FakeAnnotationApi = ReturnType; + +function setup(placeMultiple: boolean) { + const api = makeAnnotationApi(); + mocks.annotationApi = api; + const setPlacementMode = vi.fn(); + mocks.signature = { + signatureConfig: { + signatureType: "image", + signatureData: SIGNATURE_DATA, + reason: "Test", + }, + storeImageData: vi.fn(), + isPlacementMode: true, + placementPreviewSize: { width: 100, height: 50 }, + setSignaturesApplied: vi.fn(), + placeMultiple, + autoExitAfterStampPlacement: true, + setPlacementMode, + }; + const view = render(); + return { api, setPlacementMode, view }; +} + +const expectArmed = (api: FakeAnnotationApi) => + waitFor(() => expect(api.activeToolId()).toBe("stamp")); + +describe("SignatureAPIBridge stamp re-arming", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("lets the user place several stamps in a row when placeMultiple is on", async () => { + const { api, setPlacementMode } = setup(true); + await expectArmed(api); + + // First placement: the plugin disarms the tool the moment it commits. + act(() => api.placeStamp("stamp-1")); + expect(api.activeToolId()).toBeNull(); + + // The bridge must re-arm it without the user re-selecting the tool. + await expectArmed(api); + act(() => api.placeStamp("stamp-2")); + await expectArmed(api); + act(() => api.placeStamp("stamp-3")); + await expectArmed(api); + + expect(api.placedCount()).toBe(3); + expect(setPlacementMode).not.toHaveBeenCalledWith(false); + }); + + it("leaves the tool disarmed and exits placement mode when placeMultiple is off", async () => { + const { api, setPlacementMode } = setup(false); + await expectArmed(api); + + act(() => api.placeStamp("stamp-1")); + await settle(); + + expect(setPlacementMode).toHaveBeenCalledWith(false); + expect(api.activeToolId()).toBeNull(); + expect(api.placedCount()).toBe(1); + }); + + it("does not re-arm on a programmatic create (paste, undo/redo restore)", async () => { + const { api } = setup(true); + await expectArmed(api); + const armCount = api.setToolDefaults.mock.calls.length; + + act(() => api.restoreStamp("pasted-1")); + await settle(); + + expect(api.setToolDefaults.mock.calls.length).toBe(armCount); + }); + + it("clears pending re-arm timers on unmount", async () => { + const { api, view } = setup(true); + await expectArmed(api); + + act(() => api.placeStamp("stamp-1")); + act(() => view.unmount()); + await settle(); + + expect(api.activeToolId()).toBeNull(); + }); +}); diff --git a/frontend/editor/src/core/components/viewer/SignatureAPIBridge.tsx b/frontend/editor/src/core/components/viewer/SignatureAPIBridge.tsx index c1bce72eda..ce361cd83f 100644 --- a/frontend/editor/src/core/components/viewer/SignatureAPIBridge.tsx +++ b/frontend/editor/src/core/components/viewer/SignatureAPIBridge.tsx @@ -16,7 +16,10 @@ import type { import type { SignParameters } from "@app/hooks/tools/sign/useSignParameters"; import { useViewer } from "@app/contexts/ViewerContext"; import { useDocumentReady } from "@app/components/viewer/hooks/useDocumentReady"; -import { shouldAutoExitPlacement } from "@app/components/viewer/signaturePlacement"; +import { + shouldAutoExitPlacement, + shouldRearmPlacement, +} from "@app/components/viewer/signaturePlacement"; /** * Connects the PDF signature (stamp/ink) tools to the shared ViewerContext and SignatureContext. @@ -315,6 +318,13 @@ export const SignatureAPIBridge = forwardRef< cssToPdfSize, ]); + // Mirrored so the long-lived create subscription is not rebuilt per config change. + const configureStampDefaultsRef = useRef(configureStampDefaults); + useEffect(() => { + configureStampDefaultsRef.current = configureStampDefaults; + }, [configureStampDefaults]); + const rearmTimersRef = useRef(new Set()); + // Enable keyboard deletion of selected annotations useEffect(() => { // Always enable delete key when we have annotation API and are in sign mode @@ -585,6 +595,23 @@ export const SignatureAPIBridge = forwardRef< ) { annotationApi.setActiveTool(null); setPlacementMode(false); + } else if ( + shouldRearmPlacement({ + annotation, + placeMultiple: placeMultipleRef.current, + autoExitEnabled: autoExitRef.current, + userPlaced, + }) + ) { + // The plugin calls setActiveTool(null) right after this event fires, + // so re-arm on the next task rather than inline. + const timer = window.setTimeout(() => { + rearmTimersRef.current.delete(timer); + configureStampDefaultsRef.current().catch((error) => { + console.error("Error re-arming signature placement:", error); + }); + }, 0); + rearmTimersRef.current.add(timer); } } @@ -604,8 +631,11 @@ export const SignatureAPIBridge = forwardRef< } }); + const rearmTimers = rearmTimersRef.current; return () => { unsubscribe?.(); + rearmTimers.forEach((id) => window.clearTimeout(id)); + rearmTimers.clear(); }; }, [ annotationApi, diff --git a/frontend/editor/src/core/components/viewer/signaturePlacement.test.ts b/frontend/editor/src/core/components/viewer/signaturePlacement.test.ts index 01d25d9ef6..de6941829b 100644 --- a/frontend/editor/src/core/components/viewer/signaturePlacement.test.ts +++ b/frontend/editor/src/core/components/viewer/signaturePlacement.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { PdfAnnotationSubtype } from "@embedpdf/models"; import { shouldAutoExitPlacement, + shouldRearmPlacement, type AutoExitPlacementParams, } from "@app/components/viewer/signaturePlacement"; @@ -83,3 +84,83 @@ describe("shouldAutoExitPlacement", () => { ).toBe(false); }); }); + +describe("shouldRearmPlacement", () => { + it("returns true for a user-placed stamp when placeMultiple is on", () => { + expect(shouldRearmPlacement(params({ placeMultiple: true }))).toBe(true); + }); + + it("returns false when placeMultiple is off (auto-exit handles that case)", () => { + expect(shouldRearmPlacement(params())).toBe(false); + }); + + it("returns false when the mounted tool has not opted in", () => { + expect( + shouldRearmPlacement( + params({ placeMultiple: true, autoExitEnabled: false }), + ), + ).toBe(false); + }); + + it("returns false for programmatic creates (paste, undo/redo restore)", () => { + expect( + shouldRearmPlacement(params({ placeMultiple: true, userPlaced: false })), + ).toBe(false); + }); + + it("returns false for ink strokes, so multi-stroke signatures still work", () => { + expect( + shouldRearmPlacement( + params({ + placeMultiple: true, + annotation: { type: PdfAnnotationSubtype.INK }, + }), + ), + ).toBe(false); + }); + + it("returns false for FREETEXT and other non-stamp types", () => { + expect( + shouldRearmPlacement( + params({ + placeMultiple: true, + annotation: { type: PdfAnnotationSubtype.FREETEXT }, + }), + ), + ).toBe(false); + }); + + it("falls back to annotation.object.type when annotation.type is missing", () => { + expect( + shouldRearmPlacement( + params({ + placeMultiple: true, + annotation: { object: { type: PdfAnnotationSubtype.STAMP } }, + }), + ), + ).toBe(true); + }); + + it("returns false when neither annotation.type nor object.type is present", () => { + expect( + shouldRearmPlacement(params({ placeMultiple: true, annotation: {} })), + ).toBe(false); + expect( + shouldRearmPlacement(params({ placeMultiple: true, annotation: null })), + ).toBe(false); + expect( + shouldRearmPlacement( + params({ placeMultiple: true, annotation: undefined }), + ), + ).toBe(false); + }); + + it("never agrees with shouldAutoExitPlacement for the same input", () => { + for (const placeMultiple of [true, false]) { + const input = params({ placeMultiple }); + expect( + shouldAutoExitPlacement(input) && shouldRearmPlacement(input), + ).toBe(false); + } + }); +}); diff --git a/frontend/editor/src/core/components/viewer/signaturePlacement.ts b/frontend/editor/src/core/components/viewer/signaturePlacement.ts index c49f96021a..1695f81a76 100644 --- a/frontend/editor/src/core/components/viewer/signaturePlacement.ts +++ b/frontend/editor/src/core/components/viewer/signaturePlacement.ts @@ -35,3 +35,20 @@ export function shouldAutoExitPlacement( params.annotation?.type ?? params.annotation?.object?.type ?? null; return type === PdfAnnotationSubtype.STAMP; } + +/** + * Whether the stamp tool must be re-armed after a user placement. + * + * The shared "stamp" tool carries `deactivateToolAfterCreate`, so the plugin + * disarms it after every placement. Without an explicit re-arm the panel keeps + * offering "Pause placement" while clicking the page does nothing. The same + * exclusions as auto-exit apply: only user-placed stamps count. + */ +export function shouldRearmPlacement(params: AutoExitPlacementParams): boolean { + if (!params.autoExitEnabled || !params.userPlaced || !params.placeMultiple) { + return false; + } + const type = + params.annotation?.type ?? params.annotation?.object?.type ?? null; + return type === PdfAnnotationSubtype.STAMP; +}