Unverified Commit e8a82607 authored by Robert Knight's avatar Robert Knight Committed by GitHub

Merge pull request #1787 from hypothesis/remove-highlighter-jquery

Remove jQuery dependency from highlighter and add test for multiple text nodes
parents d8ef38ef ed376112
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*$/;
// 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 hl = $(
`<hypothesis-highlight class='${cssClass}'></hypothesis-highlight>`
);
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