Commit ed376112 authored by Robert Knight's avatar Robert Knight

Remove jQuery dependency from highlighter and add test for multiple text nodes

As part of an ongoing project to remove the annotator's jQuery
dependency, rewrite the highlighter to avoid it.

This change is also preparation for some upcoming changes to make
highlights more friendly for screen reader users by reducing the
number of `<hypothesis-highlight>` elements created for a run of text in
various situations. Since each `<hypothesis-highlight>` announces its
presence to the screen reader, this gets "noisy" if there are many such
elements.
parent 0debe003
import $ from 'jquery';
/**
* Wraps the DOM Nodes within the provided range with a highlight
* element of the specified class and returns the highlight Elements.
......@@ -11,25 +9,45 @@ import $ from 'jquery';
export function highlightRange(normedRange, cssClass = 'hypothesis-highlight') {
const white = /^\s*$/;
// A custom element name is used here rather than `<span>` to reduce the
// likelihood of highlights being hidden by page styling.
const hl = $(
`<hypothesis-highlight class='${cssClass}'></hypothesis-highlight>`
);
// Find text nodes within the range to highlight.
//
// We ignore text nodes that contain only whitespace to avoid inserting
// elements in places that can only contain a restricted subset of nodes such
// as table rows and lists. This does mean that there may be the odd abandoned
// whitespace node in a paragraph that is skipped but better than breaking
// table layouts.
const textNodes = normedRange
.textNodes()
.filter(node => !white.test(node.nodeValue));
// Wrap each text node with a `<hypothesis-highlight>` element.
const highlights = [];
textNodes.forEach(node => {
// A custom element name is used here rather than `<span>` to reduce the
// likelihood of highlights being hidden by page styling.
const highlightEl = document.createElement('hypothesis-highlight');
highlightEl.className = cssClass;
// Ignore text nodes that contain only whitespace characters. This prevents
// spans being injected between elements that can only contain a restricted
// subset of nodes such as table rows and lists. This does mean that there
// may be the odd abandoned whitespace node in a paragraph that is skipped
// but better than breaking table layouts.
const nodes = $(normedRange.textNodes()).filter(function() {
return !white.test(this.nodeValue);
node.parentNode.replaceChild(highlightEl, node);
highlightEl.appendChild(node);
highlights.push(highlightEl);
});
return nodes
.wrap(hl)
.parent()
.toArray();
return highlights;
}
/**
* Replace a child `node` with `replacements`.
*
* nb. This is like `ChildNode.replaceWith` but it works in IE 11.
*
* @param {Node} node
* @param {Node[]} replacements
*/
function replaceWith(node, replacements) {
const parent = node.parentNode;
replacements.forEach(r => parent.insertBefore(r, node));
node.remove();
}
/**
......@@ -40,7 +58,8 @@ export function highlightRange(normedRange, cssClass = 'hypothesis-highlight') {
export function removeHighlights(highlights) {
for (let h of highlights) {
if (h.parentNode) {
$(h).replaceWith(h.childNodes);
const children = Array.from(h.childNodes);
replaceWith(h, children);
}
}
}
......
......@@ -26,6 +26,27 @@ describe('annotator/highlighter', () => {
assert.isTrue(result[0].classList.contains('hypothesis-highlight'));
});
it('wraps multiple text nodes', () => {
const strings = ['hello', ' Brave ', ' New ', ' World'];
const textNodes = strings.map(s => document.createTextNode(s));
const el = document.createElement('span');
textNodes.forEach(n => el.append(n));
const r = new Range.NormalizedRange({
commonAncestor: el,
start: textNodes[0],
end: textNodes[textNodes.length - 1],
});
const result = highlightRange(r);
assert.equal(result.length, textNodes.length);
result.forEach((highlight, i) => {
assert.equal(highlight.nodeName, 'HYPOTHESIS-HIGHLIGHT');
assert.deepEqual(Array.from(highlight.childNodes), [textNodes[i]]);
});
});
it('skips text nodes that are only white space', () => {
const txt = document.createTextNode('one');
const blank = document.createTextNode(' ');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment