Commit 02c32fab authored by Eduardo Sanz García's avatar Eduardo Sanz García Committed by Eduardo

Use shadow DOM to encapsulate sidebar's style

A side effect of using shadow DOM for the sidebar is that the BucketBar
'plugin' could not be injected easily using a current query mechanism.
After consulting with @robertknight, we decided to avoid using the
normal plugin injection mechanism and instead instantiate the BucketBar
from the sidebar.

This PR also includes:

- a no documented configuration option to disable the shadow DOM
  encapsulation. This can be removed in the future if not needed.
- more strict types
- simplification of the logic in the sidebar

I have tested these changes in the following browsers:

Brower\OS    | MacOS              | Windows
------------ | ------------------ | -----------------
Chrome 57    |  | 
Chrome beta  |  | 
Edge 17      |                    | 
Edge beta    |  | 
Firefox 53   |  | 
Firefox beta |  | 
Safari 10    |  |
Safari 14    |  |
parent 453e0a52
...@@ -28,7 +28,7 @@ export default class Delegator { ...@@ -28,7 +28,7 @@ export default class Delegator {
* Construct the `Delegator` instance. * Construct the `Delegator` instance.
* *
* @param {HTMLElement} element * @param {HTMLElement} element
* @param {Object} [config] * @param {Record<string, any>} [config]
*/ */
constructor(element, config) { constructor(element, config) {
this.options = { ...config }; this.options = { ...config };
......
...@@ -107,7 +107,7 @@ export default class Guest extends Delegator { ...@@ -107,7 +107,7 @@ export default class Guest extends Delegator {
* @param {HTMLElement} element - * @param {HTMLElement} element -
* The root element in which the `Guest` instance should be able to anchor * The root element in which the `Guest` instance should be able to anchor
* or create annotations. In an ordinary web page this typically `document.body`. * or create annotations. In an ordinary web page this typically `document.body`.
* @param {Object} config * @param {Record<string, any>} config
* @param {typeof htmlAnchoring} anchoring - Anchoring implementation * @param {typeof htmlAnchoring} anchoring - Anchoring implementation
*/ */
constructor(element, config, anchoring = htmlAnchoring) { constructor(element, config, anchoring = htmlAnchoring) {
......
...@@ -18,7 +18,6 @@ import iconSet from './icons'; ...@@ -18,7 +18,6 @@ import iconSet from './icons';
registerIcons(iconSet); registerIcons(iconSet);
import configFrom from './config/index'; import configFrom from './config/index';
import BucketBarPlugin from './plugin/bucket-bar';
import CrossFramePlugin from './plugin/cross-frame'; import CrossFramePlugin from './plugin/cross-frame';
import DocumentPlugin from './plugin/document'; import DocumentPlugin from './plugin/document';
import Guest from './guest'; import Guest from './guest';
...@@ -28,9 +27,6 @@ import PdfSidebar from './pdf-sidebar'; ...@@ -28,9 +27,6 @@ import PdfSidebar from './pdf-sidebar';
import Sidebar from './sidebar'; import Sidebar from './sidebar';
const pluginClasses = { const pluginClasses = {
// UI plugins
BucketBar: BucketBarPlugin,
// Document type plugins // Document type plugins
PDF: PDFPlugin, PDF: PDFPlugin,
Document: DocumentPlugin, Document: DocumentPlugin,
......
...@@ -8,7 +8,6 @@ import Sidebar from './sidebar'; ...@@ -8,7 +8,6 @@ import Sidebar from './sidebar';
const defaultConfig = { const defaultConfig = {
PDF: {}, PDF: {},
BucketBar: { BucketBar: {
container: '.annotator-frame',
scrollables: ['#viewerContainer'], scrollables: ['#viewerContainer'],
}, },
}; };
...@@ -19,6 +18,10 @@ const defaultConfig = { ...@@ -19,6 +18,10 @@ const defaultConfig = {
const MIN_PDF_WIDTH = 680; const MIN_PDF_WIDTH = 680;
export default class PdfSidebar extends Sidebar { export default class PdfSidebar extends Sidebar {
/**
* @param {HTMLElement} element
* @param {Record<string, any>} config
*/
constructor(element, config) { constructor(element, config) {
super(element, { ...defaultConfig, ...config }); super(element, { ...defaultConfig, ...config });
......
...@@ -8,6 +8,8 @@ import features from './features'; ...@@ -8,6 +8,8 @@ import features from './features';
import Guest from './guest'; import Guest from './guest';
import { ToolbarController } from './toolbar'; import { ToolbarController } from './toolbar';
import { createShadowRoot } from './util/shadow-root';
import BucketBar from './plugin/bucket-bar';
/** /**
* @typedef LayoutState * @typedef LayoutState
...@@ -19,12 +21,6 @@ import { ToolbarController } from './toolbar'; ...@@ -19,12 +21,6 @@ import { ToolbarController } from './toolbar';
// Minimum width to which the frame can be resized. // Minimum width to which the frame can be resized.
const MIN_RESIZE = 280; const MIN_RESIZE = 280;
const defaultConfig = {
BucketBar: {
container: '.annotator-frame',
},
};
/** /**
* Create the iframe that will load the sidebar application. * Create the iframe that will load the sidebar application.
* *
...@@ -53,24 +49,20 @@ function createSidebarIframe(config) { ...@@ -53,24 +49,20 @@ function createSidebarIframe(config) {
* as well as the adjacent controls. * as well as the adjacent controls.
*/ */
export default class Sidebar extends Guest { export default class Sidebar extends Guest {
/**
* @param {HTMLElement} element
* @param {Record<string, any>} config
*/
constructor(element, config) { constructor(element, config) {
if (config.theme === 'clean' || config.externalContainerSelector) { super(element, config);
delete config.pluginClasses.BucketBar;
}
let externalContainer = null;
if (config.externalContainerSelector) {
externalContainer = document.querySelector(
config.externalContainerSelector
);
}
let externalFrame; let externalFrame;
let frame; let frame;
let hypothesisSidebar; // refers to <hypothesis-sidebar> element
if (externalContainer) { if (config.externalContainerSelector) {
externalFrame = externalContainer; externalFrame =
document.querySelector(config.externalContainerSelector) || element;
} else { } else {
frame = document.createElement('div'); frame = document.createElement('div');
frame.style.display = 'none'; frame.style.display = 'none';
...@@ -78,18 +70,32 @@ export default class Sidebar extends Guest { ...@@ -78,18 +70,32 @@ export default class Sidebar extends Guest {
if (config.theme === 'clean') { if (config.theme === 'clean') {
frame.classList.add('annotator-frame--theme-clean'); frame.classList.add('annotator-frame--theme-clean');
} else {
// BucketBar is a "plugin" for legacy reasons and is now constructed here so
// that the parent element can be passed into the constructor.
this.plugins.BucketBar = new BucketBar(frame, {}, this);
} }
// Undocumented switch to enable/disable the wrapping of the sidebar inside a shadow DOM
// 2021-01-22: remove this switch after the 2021-02-05
if (config.disableShadowSidebar) {
element.appendChild(frame); element.appendChild(frame);
} else {
// Wrap up the 'frame' element into a shadow DOM so it is not affected by host CSS styles
hypothesisSidebar = document.createElement('hypothesis-sidebar');
const shadowDom = createShadowRoot(hypothesisSidebar);
shadowDom.appendChild(frame);
element.appendChild(hypothesisSidebar);
}
} }
const sidebarFrame = createSidebarIframe(config); const sidebarFrame = createSidebarIframe(config);
(frame || externalFrame).appendChild(sidebarFrame);
super(element, { ...defaultConfig, ...config });
this.externalFrame = externalFrame; this.externalFrame = externalFrame;
this.frame = frame; this.frame = frame;
(frame || externalFrame).appendChild(sidebarFrame); this.hypothesisSidebar = hypothesisSidebar;
this.subscribe('panelReady', () => { this.subscribe('panelReady', () => {
// Show the UI // Show the UI
...@@ -171,6 +177,7 @@ export default class Sidebar extends Guest { ...@@ -171,6 +177,7 @@ export default class Sidebar extends Guest {
destroy() { destroy() {
this._hammerManager?.destroy(); this._hammerManager?.destroy();
this.frame?.remove(); this.frame?.remove();
this.hypothesisSidebar?.remove();
super.destroy(); super.destroy();
} }
......
...@@ -15,6 +15,7 @@ describe('Sidebar', () => { ...@@ -15,6 +15,7 @@ describe('Sidebar', () => {
// `Sidebar` instances created by current test. // `Sidebar` instances created by current test.
let sidebars; let sidebars;
let sidebarContainer;
let FakeToolbarController; let FakeToolbarController;
let fakeToolbar; let fakeToolbar;
...@@ -25,6 +26,7 @@ describe('Sidebar', () => { ...@@ -25,6 +26,7 @@ describe('Sidebar', () => {
after(() => { after(() => {
window.requestAnimationFrame.restore(); window.requestAnimationFrame.restore();
sidebarContainer?.remove();
}); });
const createSidebar = (config = {}) => { const createSidebar = (config = {}) => {
...@@ -36,9 +38,10 @@ describe('Sidebar', () => { ...@@ -36,9 +38,10 @@ describe('Sidebar', () => {
sidebarConfig, sidebarConfig,
config config
); );
const element = document.createElement('div'); sidebarContainer = document.createElement('div');
const sidebar = new Sidebar(element, config); const sidebar = new Sidebar(sidebarContainer, config);
document.body.appendChild(sidebarContainer);
sidebars.push(sidebar); sidebars.push(sidebar);
return sidebar; return sidebar;
...@@ -98,6 +101,13 @@ describe('Sidebar', () => { ...@@ -98,6 +101,13 @@ describe('Sidebar', () => {
}); });
describe('sidebar container frame', () => { describe('sidebar container frame', () => {
it('creates shadow DOM', () => {
createSidebar();
const sidebar = sidebarContainer.querySelector('hypothesis-sidebar');
assert.exists(sidebar);
assert.exists(sidebar.shadowRoot);
});
it('starts hidden', () => { it('starts hidden', () => {
const sidebar = createSidebar(); const sidebar = createSidebar();
assert.equal(sidebar.frame.style.display, 'none'); assert.equal(sidebar.frame.style.display, 'none');
...@@ -477,15 +487,19 @@ describe('Sidebar', () => { ...@@ -477,15 +487,19 @@ describe('Sidebar', () => {
}); });
describe('destruction', () => { describe('destruction', () => {
let sidebar; it('the (shadow DOMed) sidebar is destroyed and the frame is detached', () => {
const sidebar = createSidebar();
beforeEach(() => { sidebar.destroy();
sidebar = createSidebar({}); assert.called(fakeCrossFrame.destroy);
assert.notExists(sidebarContainer.querySelector('hypothesis-sidebar'));
assert.equal(sidebar.frame.parentElement, null);
}); });
it('the sidebar is destroyed and the frame is detached', () => { it('the (non-shadow DOMed) sidebar is destroyed and the frame is detached', () => {
const sidebar = createSidebar({ disableShadowSidebar: true });
sidebar.destroy(); sidebar.destroy();
assert.called(fakeCrossFrame.destroy); assert.called(fakeCrossFrame.destroy);
assert.notExists(sidebarContainer.querySelector('.annotator-frame'));
assert.equal(sidebar.frame.parentElement, null); assert.equal(sidebar.frame.parentElement, null);
}); });
}); });
...@@ -661,7 +675,7 @@ describe('Sidebar', () => { ...@@ -661,7 +675,7 @@ describe('Sidebar', () => {
const layoutChangeExternalConfig = { const layoutChangeExternalConfig = {
onLayoutChange: layoutChangeHandlerSpy, onLayoutChange: layoutChangeHandlerSpy,
sidebarAppUrl: '/', sidebarAppUrl: '/',
externalContainerSelector: '.' + EXTERNAL_CONTAINER_SELECTOR, externalContainerSelector: `.${EXTERNAL_CONTAINER_SELECTOR}`,
}; };
sidebar = createSidebar(layoutChangeExternalConfig); sidebar = createSidebar(layoutChangeExternalConfig);
...@@ -700,7 +714,7 @@ describe('Sidebar', () => { ...@@ -700,7 +714,7 @@ describe('Sidebar', () => {
document.body.appendChild(externalFrame); document.body.appendChild(externalFrame);
sidebar = createSidebar({ sidebar = createSidebar({
externalContainerSelector: '.' + EXTERNAL_CONTAINER_SELECTOR, externalContainerSelector: `.${EXTERNAL_CONTAINER_SELECTOR}`,
}); });
}); });
...@@ -724,9 +738,19 @@ describe('Sidebar', () => { ...@@ -724,9 +738,19 @@ describe('Sidebar', () => {
it('does not have the BucketBar if an external container is provided', () => { it('does not have the BucketBar if an external container is provided', () => {
const sidebar = createSidebar({ const sidebar = createSidebar({
externalContainerSelector: '.' + EXTERNAL_CONTAINER_SELECTOR, externalContainerSelector: `.${EXTERNAL_CONTAINER_SELECTOR}`,
}); });
assert.isUndefined(sidebar.plugins.BucketBar); assert.isUndefined(sidebar.plugins.BucketBar);
}); });
it('disables shadow DOM if `disableShadowSidebar` flag is set', () => {
createSidebar({
disableShadowSidebar: true,
});
assert.notExists(sidebarContainer.querySelector('hypothesis-sidebar'));
const sidebar = sidebarContainer.querySelector('.annotator-frame');
assert.exists(sidebar);
assert.notExists(sidebar.shadowRoot);
});
}); });
}); });
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