Add documentation for highlight and fix bug

Issue: https://github.com/omnivore-app/omnivore/issues/2996

packages/web/lib/highlights/highlightGenerator.ts

@@ -24,6 +24,13 @@ const maxDeepPatchDistance = 4000
 const maxDeepPatchThreshhold = 0.5
 const maxSurroundingTextLength = 2000
 
+/**
+ * Wrapper for text node
+ *
+ * @property startIndex - offset from the start of article for which the text begins
+ * @property node - the text node
+ * @property startsParagraph - whether a new paragraph is started
+ */
 type TextNode = {
   startIndex: number
   node: Node
@@ -35,12 +42,18 @@ type ArticleTextContent = {
   articleText: string
 }
 
+/**
+ * Location of a highlight as starting/ending offset from the start of article. The end offset is non-inclusive
+ */
 export type HighlightLocation = {
   id: string
   start: number
   end: number
 }
 
+/**
+ * Relevant attributes of a highlight node created in DOM
+ */
 export type HighlightNodeAttributes = {
   prefix: string
   suffix: string
@@ -73,6 +86,16 @@ function nodeAttributesFromHighlight(
   return makeHighlightNodeAttributes(patch, id, withNote, customColor, tooltip)
 }
 
+/**
+ * Make a highlight on the highlight selection and return its attributes
+ *
+ * @param patch - {@link generateDiffPatch|patch} of the highlight location
+ * @param id - highlight id
+ * @param withNote - whether highlight has notes
+ * @param customColor - color of highlight
+ * @param tooltip
+ * @returns relevant highlight attributes
+ */
 export function makeHighlightNodeAttributes(
   patch: string,
   id: string,
@@ -184,6 +207,15 @@ export function makeHighlightNodeAttributes(
   }
 }
 
+/**
+ * Given a text selection by user, annotate the article around the selection and
+ * produce a {@link https://github.com/google/diff-match-patch | diff patch} 
+ * 
+ * The diff patch is used for identifying the selection/highlight location
+ * 
+ * @param range text selection range
+ * @returns diff patch
+ */
 export function generateDiffPatch(range: Range): string {
   const articleContentElement = document.getElementById(articleContainerId)
   if (!articleContentElement)
@@ -217,7 +249,12 @@ export function generateDiffPatch(range: Range): string {
   return patch
 }
 
-export function wrapHighlightTagAroundRange(range: Range): [number, number] {
+/**
+ * Retrieve starting and ending offsets to the highlight selection
+ * @param range highlight selection
+ * @returns starting offset and ending offset (non-inclusive)
+ */
+export function retrieveOffsetsForSelection(range: Range): [number, number] {
   const patch = generateDiffPatch(range)
   const { highlightTextStart, highlightTextEnd } =
     selectionOffsetsFromPatch(patch)
@@ -265,6 +302,15 @@ const getArticleTextNodes = (
   return { textNodes, articleText }
 }
 
+/**
+ * Return the offsets to the selection/highlight
+ * 
+ * @param patch {@link generateDiffPatch|diff patch} identifying a selection/highlight location
+ * @returns 
+ * - highlightTextStart - The start of highlight, offset from the start of article by characters
+ * - highlightTextEnd - The end of highlight (non-inclusive), offset from the start of article by characters 
+ * - matchingHighlightContent - the matched highlight
+ */
 const selectionOffsetsFromPatch = (
   patch: string
 ): {

packages/web/lib/highlights/useSelection.tsx

@@ -1,11 +1,21 @@
 import { useCallback, useEffect, useState } from 'react'
 import {
-  wrapHighlightTagAroundRange,
+  retrieveOffsetsForSelection,
   getHighlightElements,
   HighlightLocation,
 } from './highlightGenerator'
 import type { SelectionAttributes } from './highlightHelpers'
 
+/**
+ * Get the range of text with {@link SelectionAttributes} that user has selected
+ * 
+ * Event Handlers for detecting/using selection are registered
+ * 
+ * If the selection overlaps with existing highlights 
+ * 
+ * @param highlightLocations existing highlights
+ * @returns selection range and its setter
+ */
 export function useSelection(
   highlightLocations: HighlightLocation[]
 ): [SelectionAttributes | null, (x: SelectionAttributes | null) => void] {
@@ -55,7 +65,7 @@ export function useSelection(
       }
 
       const { range, isReverseSelected, selection } = result
-      const [selectionStart, selectionEnd] = wrapHighlightTagAroundRange(range)
+      const [selectionStart, selectionEnd] = retrieveOffsetsForSelection(range)
       const rangeRect = rangeToPos(range, isReverseSelected)
 
       let shouldCancelSelection = false
@@ -82,8 +92,8 @@ export function useSelection(
           }
 
           if (
-            selectionStart <= highlightLocation.end &&
-            highlightLocation.start <= selectionEnd
+            selectionStart < highlightLocation.end &&
+            highlightLocation.start < selectionEnd
           ) {
             overlapHighlights.push(highlightLocation)
           }
@@ -234,9 +244,78 @@ async function makeSelectionRange(): Promise<
     range.startContainer === selection.focusNode &&
     range.endOffset === selection.anchorOffset
 
+  /**
+   * Edge case:
+   * If the selection ends on range endContainer (or startContainer in reverse select) but no text is selected (i.e. selection ends at 
+   * an empty area), the preceding text is highlighted due to range normalizing. 
+   * This is a visual bug and would sometimes lead to weird highlight behavior during removal.
+   * See X for more detail
+   */
+  const selectionEndNode = selection.focusNode
+  const selectionEndOffset = selection.focusOffset
+  const selectionStartNode = isReverseSelected ? range.endContainer : range.startContainer
+
+  if (selectionEndNode?.nodeType === Node.TEXT_NODE) {
+    const selectionEndNodeEdgeIndex = isReverseSelected ? selectionEndNode.textContent?.length : 0
+
+    if (selectionStartNode !== selectionEndNode && 
+      selectionEndOffset == selectionEndNodeEdgeIndex) {
+        clipRangeToNearestAnchor(range, selectionEndNode, isReverseSelected) 
+    }
+  } 
+  
   return isRangeAllowed ? { range, isReverseSelected, selection } : undefined
 }
 
+/**
+ * Clip selection range to the beginning/end of the adjacent anchor element
+ * 
+ * @param range selection range
+ * @param selectionEndNode the node where the selection ended at
+ * @param isReverseSelected 
+ */
+const clipRangeToNearestAnchor = (
+  range: Range,
+  selectionEndNode: Node,
+  isReverseSelected: boolean
+) => {
+  let nearestAnchorElement = selectionEndNode.parentElement
+  while (nearestAnchorElement !== null && !nearestAnchorElement.hasAttribute('data-omnivore-anchor-idx')) {
+    nearestAnchorElement = nearestAnchorElement.parentElement;
+  }
+  if (!nearestAnchorElement) {
+    throw Error('Unable to find nearest anchor element for node: ' + selectionEndNode)
+  }
+  let anchorId = Number(nearestAnchorElement.getAttribute('data-omnivore-anchor-idx')!)
+  let adjacentAnchorId, adjacentAnchor, adjacentAnchorOffset
+  if (isReverseSelected) {
+    // move down to find adjacent anchor node and clip at its beginning
+    adjacentAnchorId = anchorId + 1
+    adjacentAnchor = document.querySelectorAll(`[data-omnivore-anchor-idx='${adjacentAnchorId}']`)[0]
+    adjacentAnchorOffset = 0
+    range.setStart(adjacentAnchor, adjacentAnchorOffset)
+  } else {
+    // move up to find adjacent anchor node and clip at its end
+    do {
+      adjacentAnchorId = --anchorId
+      adjacentAnchor = document.querySelectorAll(`[data-omnivore-anchor-idx='${adjacentAnchorId}']`)[0]
+    } while (adjacentAnchor.contains(selectionEndNode))
+    if (adjacentAnchor.textContent) {
+      let lastTextNodeChild = adjacentAnchor.lastChild
+      while (!!lastTextNodeChild && lastTextNodeChild.nodeType !== Node.TEXT_NODE) {
+        lastTextNodeChild = lastTextNodeChild.previousSibling;
+      }
+      adjacentAnchor = lastTextNodeChild
+      adjacentAnchorOffset = adjacentAnchor?.nodeValue?.length ?? 0
+    } else {
+      adjacentAnchorOffset = 0
+    }
+    if (adjacentAnchor) {
+      range.setEnd(adjacentAnchor, adjacentAnchorOffset)
+    }
+  }
+}
+
 export type RangeEndPos = {
   left: number
   top: number
@@ -246,6 +325,13 @@ export type RangeEndPos = {
   height: number
 }
 
+/**
+ * Return coordinates of the screen area occupied by the last line of user selection
+ * 
+ * @param range range of user selection
+ * @param getFirst whether to get first line of user selection. Get last if false (default)
+ * @returns {RangeEndPos} selection coordinates
+ */
 const rangeToPos = (range: Range, getFirst = false): RangeEndPos => {
   if (typeof window === 'undefined' || !range) {
     return { left: 0, top: 0, right: 0, bottom: 0, width: 0, height: 0 }