Merge pull request #970 from MuckRock/allanlasser/issue969

Fix redaction box drawing logic
MuckRock · Dec 12, 2024 · 9a61b20 · 9a61b20
2 parents d76b514 + f416051
commit 9a61b20
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 44 deletions.
diff --git a/src/lib/components/viewer/AnnotationLayer.svelte b/src/lib/components/viewer/AnnotationLayer.svelte
@@ -98,10 +98,10 @@ Assumes it's a child of a ViewerContext
     const movingRight = x > startX;
     const movingDown = y > startY;
 
-    const x1 = movingRight ? $newNote.x1 : x;
-    const x2 = movingRight ? x : $newNote.x2;
-    const y1 = movingDown ? $newNote.y1 : y;
-    const y2 = movingDown ? y : $newNote.y2;
+    const x1 = movingRight ? startX : x;
+    const x2 = movingRight ? x : startX;
+    const y1 = movingDown ? startY : y;
+    const y2 = movingDown ? y : startY;
 
     $newNote = {
       x1,

diff --git a/src/lib/components/viewer/RedactionLayer.svelte b/src/lib/components/viewer/RedactionLayer.svelte
@@ -41,73 +41,77 @@ It's layered over a PDF page and allows us to render redactions and draw new one
   );
 
   // handle interaction events
-  let dragging = false;
-
-  function pointerdown(e: PointerEvent) {
-    if (!active) return;
-
-    dragging = true;
+  let drawStart: Nullable<[x: number, y: number]> = null;
+  let drawing = false;
 
+  function getLayerPosition(e: PointerEvent): [x: number, y: number] {
+    // pointer position in window
     const { offsetX, offsetY } = e;
-    const { clientWidth, clientHeight } = e.target as HTMLElement;
-
-    currentRedaction = {
-      page_number,
-      x1: offsetX / clientWidth,
-      x2: offsetX / clientWidth,
-      y1: offsetY / clientHeight,
-      y2: offsetY / clientHeight,
-    };
+    // page dimensions
+    const { clientWidth, clientHeight } = e.target as HTMLDivElement;
+    // box points
+    return [offsetX / clientWidth, offsetY / clientHeight];
   }
 
-  function pointermove(e: PointerEvent) {
-    if (!dragging || !active || !currentRedaction) return;
-
-    const { offsetX, offsetY } = e;
-    const { clientWidth, clientHeight } = e.target as HTMLElement;
+  function startDrawingBox(e: PointerEvent) {
+    if (!active) return;
 
-    const x = offsetX / clientWidth;
-    const y = offsetY / clientHeight;
+    const [x, y] = getLayerPosition(e);
 
+    drawing = true;
+    drawStart = [x, y];
+    // at the beginning, the box is just a point
     currentRedaction = {
       page_number,
-      x1: Math.min(currentRedaction.x1, x),
-      x2: Math.max(currentRedaction.x2, x),
-      y1: Math.min(currentRedaction.y1, y),
-      y2: Math.max(currentRedaction.y2, y),
+      x1: x,
+      x2: x,
+      y1: y,
+      y2: y,
     };
   }
 
-  function pointerup(e: PointerEvent) {
-    dragging = false;
-    if (!currentRedaction) return;
+  function continueDrawingBox(e: PointerEvent) {
+    if (!active || !drawing || !drawStart || !currentRedaction) return;
 
-    const { offsetX, offsetY } = e;
-    const { clientWidth, clientHeight } = e.target as HTMLElement;
+    const [x, y] = getLayerPosition(e);
+    const [startX, startY] = drawStart;
+
+    const movingRight = x > startX;
+    const movingDown = y > startY;
 
-    const x = offsetX / clientWidth;
-    const y = offsetY / clientHeight;
+    const x1 = movingRight ? startX : x;
+    const x2 = movingRight ? x : startX;
+    const y1 = movingDown ? startY : y;
+    const y2 = movingDown ? y : startY;
 
     currentRedaction = {
       page_number,
-      x1: Math.min(currentRedaction.x1, x),
-      x2: Math.max(currentRedaction.x2, x),
-      y1: Math.min(currentRedaction.y1, y),
-      y2: Math.max(currentRedaction.y2, y),
+      x1,
+      x2,
+      y1,
+      y2,
     };
+  }
+
+  function finishDrawingBox(e: PointerEvent) {
+    if (!active || !drawing || !currentRedaction) return;
 
     $redactions = [...$redactions, currentRedaction];
+
+    // reset drawing state
     currentRedaction = null;
+    drawStart = null;
+    drawing = false;
   }
 </script>
 
 <div
   class="redactions"
   class:active
   bind:this={container}
-  on:pointerdown={pointerdown}
-  on:pointermove={pointermove}
-  on:pointerup={pointerup}
+  on:pointerdown={startDrawingBox}
+  on:pointermove={continueDrawingBox}
+  on:pointerup={finishDrawingBox}
 >
   {#each redactions_for_page as redaction}
     <span