From b23ddcb6bcb026fa70b2916d7c9d01f4fc4ce6eb Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 27 Dec 2023 19:36:08 -0500 Subject: [PATCH] Add documentation for highlight and fix bug Issue: https://github.com/omnivore-app/omnivore/issues/2996 --- .../web/lib/highlights/highlightGenerator.ts | 48 +++++++++- packages/web/lib/highlights/useSelection.tsx | 94 ++++++++++++++++++- 2 files changed, 137 insertions(+), 5 deletions(-) diff --git a/packages/web/lib/highlights/highlightGenerator.ts b/packages/web/lib/highlights/highlightGenerator.ts index c852d67af..19d1131bb 100644 --- a/packages/web/lib/highlights/highlightGenerator.ts +++ b/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 ): { diff --git a/packages/web/lib/highlights/useSelection.tsx b/packages/web/lib/highlights/useSelection.tsx index fcb02d0dc..f1de3563a 100644 --- a/packages/web/lib/highlights/useSelection.tsx +++ b/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 }