Merge pull request recogito#123 from oleksandr-danylchenko/spatial-tree-positioning

Made `TextAnnotatorPopup` resilient to the underlying DOM mutations

Author: rsimon

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

@@ -1,6 +1,13 @@
 import { ReactNode, useEffect, useMemo, useRef, useState } from 'react';
 import { useAnnotator, useSelection } from '@annotorious/react';
-import { isRevived, NOT_ANNOTATABLE_CLASS, TextAnnotation, TextAnnotator } from '@recogito/text-annotator';
+import {
+  NOT_ANNOTATABLE_CLASS,
+  denormalizeRectWithOffset,
+  toDomRectList,
+  type TextAnnotation,
+  type TextAnnotator,
+} from '@recogito/text-annotator';
+
 import { isMobile } from './isMobile';
 import {
   arrow,
@@ -64,8 +71,8 @@ export const TextAnnotationPopup = (props: TextAnnotationPopupProps) => {
       shift({ crossAxis: true, padding: 10 })
     ];
 
-    return props.arrow 
-      ? [...m, arrow({ element: arrowRef }) ] 
+    return props.arrow
+      ? [...m, arrow({ element: arrowRef }) ]
       : m;
   }, [props.arrow]);
 
@@ -89,26 +96,37 @@ export const TextAnnotationPopup = (props: TextAnnotationPopupProps) => {
   const { getFloatingProps } = useInteractions([dismiss, role]);
 
   useEffect(() => {
-    const annotationSelector = annotation?.target.selector;
-    setOpen(annotationSelector?.length > 0 ? isRevived(annotationSelector) : false);
-  }, [annotation]);
+    if (annotation?.id) {
+      const bounds = r?.state.store.getAnnotationBounds(annotation.id);
+      setOpen(Boolean(bounds));
+    } else {
+      setOpen(false);
+    }
+  }, [annotation?.id, r?.state.store]);
 
   useEffect(() => {
-    if (isOpen && annotation) {
-      const {
-        target: {
-          selector: [{ range }]
-        }
-      } = annotation;
+    if (!r) return;
 
+    if (isOpen && annotation?.id) {
       refs.setPositionReference({
-        getBoundingClientRect: () => range.getBoundingClientRect(),
-        getClientRects: () => range.getClientRects()
+        getBoundingClientRect: () => {
+          const bounds = r.state.store.getAnnotationBounds(annotation.id);
+          return bounds
+            ? denormalizeRectWithOffset(bounds, r.element.getBoundingClientRect())
+            : new DOMRect();
+        },
+        getClientRects: () => {
+          const rects = r.state.store.getAnnotationRects(annotation.id);
+          const denormalizedRects = rects.map((rect) =>
+            denormalizeRectWithOffset(rect, r.element.getBoundingClientRect())
+          );
+          return toDomRectList(denormalizedRects);
+        }
       });
     } else {
       refs.setPositionReference(null);
     }
-  }, [isOpen, annotation, refs]);
+  }, [isOpen, annotation?.id, annotation?.target, r]);
 
   useEffect(() => {
     const config: MutationObserverInit = { attributes: true, childList: true, subtree: true };
@@ -151,9 +169,9 @@ export const TextAnnotationPopup = (props: TextAnnotationPopupProps) => {
           })}
 
           {props.arrow && (
-            <FloatingArrow 
+            <FloatingArrow
               ref={arrowRef}
-              context={context} 
+              context={context}
               {...(props.arrowProps || {})} />
           )}
 

packages/text-annotator/src/state/TextAnnotationStore.ts

@@ -11,7 +11,9 @@ export interface TextAnnotationStore<T extends TextAnnotation = TextAnnotation>
 
   bulkUpsertAnnotations(annotations: T[], origin?: Origin): T[];
 
-  getAnnotationBounds(id: string, hintX?: number, hintY?: number, buffer?: number): DOMRect;
+  getAnnotationRects(id: string): DOMRect[];
+
+  getAnnotationBounds(id: string, hintX?: number, hintY?: number, buffer?: number): DOMRect | undefined;
 
   getAnnotationRects(id: string): DOMRect[];
 

packages/text-annotator/src/state/TextAnnotatorState.ts

@@ -13,7 +13,7 @@ import type {
 } from '@annotorious/core';
 import { createSpatialTree } from './spatialTree';
 import type { TextAnnotation, TextAnnotationTarget } from '../model';
-import type { TextAnnotationStore } from './TextAnnotationStore';
+import type { AnnotationRects, TextAnnotationStore } from './TextAnnotationStore';
 import { isRevived, reviveAnnotation, reviveTarget } from '../utils';
 
 export interface TextAnnotatorState<I extends TextAnnotation = TextAnnotation, E extends unknown = TextAnnotation> extends AnnotatorState<I, E> {
@@ -114,7 +114,7 @@ export const createTextAnnotatorState = <I extends TextAnnotation = TextAnnotati
     return all ? filtered : filtered[0];
   }
 
