Merge branch 'recogito#169-fix-repeated-update-emission' into staging…

…-testing # Conflicts: # packages/text-annotator/src/SelectionHandler.ts
oleksandr-danylchenko · Nov 6, 2024 · 6c09045 · 6c09045
2 parents aab70fb + 7eb3603
commit 6c09045
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 38 deletions.
diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts
@@ -104,9 +104,10 @@ export const createSelectionHandler = (
     }
 
     /**
-     * This is to handle cases where the selection is "hijacked" by another element
-     * in a not-annotatable area. A rare case in theory. But rich text editors
-     * will like Quill do it...
+     * This is to handle cases where the selection is "hijacked"
+     * by another element in a not-annotatable area.
+     * A rare case in theory.
+     * But rich text editors will like Quill do it.
      */
     if (isNotAnnotatable(sel.anchorNode)) {
       currentTarget = undefined;
@@ -162,7 +163,6 @@ export const createSelectionHandler = (
     const hasChanged =
       annotatableRanges.length !== currentTarget.selector.length ||
       annotatableRanges.some((r, i) => r.toString() !== currentTarget.selector[i]?.quote);
-
     if (!hasChanged) return;
 
     currentTarget = {
@@ -172,8 +172,8 @@ export const createSelectionHandler = (
     };
 
     /**
-     * During mouse selection on the desktop, annotation won't usually exist while the selection is being edited.
-     * But it will be typical during keyboard or mobile handlebars selection!
+     * During mouse selection on the desktop, the annotation won't usually exist while the selection is being edited.
+     * But it'll be typical during selection via the keyboard or mobile's handlebars.
      */
     if (store.getAnnotation(currentTarget.annotation)) {
       store.updateTarget(currentTarget, Origin.LOCAL);
@@ -184,7 +184,7 @@ export const createSelectionHandler = (
   }, 10);
 
   /**
-   * Select events don't carry information about the mouse button
+   * Select events don't carry information about the mouse button.
    * Therefore, to prevent right-click selection, we need to listen
    * to the initial pointerdown event and remember the button
    */
@@ -199,20 +199,6 @@ export const createSelectionHandler = (
     isLeftClick = lastDownEvent.button === 0;
   };
 
-  // Helper
-  const upsertCurrentTarget = () => {
-    const exists = store.getAnnotation(currentTarget.annotation);
-    if (exists) {
-      store.updateTarget(currentTarget);
-    } else {
-      store.addAnnotation({
-        id: currentTarget.annotation,
-        bodies: [],
-        target: currentTarget
-      });
-    }
-  }
-
   const onPointerUp = async (evt: PointerEvent) => {
     if (isNotAnnotatable(evt.target as Node) || !isLeftClick) return;
 
@@ -369,6 +355,29 @@ export const createSelectionHandler = (
 
   hotkeys(ARROW_KEYS.join(','), { keydown: true, keyup: false }, handleArrowKeyPress);
 
+  // Helper
+  const upsertCurrentTarget = () => {
+    const existingAnnotation = store.getAnnotation(currentTarget.annotation);
+    if (!existingAnnotation) {
+      store.addAnnotation({
+        id: currentTarget.annotation,
+        bodies: [],
+        target: currentTarget
+      });
+      return;
+    }
+
+    const { target: { updated: existingTargetUpdated } } = existingAnnotation;
+    const { updated: currentTargetUpdated } = currentTarget;
+    if (
+      !existingAnnotation ||
+      !currentTargetUpdated ||
+      existingTargetUpdated < currentTargetUpdated
+    ) {
+      store.updateTarget(currentTarget);
+    }
+  };
+
   document.addEventListener('pointerdown', onPointerDown);
   document.addEventListener('pointerup', onPointerUp);
   document.addEventListener('contextmenu', onContextMenu);

diff --git a/packages/text-annotator/src/model/w3c/W3CTextFormatAdapter.ts b/packages/text-annotator/src/model/w3c/W3CTextFormatAdapter.ts
@@ -153,7 +153,7 @@ export const serializeW3CTextAnnotation = (
     return {
       ...targetRest,
       id,
-      // @ts-expect-error: `scope` is not part of the core `TextAnnotationTarget` type
+      // @ts-expect-error: `scope` is not part of the core `TextSelector` type
       scope: 'scope' in s ? s.scope : undefined,
       source,
       selector: w3cSelectors

diff --git a/packages/text-annotator/src/state/TextAnnotatorState.ts b/packages/text-annotator/src/state/TextAnnotatorState.ts
@@ -65,19 +65,9 @@ export const createTextAnnotatorState = <I extends TextAnnotation = TextAnnotati
 
     // Initial page load might take some time. Retry for more robustness.
     const couldNotRevive = revived.filter(a => !isRevived(a.target.selector));
-    if (couldNotRevive.length > 0) {
-      console.warn('Could not revive all targets for these annotations:', couldNotRevive);
-
-      // Note: we want to keep ALL annotations in the store, even those that
-      // were not revived - even if the highlighter won't be able to render
-      // the un-revived ones to the screen.
-      store.bulkAddAnnotation(revived, replace, origin);
-
-      return couldNotRevive;
-    } else {
-      store.bulkAddAnnotation(revived, replace, origin);
-      return [];
-    }
+    store.bulkAddAnnotation(revived, replace, origin);
+
+    return couldNotRevive;
   }
 
   const bulkUpsertAnnotations = (
@@ -88,9 +78,7 @@ export const createTextAnnotatorState = <I extends TextAnnotation = TextAnnotati
 
     // Initial page load might take some time. Retry for more robustness.
     const couldNotRevive = revived.filter(a => !isRevived(a.target.selector));
-    if (couldNotRevive.length > 0)
-      console.warn('Could not revive all targets for these annotations:', couldNotRevive);
-
+
     revived.forEach(a => {
       if (store.getAnnotation(a.id))
         store.updateAnnotation(a, origin);