Commit 5fbb3e20 authored by Robert Knight's avatar Robert Knight

Simplify "no selectable text" banner styling

Avoid the need to customize various inline styles of the
`<hypothesis-banner>` element by positioning it at the top of the DOM as
the first child of `document.body`. As a result it naturally pushes the
PDF.js UI down. The only change we need to make to PDF.js's UI is to adjust
the size of the `#outerContainer` element so that it does not overflow the
bottom of the viewport.

Aside from making styling simpler, making the banner's DOM position
better match its visual position may help with reading order for screen readers.
Related to this the banner is currently missing useful semantic
information for a11y purposes.

I also removed the banner resizing handling for the moment on the basis that it
isn't that important.
parent 15eb9ec1
...@@ -113,54 +113,38 @@ export default class PDF extends Delegator { ...@@ -113,54 +113,38 @@ export default class PDF extends Delegator {
* @param {boolean} showWarning * @param {boolean} showWarning
*/ */
_showNoSelectableTextWarning(showWarning) { _showNoSelectableTextWarning(showWarning) {
// Get a reference to the top-level DOM element associated with the PDF.js
// viewer.
const outerContainer = /** @type {HTMLElement} */ (document.querySelector(
'#outerContainer'
));
if (!showWarning) { if (!showWarning) {
this._warningBanner?.remove(); this._warningBanner?.remove();
this._warningBanner = null; this._warningBanner = null;
return;
}
// Get a reference to the root element of the main content area in // Undo inline styles applied when the banner is shown. The banner will
// PDF.js. // then gets its normal 100% height set by PDF.js's CSS.
const mainContainer = /** @type {HTMLElement} */ (document.querySelector( outerContainer.style.height = '';
'#mainContainer'
));
// Update the position of the warning banner and `mainContainer` element
// below it when the size of the banner changes, eg. due to a window resize.
const updateBannerHeight = () => {
/* istanbul ignore next */
if (!this._warningBanner) {
return; return;
} }
const rect = this._warningBanner.getBoundingClientRect();
mainContainer.style.top = rect.height + 'px';
this._warningBanner.style.top = -rect.height + 'px';
};
this._warningBanner = document.createElement('hypothesis-banner'); this._warningBanner = document.createElement('hypothesis-banner');
Object.assign(this._warningBanner.style, { document.body.prepend(this._warningBanner);
// Position the banner at the top of the viewer and make it span the full width.
position: 'absolute',
left: '0',
right: '0',
// The Z-index is necessary to raise the banner above the toolbar at the
// top of the PDF.js viewer.
zIndex: '10000',
});
mainContainer.appendChild(this._warningBanner);
const warningBannerContent = createShadowRoot(this._warningBanner); const warningBannerContent = createShadowRoot(this._warningBanner);
render(<WarningBanner />, warningBannerContent); render(<WarningBanner />, warningBannerContent);
// nb. In browsers that don't support `ResizeObserver` the banner height const bannerHeight = this._warningBanner.getBoundingClientRect().height;
// will simply be static and not adjust if the window is resized.
if (typeof ResizeObserver !== 'undefined') { // The `#outerContainer` element normally has height set to 100% of the body.
// Update the banner when the window is resized or the Hypothesis //
// sidebar is opened. // Reduce this by the height of the banner so that it doesn't extend beyond
new ResizeObserver(updateBannerHeight).observe(this._warningBanner); // the bottom of the viewport.
} //
updateBannerHeight(); // We don't currently handle the height of the banner changing here.
outerContainer.style.height = `calc(100% - ${bannerHeight}px)`;
} }
// This method (re-)anchors annotations when pages are rendered and destroyed. // This method (re-)anchors annotations when pages are rendered and destroyed.
......
...@@ -15,7 +15,12 @@ function awaitEvent(target, eventName) { ...@@ -15,7 +15,12 @@ function awaitEvent(target, eventName) {
} }
describe('annotator/plugin/pdf', () => { describe('annotator/plugin/pdf', () => {
let container; // Fake for the top-level `#outerContainer` DOM element created by PDF.js.
let outerContainer;
// Fake for the `#viewerContainer` DOM element created by PDF.js that contains
// the actual PDF content.
let viewerContainer;
let fakeAnnotator; let fakeAnnotator;
let fakePdfAnchoring; let fakePdfAnchoring;
let fakePDFMetadata; let fakePDFMetadata;
...@@ -28,10 +33,15 @@ describe('annotator/plugin/pdf', () => { ...@@ -28,10 +33,15 @@ describe('annotator/plugin/pdf', () => {
beforeEach(() => { beforeEach(() => {
// Setup fake PDF.js viewer. // Setup fake PDF.js viewer.
container = document.createElement('div'); outerContainer = document.createElement('div');
document.body.appendChild(container); outerContainer.id = 'outerContainer';
document.body.appendChild(outerContainer);
viewerContainer = document.createElement('div');
outerContainer.appendChild(viewerContainer);
fakePDFViewerApplication = new FakePDFViewerApplication({ fakePDFViewerApplication = new FakePDFViewerApplication({
container, container: viewerContainer,
content: ['First page', 'Second page'], content: ['First page', 'Second page'],
}); });
fakePDFViewerApplication.pdfViewer.setCurrentPage(0); fakePDFViewerApplication.pdfViewer.setCurrentPage(0);
...@@ -62,10 +72,9 @@ describe('annotator/plugin/pdf', () => { ...@@ -62,10 +72,9 @@ describe('annotator/plugin/pdf', () => {
}); });
afterEach(() => { afterEach(() => {
container.remove();
pdfPlugin?.destroy(); pdfPlugin?.destroy();
delete window.PDFViewerApplication; delete window.PDFViewerApplication;
outerContainer.remove();
$imports.$restore(); $imports.$restore();
}); });
...@@ -150,9 +159,6 @@ describe('annotator/plugin/pdf', () => { ...@@ -150,9 +159,6 @@ describe('annotator/plugin/pdf', () => {
}); });
it('shows a warning when PDF has no selectable text', async () => { it('shows a warning when PDF has no selectable text', async () => {
const mainContainer = document.createElement('div');
mainContainer.id = 'mainContainer';
document.body.appendChild(mainContainer);
fakePdfAnchoring.documentHasText.resolves(false); fakePdfAnchoring.documentHasText.resolves(false);
pdfPlugin = createPDFPlugin(); pdfPlugin = createPDFPlugin();
...@@ -161,13 +167,10 @@ describe('annotator/plugin/pdf', () => { ...@@ -161,13 +167,10 @@ describe('annotator/plugin/pdf', () => {
assert.called(fakePdfAnchoring.documentHasText); assert.called(fakePdfAnchoring.documentHasText);
const banner = getWarningBanner(); const banner = getWarningBanner();
assert.isNotNull(banner); assert.isNotNull(banner);
assert.isTrue(mainContainer.contains(banner));
assert.include( assert.include(
banner.shadowRoot.textContent, banner.shadowRoot.textContent,
'This PDF does not contain selectable text' 'This PDF does not contain selectable text'
); );
mainContainer.remove();
}); });
context('when the PDF viewer content changes', () => { context('when the PDF viewer content changes', () => {
......
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