-  const getAnnotationBounds = (id: string, x?: number, y?: number, buffer = 5): DOMRect => {
+  const getAnnotationBounds = (id: string, x?: number, y?: number, buffer = 5): DOMRect | undefined => {
     const rects = tree.getAnnotationRects(id);
     if (rects.length === 0) return;
 
@@ -129,6 +129,13 @@ export const createTextAnnotatorState = <I extends TextAnnotation = TextAnnotati
     return tree.getAnnotationBounds(id);
   }
 
+  const getIntersecting = (
+    minX: number,
+    minY: number,
+    maxX: number,
+    maxY: number,
+  ): AnnotationRects<I>[] => tree.getIntersecting(minX, minY, maxX, maxY);
+
   const getAnnotationRects = (id: string): DOMRect[] => tree.getAnnotationRects(id);
 
   const recalculatePositions = () => tree.recalculate();
@@ -157,8 +164,8 @@ export const createTextAnnotatorState = <I extends TextAnnotation = TextAnnotati
       bulkUpsertAnnotations,
       getAnnotationBounds,
       getAnnotationRects,
+      getIntersecting,
       getAt,
-      getIntersecting: tree.getIntersecting,
       recalculatePositions,
       updateTarget
     },

packages/text-annotator/src/state/spatialTree.ts

@@ -1,8 +1,12 @@
 import RBush from 'rbush';
 import type { Store } from '@annotorious/core';
 import type { TextAnnotation, TextAnnotationTarget } from '../model';
-import { isRevived, mergeClientRects } from '../utils';
-import { reviveSelector } from '../utils';
+import {
+  isRevived,
+  reviveSelector,
+  mergeClientRects,
+  normalizeRectWithOffset
+} from '../utils';
 import type { AnnotationRects } from './TextAnnotationStore';
 
 interface IndexedHighlightRect {
@@ -38,11 +42,11 @@ export const createSpatialTree = <T extends TextAnnotation>(store: Store<T>, con
       return Array.from(revivedRange.getClientRects());
     });
 
-    const merged = mergeClientRects(rects)
-      // Offset the merged client rects so that coords
-      // are relative to the parent container
-      .map(({ left, top, right, bottom }) =>  
-        new DOMRect(left - offset.left, top - offset.top, right - left, bottom - top));
+    /**
+     * Offset the merged client rects so that coords
+     * are relative to the parent container
+     */
+    const merged = mergeClientRects(rects).map(rect => normalizeRectWithOffset(rect, offset));
 
     return merged.map(rect => {
       const { x, y, width, height } = rect;
@@ -145,7 +149,7 @@ export const createSpatialTree = <T extends TextAnnotation>(store: Store<T>, con
 
   const getAnnotationRects = (id: string): DOMRect[] => {
     const indexed = index.get(id);
-    if (indexed) {
+    if (indexed[0]) {
       // Reminder: *each* IndexedHighlightRect stores *all*
       // DOMRects for the annotation for convenience
       return indexed[0].annotation.rects;

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

@@ -14,5 +14,6 @@ export * from './reviveSelector';
 export * from './reviveTarget';
 export * from './splitAnnotatableRanges';
 export * from './trimRangeToContainer';
-
+export * from './cloneEvents';
+export * from './normalizeRects';
 

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

@@ -65,7 +65,7 @@ const union = (a: DOMRect, b: DOMRect): DOMRect => {
   return new DOMRect(left, top, right - left, bottom - top);
 }
 
-export const mergeClientRects = (rects: DOMRect[]) => rects.reduce((merged, rectA) => {
+export const mergeClientRects = (rects: DOMRect[]) => rects.reduce<DOMRect[]>((merged, rectA) => {
   // Some browser report empty rects - discard
   if (rectA.width === 0 || rectA.height === 0)
     return merged;
@@ -102,7 +102,16 @@ export const mergeClientRects = (rects: DOMRect[]) => rects.reduce((merged, rect
   }
 
   return wasMerged ? next : [ ...next, rectA ];
-}, [] as DOMRect[]);
+}, []);
+
+export const toDomRectList = (rects: DOMRect[]): DOMRectList => ({
+  length: rects.length,
+  item: (index) => rects[index],
+  [Symbol.iterator]: function* (): ArrayIterator<DOMRect> {
+    for (let i = 0; i < this.length; i++)
+      yield this.item(i)!;
+  }
+})
 
 /* Pixels that rects can be apart vertically while still
 // being considered to be on the same line.
@@ -142,7 +151,7 @@ export const mergeClientRects = (rects: DOMRect[]) => {
   }).filter(r => r.height > 0 && r.width > 0);
 
   // Checks if the given rect contains any other rects
-  const containsOthers = (rect: DOMRect) => mergedRects.some(other => 
+  const containsOthers = (rect: DOMRect) => mergedRects.some(other =>
     other !== rect &&
     other.left >= rect.left &&
     other.right <= rect.right &&

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

@@ -0,0 +1,9 @@
+export const normalizeRectWithOffset = (rect: DOMRect, offset: DOMRect): DOMRect => {
+  const { left, top, right, bottom } = rect;
+  return new DOMRect(left - offset.left, top - offset.top, right - left, bottom - top);
+};
+
+export const denormalizeRectWithOffset = (rect: DOMRect, offset: DOMRect): DOMRect => {
+  const { left, top, right, bottom } = rect;
+  return new DOMRect(left + offset.left, top + offset.top, right - left, bottom - top);
+}