feat(annotations): allow user to resize and move annotation boxes after drawing #233
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
After drawing a rectangular annotation on the PDF viewer, users have no way to adjust its position or size. If the box is slightly off — too wide, shifted, or misaligned with the handwriting — the only option is to delete it and redraw from scratch. This is especially frustrating when fine-tuning regions for guided OCR.
Proposed Solution
When a rectangle annotation is selected in transcription mode, show interactive resize handles and allow the whole box to be dragged to a new position.
UX Design
Edit mode is active when: annotation is selected +
transcribeModeis on + annotation has no polygon (i.e. user-drawn, not OCR-placed).8 resize handles appear at corners and edge midpoints (10×10 px squares, white fill, 2px colored border — matching the annotation color).
Interaction:
nwse-resize/nesw-resizecursor)ns-resize/ew-resizecursor)movecursor)pointerupviaPATCH /api/documents/{id}/annotations/{annotationId}Polygon (OCR) annotations show NO handles — they remain click-only.
Backend Changes
UpdateAnnotationDTO(new):{ x, y, width, height }— all optional doublesAnnotationService.updateAnnotation()— load, verify ownership, update fieldsAnnotationController— addPATCH /api/documents/{docId}/annotations/{annotationId}with@RequirePermission(WRITE_ALL)Frontend Changes
AnnotationShape.sveltecanEditpropAnnotationLayer.svelteonAnnotationUpdate+canEditdownPdfViewer.svelteonAnnotationUpdatecallback+page.svelte(document)handleAnnotationUpdate()→PATCHrequestAcceptance Criteria
./mvnw testpasses including newAnnotationServiceupdate tests👨💻 Felix Brandt — Senior Fullstack Developer
Questions & Observations
Component size risk:
AnnotationShape.svelteis taking on resize handle rendering, 8 separate pointer event handlers, cursor logic, drag state, and PATCH triggering. That's easily 150+ lines. Will it still have a name that means one thing, or does it become a "ResizeableDraggableAnnotationShapeWithHandlesAndPatchManager"? Worth deciding up front whether to splitResizeHandle.svelteorAnnotationEditOverlay.svelteas a sibling.Callback chain depth:
onAnnotationUpdateis threaded throughAnnotationShape → AnnotationLayer → PdfViewer → +page.svelte. That's four hops for a callback. At what point does a Svelte context (setContext/getContext) or a small local$staterune on the page become cleaner than prop-drilling four levels?Naming:
canEditas a prop name is generic. Since this specific condition is "selected AND transcribeMode AND no polygon", something likeisResizableoreditModeActivecarries intent more directly. The name should answer "why is this prop here?" not just "what does it enable?"Rapid drag → PATCH storm: The spec says PATCH fires on
pointerup, which is correct. But what about a user who clicks, micro-drags (less than 1px of real movement), and releases? Should we skip the PATCH if the position didn't actually change? Otherwise every accidental click on a selected annotation fires a no-op PATCH.Rollback on PATCH failure: Optimistic updates are listed, but there's no mention of what happens when the PATCH returns a 4xx or 5xx. The box has already moved visually. Does it snap back? Does an error appear? This needs a defined path.
Keyboard users: The issue lists only pointer interactions. Is keyboard-based resize/move explicitly out of scope for this issue, or just forgotten? Worth a conscious decision either way.
Suggestions
newX === annotation.x && newY === annotation.y && newWidth === annotation.width && newHeight === annotation.height, skip the PATCH entirely.AnnotationServiceupdate tests before writing any production code — they'll drive the service API design cleanly.🏗️ Markus Keller — Application Architect
Questions & Observations
DB-layer bounds enforcement:
UpdateAnnotationDTOhasx, y, width, heightas optional doubles. The issue mentions a minimum size of 1%×1% (0.01 normalized), but I see no mention of a CHECK constraint in the database to enforce valid ranges (0.0–1.0 for all four fields). Application-layer validation has race-condition and bypass risks. A migration addingCHECK (x >= 0 AND x <= 1 AND y >= 0 AND y <= 1 AND width >= 0.01 AND height >= 0.01)would be the right safety net.Optimistic update without a PATCH failure path is an architectural gap: The spec describes optimistic UI updates but is silent on failure recovery. This isn't just a UI detail — it's a data consistency question. If the PATCH fails (network drop, 403, 500), the frontend has a stale visual state that doesn't match the backend. The architecture for this feature needs a defined rollback: store the pre-drag state locally, restore it on error.
Is PATCH the right verb here? Yes — the endpoint accepts a partial update of position/size fields. That's semantically correct. Just noting it was a deliberate choice, not a default.
Prop drilling vs. context: Four-level callback threading (
AnnotationShape → AnnotationLayer → PdfViewer → +page.svelte) for a single write operation looks like a candidate for Svelte context. The document detail page already manages a lot of local state. At what point does the annotation update context belong to a page-level store rather than a prop chain?No schema migration needed?: The issue lists no migration, which makes sense if the
annotationstable already hasx, y, width, heightcolumns. Worth confirming — if those columns don't already exist, this is missing a Flyway step.Suggestions
CHECKconstraint bounding normalized coordinates to[0, 1]and minimum size>= 0.01. This is free integrity enforcement that no application bug can bypass.AnnotationShape.svelte.$derivedwith an intent-revealing name (isInResizeMode) rather than composing the boolean inline at the call site.🔍 Sara Holt — QA Engineer
Questions & Observations
Acceptance criteria gaps:
WRITE_ALL? These are two different access checks and need two separate test cases.Test strategy questions:
page.mouse.move()+down/up, but pointer capture (setPointerCapture) can interfere with drag behavior in some browsers. Has this been tested on Firefox as well as Chrome?pointerdown/pointermove/pointerup. But are there specific touch UX concerns (e.g. long-press to enter edit mode vs. tap to select)? This affects what E2E tests need to cover.AnnotationService.updateAnnotation()— I'd want tests for: update with valid bounds, update with x out of range, update with zero-size dimensions, update for annotation that belongs to a different document, update with a non-existent annotation ID.CI impact:
AnnotationServiceunit tests (Mockito) + one@WebMvcTestfor the PATCH endpoint. That's fast and shouldn't add >5s to CI.Suggestions
@WebMvcTestforPATCH /api/documents/{docId}/annotations/{annotationId}covering: 200 (valid), 403 (wrong user), 404 (unknown annotation), 400 (out-of-bounds coordinates).makeAnnotation(UUID docId, double x, double y, double w, double h)to avoid repeated builder chains across tests.🔐 Nora Steiner — Security Engineer
Questions & Observations
IDOR risk on the PATCH endpoint:
PATCH /api/documents/{docId}/annotations/{annotationId}— the issue says "verify ownership" inAnnotationService.updateAnnotation(), but doesn't specify what exactly is verified. There are two distinct checks needed:WRITE_ALL? ✅ Covered by@RequirePermission(WRITE_ALL).annotationIdactually belong todocId? This is the IDOR check. If the service loads the annotation byannotationIdalone without also filtering bydocId, an attacker can craft a request likePATCH /api/documents/docA/annotations/annotationFromDocBand move someone else's annotation.The fix is a single JOIN:
annotationRepository.findByIdAndDocumentId(annotationId, docId), throwingnotFoundif the result is empty. This prevents cross-document annotation manipulation without leaking whether the annotation exists.Input validation — floating point edge cases:
UpdateAnnotationDTOcarriesDoublefields (x, y, width, height). What happens if the client sendsNaN,Infinity, or-Infinity? Jackson will deserialize these as valid Java doubles by default. The service should reject them explicitly before persisting. A@DecimalMin/@DecimalMaxbean validation constraint on the DTO fields handles this cleanly:NaNfails@DecimalMinbecause comparisons with NaN are always false.No new attack surface otherwise:
@RequirePermission(WRITE_ALL)on the PATCH endpoint: correct.Suggestions
(annotationId, docId)pair, not by ID alone.@Validto the controller parameter and bean validation constraints to the DTO for all four coordinate fields.PATCH /api/documents/{docA}/annotations/{annotationFromDocB}should return 404, not 200.🎨 Leonie Voss — UI/UX & Accessibility
Questions & Observations
Handle size is too small for the target audience:
The spec says 10×10 px squares. That fails WCAG 2.2 Target Size (Minimum) — which requires 24×24 px, and our senior audience (60+) needs at minimum 44×44 px. On a tablet with a finger, a 10px hit target is essentially unreachable. I'd recommend 16px visual square with a 44px invisible pointer event area (padding or a transparent
<rect>overlay):Keyboard accessibility is absent:
The entire interaction (drag handles, drag interior) is pointer-only. A user who navigates by keyboard cannot resize or move annotations at all. For a minimum viable accessible solution, a selected annotation in edit mode could expose four arrow-key nudge behaviors (e.g.
ArrowLeft/Right/Up/DownwithShiftfor larger steps). This also makes the feature usable for users with fine motor impairments. Consider opening a follow-up issue explicitly scoped to keyboard resize if this iteration is pointer-only.Screen reader discoverability:
When the 8 handles appear, screen reader users get no announcement. An
aria-live="polite"region saying something like "Annotation selected. Drag handles visible." would close this gap without disrupting non-AT users.PATCH failure feedback:
The spec mentions optimistic updates but no feedback on failure. If the drag snaps back, the user needs to understand why. An
aria-liveerror message ("Could not save annotation position") ensures screen reader users aren't left in an invisible rollback state.Contrast on handles:
"White fill, 2px colored border matching the annotation color" — annotation colors aren't specified in the issue. If a light-colored annotation is used on a white or light background, the handle border may fail WCAG AA contrast (4.5:1). Consider always using
brand-navyas the handle border regardless of annotation color for guaranteed contrast.Suggestions
aria-liveannouncement when edit mode activates: "Annotation selected — resize handles visible."brand-navy(#002850) as the fixed handle border color for contrast reliability.⚙️ Tobias Wendt — DevOps & Platform Engineer
Questions & Observations
No new infrastructure — good. This feature adds a PATCH endpoint and frontend pointer event logic. No new services, no new containers, no migration to deploy separately. Clean footprint.
PATCH request volume under heavy use:
The spec correctly fires PATCH on
pointeruprather than on eachpointermove— that's the right call and prevents a request storm. Even so, worth confirming: a user doing 50 drag operations in a session fires 50 PATCHes. At 5 concurrent users that's 250 annotation updates per session. That's well within the CX32's capacity, but worth adding a metric or counter if annotation usage grows.No new Flyway migration listed:
The issue mentions no schema change. That's fine if
x, y, width, heightalready exist in theannotationstable. If they don't, the deployment will fail on startup when Flyway runs and finds nothing to alter. Double-check this before merging — a missing migration is the easiest production incident to trigger and the easiest to catch in review.Observability:
The PATCH endpoint will appear in Spring Boot access logs automatically. If you want to track annotation edit frequency (useful for knowing how often users actually need to adjust boxes), a single Micrometer counter in
AnnotationService.updateAnnotation()takes one line:Not blocking, but cheap to add while the method is being written.
E2E test infrastructure consideration:
If Playwright drag tests are added (which they should be), they'll need a real PDF document loaded in the test browser. Make sure the E2E Docker Compose setup has a fixture document with at least one annotation pre-seeded via the test database setup — otherwise the drag test creates its own annotation first and the test scope widens.
Suggestions
x, y, width, heightcolumns already exist in theannotationstable before the PR is opened. If not, write the migration.AnnotationServicewhile you're touching the method — one line, zero operational cost.👨💻 Felix Brandt — Developer Discussion Summary
Worked through 6 open items before implementation starts. All resolved.
1. PATCH failure rollback
Decision: Snap back + show error message.
AnnotationEditOverlaycaptures apreDragStatesnapshot atpointerdown. If the PATCH returns non-ok, restore from snapshot and surface an error via the existinggetErrorMessage()pipeline (witharia-liveso screen readers catch it).2. Component split boundary
Decision: Extract
AnnotationEditOverlay.sveltefor handles and drag state only.AnnotationShape.sveltekeeps the box render and conditionally includes the overlay whenisResizableis true. Zero render code duplication — the overlay is purely additive (handles only, no box).3. Callback chain → Svelte context
Decision: Checked the code.
PdfVieweralready ownsannotationsstate,documentId, andloadAnnotations()— the page doesn't manage annotation data at all. The PATCH therefore lives inPdfViewer, not+page.svelte.PdfViewerexposes the update function viasetContext;AnnotationEditOverlaycalls it directly viagetContext. No prop threading throughAnnotationLayerorAnnotationShape.4. Prop naming
Decision:
isResizable— reads the intent directly (selected AND transcribeMode AND no polygon).5. No-op PATCH guard
Decision: Before firing the PATCH, compare new coordinates against the
pointerdownsnapshot. If unchanged (micro-drag or accidental click), skip the PATCH entirely.6. Keyboard + touch scope (was missed in spec)
Decision:
touch-action: noneonAnnotationEditOverlay+event.isPrimaryguard to ignore multi-finger gestures. Handle hit areas 44px from the start (required for touch, not optional).Overall the spec is solid. The main structural insight from the code review:
PdfVieweris the right home for annotation update logic — it already owns everything needed. The context pattern keeps intermediary components clean.🎨 Leonie Voss — UI/UX Discussion Summary
Worked through 6 open UI/UX and accessibility items. All resolved.
1. Handle border color
Decision:
brand-navy(#002850) fixed for all handle borders, regardless of annotation color. The teal annotation color (#00C7B1) on white gives ~2.8:1 — fails WCAG AA. Navy gives 14.5:1 and visually distinguishes handles (controls) from the box itself.2. Discoverability
Decision:
pointercursor on hover for resizable annotations in transcription mode. No tooltip or highlight needed. The primary flow is draw → immediately correct — the user just drew the box, so discoverability is naturally high. Correcting others' boxes is not the primary use case.3. Arrow key step size
Decision: 0.005 normalized (0.5% of document dimensions) per keypress.
Shift+Arrow= 0.05 (5%) for coarser moves.4. Error message placement
Decision: Inline message near the PDF viewer on PATCH failure. Transient (~4s), dismissible, wired to the existing
aria-livepattern. Not a toast — the user's eyes are on the annotation, not the corner of the screen.5. Drag boundary visual feedback
Decision: Silent clamping at document edges. No visual indicator needed — the primary flow doesn't involve extreme drags, and the added complexity isn't justified.
6.
aria-liveon edit mode activationDecision: Yes —
aria-live="polite"announcement when handles appear (e.g. "Annotation selected — resize handles visible"). Requires a new Paraglide translation key inde.json,en.json, andes.json.The primary flow insight shaped most decisions here: draw → immediately correct a small miss. That context kept the design appropriately simple — no tooltips, no boundary indicators, pointer cursor only.
🏗️ Markus Keller — Architecture Discussion Summary
Worked through 3 open backend/schema items. All resolved.
1. DB CHECK constraint
Decision: Add
chk_annotation_boundsas part of this issue's migration (V33). Enforces normalized coordinate ranges at the database layer — no application bug or raw API call can persist invalid data.Consistent with the V23 precedent (
chk_annotation_polygon_quad).2. Schema confirmation
Decision: Confirmed by reading V10.
x,y,width,heightare alreadyDOUBLE PRECISION NOT NULLindocument_annotations. No new columns needed — only the CHECK constraint from item 1.3. Error code for PATCH failure
Decision:
ANNOTATION_UPDATE_FAILED— new value inErrorCode.java, newcaseinerrors.ts, translation key inde.json,en.json,es.json. Maps to "Could not save annotation position" (or equivalent per locale).The backend scope for this issue is well-contained: one new DTO, one new service method, one new endpoint, and one migration (CHECK constraint only). No structural schema changes needed.
✅ Implementation complete — branch
feat/issue-233-annotation-resize-moveAll 20 planned tasks done. Every test is green.
Commits
f76a6c026c71811558881ff231db953cb2cf5362a53b756cd3fb32eac610a3ce7f88a4What was built
Backend:
V33__add_annotation_bounds_constraint.sql— DB-level CHECK constraint (x/y ∈ [0,1],width/height ∈ [0.01,1])UpdateAnnotationDTO— partial update DTO with@DecimalMin/@DecimalMaxbean validation on all 4 fieldsAnnotationService.updateAnnotation()— loads by(annotationId, docId)pair (IDOR guard via existingfindByIdAndDocumentId), applies only non-null fieldsPATCH /api/documents/{docId}/annotations/{annotationId}—@RequirePermission({ANNOTATE_ALL, WRITE_ALL}),@ValidbodyANNOTATION_UPDATE_FAILEDerror codeFrontend:
AnnotationEditOverlay.svelte— SVG overlay with 8 resize handles (44px hit areas, 16px visible squares, navy border), full-box move area, drag with optimistic preview, PATCH-on-release, rollback on error, arrow-key nudge (0.005/0.05 normalized, 300ms debounce),aria-liveannouncement,touch-action: noneAnnotationShape.svelte— newisResizableprop conditionally renders the overlayAnnotationLayer.svelte— computesisResizable = canDraw && id === activeAnnotationId && !polygonper annotationPdfViewer.svelte—setContext('annotationUpdate', updateAnnotation), 4s auto-dismiss inline error banner witharia-live="assertive"Key correctness properties:
Test coverage: 891 backend tests + 718 frontend tests, all green.