Commit 3b054855 authored by Robert Knight's avatar Robert Knight

Encapsulate logic for anchoring placeholders in a new module

Several different parts of the code need to know about placeholder
elements used to anchor annotations that refer to not-yet-rendered pages
of a document in a PDF. Centralize the logic for creating placeholders
and testing whether a node is inside a placeholder in one module.

A trivial changes it that the "Loading annotations" text now ends with
an ASCII "..." instead of unicode ellipsis "…". This is easier to type
since the text is not intended to be seen by the user.
parent 97822425
/* global PDFViewerApplication */
import { createPlaceholder } from './placeholder';
import { TextPosition, TextRange } from './text-range';
import { TextQuoteAnchor } from './types';
......@@ -309,13 +310,7 @@ async function anchorByPosition(pageIndex, start, end) {
// The page has not been rendered yet. Create a placeholder element and
// anchor to that instead.
let placeholder = page.div.querySelector('.annotator-placeholder');
if (!placeholder) {
placeholder = document.createElement('span');
placeholder.classList.add('annotator-placeholder');
placeholder.textContent = 'Loading annotations…';
page.div.appendChild(placeholder);
}
const placeholder = createPlaceholder(page.div);
const range = document.createRange();
range.setStartBefore(placeholder);
range.setEndAfter(placeholder);
......
/**
* CSS selector that will match the placeholder within a page/tile container.
*/
const placeholderSelector = '.annotator-placeholder';
/**
* Create or return a placeholder element for anchoring.
*
* In document viewers such as PDF.js which only render a subset of long
* documents at a time, it may not be possible to anchor annotations to the
* actual text in pages which are off-screen. Instead the annotations are anchored
* to a placeholder element which is in roughly the correct location, in terms
* of X/Y scroll offset.
*
* When the viewport is scrolled to the region of the document, the placeholder
* will be removed and annotations will be re-anchored to the real content.
*
* @param {HTMLElement} container - The container element for the page or tile
* which is not rendered.
*/
export function createPlaceholder(container) {
let placeholder = container.querySelector(placeholderSelector);
if (placeholder) {
return placeholder;
}
placeholder = document.createElement('span');
placeholder.classList.add('annotator-placeholder');
placeholder.textContent = 'Loading annotations...';
container.appendChild(placeholder);
return placeholder;
}
/**
* Return true if a page/tile container has a placeholder.
*
* @param {HTMLElement} container
*/
export function hasPlaceholder(container) {
return container.querySelector(placeholderSelector) !== null;
}
/**
* Remove the placeholder element in `container`, if present.
*
* @param {HTMLElement} container
*/
export function removePlaceholder(container) {
container.querySelector(placeholderSelector)?.remove();
}
/**
* Return true if `node` is inside a placeholder element created with `createPlaceholder`.
*
* This is typically used to test if a highlight element associated with an
* anchor is inside a placeholder.
*
* @param {Node} node
*/
export function isInPlaceholder(node) {
return node.parentElement?.closest(placeholderSelector) !== null;
}
......@@ -298,7 +298,7 @@ describe('annotator/anchoring/pdf', function () {
return pdfAnchoring.anchor(container, selectors);
})
.then(function (anchoredRange) {
assert.equal(anchoredRange.toString(), 'Loading annotations');
assert.equal(anchoredRange.toString(), 'Loading annotations...');
});
});
......
import {
createPlaceholder,
hasPlaceholder,
isInPlaceholder,
removePlaceholder,
} from '../placeholder';
describe('annotator/anchoring/placeholder', () => {
describe('createPlaceholder', () => {
it('adds a placeholder element to the container', () => {
const container = document.createElement('div');
const placeholder = createPlaceholder(container);
assert.ok(placeholder);
assert.equal(
container.querySelector('.annotator-placeholder'),
placeholder
);
});
it('returns the existing placeholder if present', () => {
const container = document.createElement('div');
const placeholderA = createPlaceholder(container);
const placeholderB = createPlaceholder(container);
assert.equal(placeholderA, placeholderB);
});
});
describe('removePlaceholder', () => {
it('removes the placeholder if present', () => {
const container = document.createElement('div');
createPlaceholder(container);
assert.isTrue(hasPlaceholder(container));
removePlaceholder(container);
assert.isFalse(hasPlaceholder(container));
});
it('does nothing if placeholder is not present', () => {
const container = document.createElement('div');
removePlaceholder(container);
removePlaceholder(container);
});
});
describe('isInPlaceholder', () => {
it('returns true if node is inside a placeholder', () => {
const container = document.createElement('div');
const placeholder = createPlaceholder(container);
const child = document.createElement('div');
placeholder.append(child);
assert.isTrue(isInPlaceholder(child));
});
it('returns false if node is not inside a placeholder', () => {
assert.isFalse(isInPlaceholder(document.body));
});
});
});
import { isInPlaceholder } from './anchoring/placeholder';
import { isNodeInRange } from './range-util';
const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
......@@ -207,13 +208,9 @@ function wholeTextNodesInRange(range) {
export function highlightRange(range, cssClass = 'hypothesis-highlight') {
const textNodes = wholeTextNodesInRange(range);
// Check if this range refers to a placeholder for not-yet-rendered text in
// Check if this range refers to a placeholder for not-yet-rendered content in
// a PDF. These highlights should be invisible.
const isPlaceholder =
textNodes.length > 0 &&
/** @type {Element} */ (textNodes[0].parentNode).closest(
'.annotator-placeholder'
) !== null;
const isPlaceholder = textNodes.length > 0 && isInPlaceholder(textNodes[0]);
// Group text nodes into spans of adjacent nodes. If a group of text nodes are
// adjacent, we only need to create one highlight element for the group.
......
......@@ -8,6 +8,7 @@ import {
describe,
documentHasText,
} from '../anchoring/pdf';
import { removePlaceholder } from '../anchoring/placeholder';
import WarningBanner from '../components/WarningBanner';
import { createShadowRoot } from '../util/shadow-root';
import { ListenerCollection } from '../util/listener-collection';
......@@ -231,12 +232,7 @@ export class PDFIntegration {
// means the PDF anchoring module anchored annotations before it was
// rendered. Remove this, which will cause the annotations to anchor
// again, below.
{
const placeholder = page.div.querySelector(
'.annotator-placeholder'
);
placeholder?.parentNode.removeChild(placeholder);
}
removePlaceholder(page.div);
break;
}
}
......
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