Merge branch 'main' into recogito#130-spatial-tree-events-emission

packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx

@@ -1,6 +1,6 @@
 import { PointerEvent, ReactNode, useCallback, useEffect, useMemo, useState } from 'react';
 import { useAnnotator, useSelection } from '@annotorious/react';
-import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator';
+import { isRevived, TextAnnotation, TextAnnotator } from '@recogito/text-annotator';
 import { isMobile } from './isMobile';
 import {
   autoUpdate,
@@ -71,15 +71,10 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => {
   const { getFloatingProps } = useInteractions([dismiss, role]);
 
   useEffect(() => {
-    setOpen(
-      // Selected annotation exists and has a selector?
-      annotation?.target.selector && 
-      // Selector not empty? (Annotations from plugins, general defensive programming)
-      annotation.target.selector.length > 0 && 
-      // Range not collapsed? (E.g. lazy loading PDFs. Note that this will have to 
-      // change if we switch from ranges to pre-computed bounds!)
-      !annotation.target.selector[0].range.collapsed
-    );
+    const annotationSelector = annotation?.target.selector;
+    if (!annotationSelector) return;
+
+    setOpen(isRevived(annotationSelector));
   }, [annotation]);
 
   useEffect(() => {
@@ -154,4 +149,4 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => {
     </FloatingPortal>
   ) : null;
 
-}
+}

packages/text-annotator/src/SelectionHandler.ts

@@ -13,7 +13,7 @@ import {
   isMac,
   isWhitespaceOrEmpty,
   trimRangeToContainer,
-  NOT_ANNOTATABLE_SELECTOR
+  isNotAnnotatable
 } from './utils';
 
 const CLICK_TIMEOUT = 300;
@@ -64,13 +64,14 @@ export const SelectionHandler = (
      * be annotatable (like a component popup).
      * Note that Chrome/iOS will sometimes return the root doc as target!
      */
-    const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
-    currentTarget = annotatable ? {
-      annotation: uuidv4(),
-      selector: [],
-      creator: currentUser,
-      created: new Date()
-    } : undefined;
+    currentTarget = isNotAnnotatable(evt.target as Node)
+      ? undefined
+      : {
+        annotation: uuidv4(),
+        selector: [],
+        creator: currentUser,
+        created: new Date()
+      };
   };
 
   const onSelectionChange = debounce((evt: Event) => {
@@ -79,8 +80,7 @@ export const SelectionHandler = (
     // 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...
-    const annotatable = !sel.anchorNode?.parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
-    if (!annotatable) {
+    if (isNotAnnotatable(sel.anchorNode)) {
       currentTarget = undefined;
       return;
     }
@@ -149,6 +149,9 @@ export const SelectionHandler = (
      */
     if (store.getAnnotation(currentTarget.annotation)) {
       store.updateTarget(currentTarget, Origin.LOCAL);
+    } else {
+      // Proper lifecycle management: clear the previous selection first...
+      selection.clear();
     }
   });
 
@@ -160,8 +163,7 @@ export const SelectionHandler = (
   const onPointerDown = (evt: PointerEvent) => {
     if (isContextMenuOpen) return;
 
-    const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
-    if (!annotatable) return;
+    if (isNotAnnotatable(evt.target as Node)) return;
 
     /**
      * Cloning the event to prevent it from accidentally being `undefined`
@@ -188,8 +190,7 @@ export const SelectionHandler = (
   const onPointerUp = (evt: PointerEvent) => {
     if (isContextMenuOpen) return;
 
-    const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
-    if (!annotatable || !isLeftClick) return;
+    if (isNotAnnotatable(evt.target as Node) || !isLeftClick) return;
 
     // Logic for selecting an existing annotation
     const clickSelect = () => {
@@ -206,7 +207,7 @@ export const SelectionHandler = (
         if (selected.length !== 1 || selected[0].id !== hovered.id) {
           selection.userSelect(hovered.id, evt);
         }
-      } else if (!selection.isEmpty()) {
+      } else {
         selection.clear();
       }
     };

packages/text-annotator/src/utils/cancelSingleClickEvents.ts

@@ -1,4 +1,4 @@
-import { NOT_ANNOTATABLE_SELECTOR } from './splitAnnotatableRanges';
+import { NOT_ANNOTATABLE_SELECTOR } from './isNotAnnotatable';
 
 /**
  * Calls .preventDefault() on click events in annotable areas, in order

packages/text-annotator/src/utils/index.ts

@@ -1,17 +1,18 @@
 export * from './cancelSingleClickEvents';
+export * from './cloneEvents';
 export * from './device';
 export * from './programmaticallyFocusable';
 export * from './debounce';
-export * from './getAnnotatableFragment';
 export * from './getQuoteContext';
-export * from './isWhitespaceOrEmpty';
+export * from './isNotAnnotatable';
 export * from './isRevived';
+export * from './isWhitespaceOrEmpty';
 export * from './mergeClientRects';
 export * from './rangeToSelector';
 export * from './reviveAnnotation';
 export * from './reviveSelector';
 export * from './reviveTarget';
 export * from './splitAnnotatableRanges';
 export * from './trimRangeToContainer';
-export * from './cloneEvents';
+
 

packages/text-annotator/src/utils/isNotAnnotatable.ts

@@ -0,0 +1,15 @@
+export const NOT_ANNOTATABLE_CLASS = 'not-annotatable';
+
+export const NOT_ANNOTATABLE_SELECTOR = `.${NOT_ANNOTATABLE_CLASS}`;
+
+export const isNotAnnotatable = (node: Node): boolean => {
+  const closestNotAnnotatable = node instanceof HTMLElement
+    ? node.closest(NOT_ANNOTATABLE_SELECTOR)
+    : node.parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
+  return Boolean(closestNotAnnotatable);
+}
+
+export const isRangeAnnotatable = (range: Range): boolean => {
+  const ancestor = range.commonAncestorContainer;
+  return !isNotAnnotatable(ancestor);
+}

packages/text-annotator/src/utils/reviveSelector.ts

@@ -1,5 +1,5 @@
 import type { TextSelector } from '../model';
-import { NOT_ANNOTATABLE_SELECTOR } from './splitAnnotatableRanges';
+import { NOT_ANNOTATABLE_SELECTOR } from './isNotAnnotatable';
 
 /**
  * Creates a new selector object with the revived DOM range from the given text annotation position

packages/text-annotator/src/utils/splitAnnotatableRanges.ts

@@ -1,13 +1,4 @@
-export const NOT_ANNOTATABLE_CLASS = 'not-annotatable';
-
-export const NOT_ANNOTATABLE_SELECTOR = `.${NOT_ANNOTATABLE_CLASS}`;
-
-const isRangeAnnotatable = (range: Range): boolean => {
-  const ancestor = range.commonAncestorContainer;
-  return ancestor instanceof HTMLElement
-    ? !ancestor.closest(NOT_ANNOTATABLE_SELECTOR)
-    : !ancestor.parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
-}
+import { isRangeAnnotatable, NOT_ANNOTATABLE_CLASS, NOT_ANNOTATABLE_SELECTOR } from './isNotAnnotatable';
 
 const iterateNotAnnotatableElements = function*(range: Range): Generator<HTMLElement> {
   const notAnnotatableIterator = document.createNodeIterator(

packages/text-annotator/test/model/w3c/fixtures.ts

@@ -28,8 +28,8 @@ export const textAnnotation: W3CTextAnnotation = {
         },
         {
           type: 'TextPositionSelector',
-          start: 986,
-          end: 998
+          start: 958,
+          end: 970
         },
       ]
     }