Fix excessive reviveSelector execution in spacialTree after splitting the annotatable ranges

[oleksandr-danylchenko]

Current State

In the #34 PR, I added the splitAnnotatableRanges method that "Splits a DOM Range into one or more ranges that span annotatable content only". It works properly and the ranges it captures skip the .not-annotatable elements.

Issue

The problem is that the ranges are generated with the .setStartAfter & .setEndBefore that don't precisely capture the text nodes:

text-annotator-js/packages/text-annotator/src/utils/splitAnnotatableRanges.ts

Lines 44 to 54 in 3a70b9e

	// From the start of the range to the not annotatable element.
	// If selection starts on the non-annotatable element, a collapsed range will be created and ignored.
	if (!prevNotAnnotatable) {
	subRange = range.cloneRange();
	subRange.setEndBefore(notAnnotatable);
	} else {
	// From the previous not annotatable element to the current not annotatable element
	subRange = document.createRange();
	subRange.setStartAfter(prevNotAnnotatable);
	subRange.setEndBefore(notAnnotatable);
	}

Instead, they can capture the preceding/following divs as the start/end containers:

However, the spacialTree doesn't consider such ranges as "valid"! When it generates the client rects, it'll try to revive the range again from its positions:

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

Lines 43 to 49 in 3a70b9e

	const isValidRange =
	s.range instanceof Range &&
	!s.range.collapsed &&
	s.range.startContainer.nodeType === Node.TEXT_NODE &&
	s.range.endContainer.nodeType === Node.TEXT_NODE;
	const revivedRange = isValidRange ? s.range : reviveSelector(s, container).range;

It feels like an excessive computational step, considering that the provided range already covers only the annotatable area and it certainly doesn't improve the performance 🤷🏻‍♂️

Issue Demo

Screen.Recording.2024-02-27.at.10.45.42.mov

Possible Solutions

Don't run the trimRange right on the selection range:

text-annotator-js/packages/text-annotator/src/SelectionHandler.ts

Lines 56 to 58 in 3a70b9e

	const selectionRange = sel.getRangeAt(0);
	const trimmedRange = trimRange(selectionRange.cloneRange())
	const annotatableRanges = splitAnnotatableRanges(trimmedRange);

But on the ranges created after splitting

[oleksandr-danylchenko]

Unfortunately, just running the trimRange on the split ranges doesn't work properly out of the box and creates collapsed ranges:

[oleksandr-danylchenko]

Also, notice how the startContainer wasn't trimmed 👁️

Which brings us back to the #23 issue. Where I pointed out that trimRange uses setEnd instead of setStart:

text-annotator-js/packages/text-annotator/src/utils/trimRange.ts

Lines 11 to 31 in 80c2f92

	if (startContainer.nodeType !== Node.TEXT_NODE) {
	const nextContainer = startContainer.nextSibling || startContainer.parentNode;
	// Get first text child in next container
	const nextText = nextContainer?.nodeType === Node.TEXT_NODE ?
	nextContainer :
	Array.from(nextContainer!.childNodes).filter(n => n.nodeType === Node.TEXT_NODE).shift();
	range.setEnd(nextText!, 0);
	}
	if (endContainer.nodeType !== Node.TEXT_NODE) {
	const prevContainer = endContainer.previousSibling || endContainer.parentNode;
	// Get last text child in previous container
	const prevText = prevContainer?.nodeType === Node.TEXT_NODE ?
	prevContainer :
	Array.from(prevContainer!.childNodes).filter(n => n.nodeType === Node.TEXT_NODE).pop();
	range.setEnd(prevText!, prevText?.textContent?.length || 0);
	}