Merge pull request #157 from oleksandr-danylchenko/fix-not-annotatable-elements-check

⚠️ Fixed false-negative `.not-annotatable` presence check ⚠️

Author: rsimon

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;
     }
@@ -163,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`
@@ -191,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 = () => {

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

@@ -2,7 +2,6 @@ export * from './cancelSingleClickEvents';
 export * from './device';
 export * from './programmaticallyFocusable';
 export * from './debounce';
-export * from './getAnnotatableFragment';
 export * from './getQuoteContext';
 export * from './isWhitespaceOrEmpty';
 export * from './isRevived';

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

@@ -2,11 +2,16 @@ export const NOT_ANNOTATABLE_CLASS = 'not-annotatable';
 
 export const NOT_ANNOTATABLE_SELECTOR = `.${NOT_ANNOTATABLE_CLASS}`;
 
-const isRangeAnnotatable = (range: Range): boolean => {
+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 ancestor instanceof HTMLElement
-    ? !ancestor.closest(NOT_ANNOTATABLE_SELECTOR)
-    : !ancestor.parentElement?.closest(NOT_ANNOTATABLE_SELECTOR);
+  return !isNotAnnotatable(ancestor);
 }
 
 const iterateNotAnnotatableElements = function*(range: Range): Generator<HTMLElement> {