From 431bfc90cb68771643708ad840a9d0c713c3b7ee Mon Sep 17 00:00:00 2001 From: Devinda Senanayaka <3579142+dca123@users.noreply.github.com> Date: Fri, 28 Aug 2026 10:29:44 +0530 Subject: [PATCH 1/2] fix: prevent triple-click highlight bleed --- packages/ui/hooks/useAnnotationHighlighter.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/ui/hooks/useAnnotationHighlighter.ts b/packages/ui/hooks/useAnnotationHighlighter.ts index c32df6fcb..f7461dcd9 100644 --- a/packages/ui/hooks/useAnnotationHighlighter.ts +++ b/packages/ui/hooks/useAnnotationHighlighter.ts @@ -7,6 +7,7 @@ import { useEffect, useRef, useState, useCallback, type RefObject } from 'react'; import Highlighter from '@plannotator/web-highlighter'; +import type { SelectedNode } from '@plannotator/web-highlighter/dist/types'; import type { Annotation, EditorMode, ImageAttachment } from '../types'; import { AnnotationType } from '../types'; import type { QuickLabel } from '../utils/quickLabels'; @@ -99,6 +100,20 @@ const selectionContainsNode = (range: Range, node: Node): boolean => { } }; +const trimWhitespaceOnlyBoundaryNodes = (selectedNodes: SelectedNode[]): SelectedNode[] => { + let start = 0; + while (start < selectedNodes.length && !/\S/.test(selectedNodes[start].$node.textContent ?? '')) { + start += 1; + } + + let end = selectedNodes.length; + while (end > start && !/\S/.test(selectedNodes[end - 1].$node.textContent ?? '')) { + end -= 1; + } + + return selectedNodes.slice(start, end); +}; + const mathSourceFromElement = (element: HTMLElement): MathAnnotationSource | null => { const text = element.dataset.mathTex; if (!text) return null; @@ -845,6 +860,12 @@ export function useAnnotationHighlighter({ style: { className: 'annotation-highlight' }, }); + // Chromium can extend a triple-clicked line into empty or indentation nodes of the next block. + // Trim only boundary whitespace so spacing inside genuine multi-node selections stays highlighted. + highlighter.hooks.Render.SelectedNodes.tap((_id, selectedNodes: SelectedNode[]) => + trimWhitespaceOnlyBoundaryNodes(selectedNodes), + ); + highlighterRef.current = highlighter; highlighter.on(Highlighter.event.CREATE, ({ sources, type }: { sources: any[]; type?: string }) => { From 53fb3593812c4275fee44b8fe5b4cc367ec72367 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 28 Aug 2026 08:46:28 -0700 Subject: [PATCH 2/2] fix(ui): satisfy HookCallback signature, drop deep dist import, add boundary-trim test --- packages/ui/hooks/useAnnotationHighlighter.ts | 28 +++----- packages/ui/utils/selectionBoundary.test.ts | 70 +++++++++++++++++++ packages/ui/utils/selectionBoundary.ts | 40 +++++++++++ 3 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 packages/ui/utils/selectionBoundary.test.ts create mode 100644 packages/ui/utils/selectionBoundary.ts diff --git a/packages/ui/hooks/useAnnotationHighlighter.ts b/packages/ui/hooks/useAnnotationHighlighter.ts index f7461dcd9..58c9e5605 100644 --- a/packages/ui/hooks/useAnnotationHighlighter.ts +++ b/packages/ui/hooks/useAnnotationHighlighter.ts @@ -7,10 +7,13 @@ import { useEffect, useRef, useState, useCallback, type RefObject } from 'react'; import Highlighter from '@plannotator/web-highlighter'; -import type { SelectedNode } from '@plannotator/web-highlighter/dist/types'; import type { Annotation, EditorMode, ImageAttachment } from '../types'; import { AnnotationType } from '../types'; import type { QuickLabel } from '../utils/quickLabels'; +import { + trimWhitespaceOnlyBoundaryNodes, + type SelectedNodeLike, +} from '../utils/selectionBoundary'; import { getIdentity } from '../utils/identity'; import { transformPlainText } from '../utils/inlineTransforms'; @@ -100,20 +103,6 @@ const selectionContainsNode = (range: Range, node: Node): boolean => { } }; -const trimWhitespaceOnlyBoundaryNodes = (selectedNodes: SelectedNode[]): SelectedNode[] => { - let start = 0; - while (start < selectedNodes.length && !/\S/.test(selectedNodes[start].$node.textContent ?? '')) { - start += 1; - } - - let end = selectedNodes.length; - while (end > start && !/\S/.test(selectedNodes[end - 1].$node.textContent ?? '')) { - end -= 1; - } - - return selectedNodes.slice(start, end); -}; - const mathSourceFromElement = (element: HTMLElement): MathAnnotationSource | null => { const text = element.dataset.mathTex; if (!text) return null; @@ -862,9 +851,12 @@ export function useAnnotationHighlighter({ // Chromium can extend a triple-clicked line into empty or indentation nodes of the next block. // Trim only boundary whitespace so spacing inside genuine multi-node selections stays highlighted. - highlighter.hooks.Render.SelectedNodes.tap((_id, selectedNodes: SelectedNode[]) => - trimWhitespaceOnlyBoundaryNodes(selectedNodes), - ); + // The hook's callback type is `(...args: unknown[]) => SelectedNode[]`; a SelectedNode + // structurally satisfies SelectedNodeLike, so the trimmed subset goes back through the + // hook's own callback type rather than importing SelectedNode from the package's dist/. + type SelectedNodesTap = Parameters[0]; + highlighter.hooks.Render.SelectedNodes.tap(((...args: unknown[]) => + trimWhitespaceOnlyBoundaryNodes(args[1] as SelectedNodeLike[])) as SelectedNodesTap); highlighterRef.current = highlighter; diff --git a/packages/ui/utils/selectionBoundary.test.ts b/packages/ui/utils/selectionBoundary.test.ts new file mode 100644 index 000000000..ce3c57764 --- /dev/null +++ b/packages/ui/utils/selectionBoundary.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from 'bun:test'; +import { trimWhitespaceOnlyBoundaryNodes, type SelectedNodeLike } from './selectionBoundary'; + +// Failure this guards: a triple-click leaves the selection's end boundary on the +// NEXT block's text node at offset 0, so web-highlighter's splitText(0) hands the +// painter an empty (or pure-indentation) boundary node. Wrapping that node in a +// styled paints a visible sliver on the following line. A boundary node +// with no non-whitespace character must never reach the painter, while +// whitespace BETWEEN real nodes must survive, or genuine multi-node selections +// lose their inter-node spacing. +// +// Lives here rather than in useAnnotationHighlighter.test.tsx because that suite +// is DOM-gated (skipped unless DOM_TESTS=1) and so guards nothing in CI. + +const nodes = (...texts: (string | null)[]): SelectedNodeLike[] => + texts.map((textContent) => ({ $node: { textContent } })); + +const texts = (list: SelectedNodeLike[]): (string | null)[] => + list.map((node) => node.$node.textContent); + +describe('trimWhitespaceOnlyBoundaryNodes', () => { + test('drops an empty trailing node (the splitText(0) sliver)', () => { + expect(texts(trimWhitespaceOnlyBoundaryNodes(nodes('a line of text', '')))).toEqual([ + 'a line of text', + ]); + }); + + test('drops a trailing pure-indentation node', () => { + expect(texts(trimWhitespaceOnlyBoundaryNodes(nodes('a line of text', '\n ')))).toEqual([ + 'a line of text', + ]); + }); + + test('drops leading whitespace-only nodes', () => { + expect(texts(trimWhitespaceOnlyBoundaryNodes(nodes('\n ', '', 'a line of text')))).toEqual([ + 'a line of text', + ]); + }); + + test('trims both ends at once', () => { + expect( + texts(trimWhitespaceOnlyBoundaryNodes(nodes(' ', 'first', 'second', '\n\t'))), + ).toEqual(['first', 'second']); + }); + + test('keeps whitespace-only nodes between content nodes', () => { + expect(texts(trimWhitespaceOnlyBoundaryNodes(nodes('first', ' ', 'second')))).toEqual([ + 'first', + ' ', + 'second', + ]); + }); + + test('returns nothing when every node is whitespace-only', () => { + expect(trimWhitespaceOnlyBoundaryNodes(nodes('', ' ', '\n '))).toEqual([]); + }); + + test('leaves an ordinary selection untouched', () => { + const selection = nodes('first', 'second', 'third'); + expect(trimWhitespaceOnlyBoundaryNodes(selection)).toEqual(selection); + }); + + test('treats a null textContent as whitespace-only', () => { + expect(texts(trimWhitespaceOnlyBoundaryNodes(nodes(null, 'text', null)))).toEqual(['text']); + }); + + test('handles an empty node list', () => { + expect(trimWhitespaceOnlyBoundaryNodes([])).toEqual([]); + }); +}); diff --git a/packages/ui/utils/selectionBoundary.ts b/packages/ui/utils/selectionBoundary.ts new file mode 100644 index 000000000..bef3284c1 --- /dev/null +++ b/packages/ui/utils/selectionBoundary.ts @@ -0,0 +1,40 @@ +/** + * Boundary trimming for web-highlighter's `Render.SelectedNodes` hook. + * + * Lives in its own pure module so it can be unit-tested without a DOM: + * useAnnotationHighlighter imports @plannotator/web-highlighter, whose UMD + * bundle reads `window` at module-eval time. + */ + +/** + * Minimal structural view of web-highlighter's `SelectedNode`: only the field + * the trim actually reads. Declared locally rather than imported from the + * package's `dist/`, which resolves today only because the fork ships no + * `exports` map; a republish that adds one would break consumers compiling + * @plannotator/ui from source. + */ +export interface SelectedNodeLike { + $node: { textContent: string | null }; +} + +/** + * Drop whitespace-only nodes from the leading and trailing ends of the node + * list web-highlighter is about to wrap. Whitespace-only nodes in the MIDDLE of + * a genuine multi-node selection are kept, so inter-node spacing stays + * highlighted. + */ +export const trimWhitespaceOnlyBoundaryNodes = ( + selectedNodes: SelectedNodeLike[], +): SelectedNodeLike[] => { + let start = 0; + while (start < selectedNodes.length && !/\S/.test(selectedNodes[start].$node.textContent ?? '')) { + start += 1; + } + + let end = selectedNodes.length; + while (end > start && !/\S/.test(selectedNodes[end - 1].$node.textContent ?? '')) { + end -= 1; + } + + return selectedNodes.slice(start, end); +};