From 11542d1f0850d6d6452b1103f8a1253176a736bd Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Wed, 21 Oct 2020 12:38:28 +0100 Subject: [PATCH] Merge Host and Sidebar classes The `Host` class is never constructed directly and only has one subclass, `Sidebar`. There wasn't a clear separation of responsibilities between `Host` and `Sidebar`. This commit therefore moves the functionality of `Host` into `Sidebar` and merges the tests. --- src/annotator/host.js | 110 ------------------------- src/annotator/sidebar.js | 121 ++++++++++++++++++++++++++- src/annotator/test/host-test.js | 127 ----------------------------- src/annotator/test/sidebar-test.js | 89 ++++++++++++++++++-- 4 files changed, 203 insertions(+), 244 deletions(-) delete mode 100644 src/annotator/host.js delete mode 100644 src/annotator/test/host-test.js diff --git a/src/annotator/host.js b/src/annotator/host.js deleted file mode 100644 index 10d507ec118..00000000000 --- a/src/annotator/host.js +++ /dev/null @@ -1,110 +0,0 @@ -import Guest from './guest'; - -export default class Host extends Guest { - constructor(element, config) { - // Some config settings are not JSON-stringifiable (e.g. JavaScript - // functions) and will be omitted when the config is JSON-stringified. - // Add a JSON-stringifiable option for each of these so that the sidebar can - // at least know whether the callback functions were provided or not. - if (config.services?.length > 0) { - const service = config.services[0]; - if (service.onLoginRequest) { - service.onLoginRequestProvided = true; - } - if (service.onLogoutRequest) { - service.onLogoutRequestProvided = true; - } - if (service.onSignupRequest) { - service.onSignupRequestProvided = true; - } - if (service.onProfileRequest) { - service.onProfileRequestProvided = true; - } - if (service.onHelpRequest) { - service.onHelpRequestProvided = true; - } - } - - // Make a copy of the config for use by the sidebar app with several - // annotator-only properties removed. nb. We don't currently strip all the - // annotator-only properties here. That's OK because validation / filtering - // happens in the sidebar app itself. It just results in unnecessary content - // in the sidebar iframe's URL string. - const sidebarConfig = { ...config }; - ['sidebarAppUrl', 'pluginClasses'].forEach( - key => delete sidebarConfig[key] - ); - - const configParam = - 'config=' + encodeURIComponent(JSON.stringify(sidebarConfig)); - - const sidebarAppSrc = config.sidebarAppUrl + '#' + configParam; - - // Create the sidebar iframe - const sidebarFrame = document.createElement('iframe'); - sidebarFrame.setAttribute('name', 'hyp_sidebar_frame'); - // Enable media in annotations to be shown fullscreen - sidebarFrame.setAttribute('allowfullscreen', ''); - sidebarFrame.setAttribute('seamless', ''); - sidebarFrame.src = sidebarAppSrc; - sidebarFrame.title = 'Hypothesis annotation viewer'; - sidebarFrame.className = 'h-sidebar-iframe'; - - let externalContainer = null; - - if (config.externalContainerSelector) { - // Use the native method to also validate the input - externalContainer = document.querySelector( - config.externalContainerSelector - ); - } - - let externalFrame; - let frame; - - if (externalContainer) { - externalFrame = externalContainer; - } else { - frame = document.createElement('div'); - frame.style.display = 'none'; - frame.className = 'annotator-frame annotator-outer'; - - if (config.theme === 'clean') { - frame.classList.add('annotator-frame--drop-shadow-enabled'); - } - - element.appendChild(frame); - } - - // FIXME: We have to call the parent constructor here instead of at the top - // of the function because it triggers plugin construction and the BucketBar - // plugin constructor in turn assumes that the `.annotator-frame` element is - // already in the DOM. - super(element, config); - - this.externalFrame = externalFrame; - this.frame = frame; - (frame || externalFrame).appendChild(sidebarFrame); - - this.subscribe('panelReady', () => { - // Show the UI - if (this.frame) { - this.frame.style.display = ''; - } - }); - - this.subscribe('beforeAnnotationCreated', annotation => { - // When a new non-highlight annotation is created, focus - // the sidebar so that the text editor can be focused as - // soon as the annotation card appears - if (!annotation.$highlight) { - /** @type {Window} */ (sidebarFrame.contentWindow).focus(); - } - }); - } - - destroy() { - this.frame?.remove(); - super.destroy(); - } -} diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js index 56d4968c53e..c84cb498bef 100644 --- a/src/annotator/sidebar.js +++ b/src/annotator/sidebar.js @@ -5,7 +5,7 @@ import sidebarTrigger from './sidebar-trigger'; import events from '../shared/bridge-events'; import features from './features'; -import Host from './host'; +import Guest from './guest'; import { ToolbarController } from './toolbar'; /** @@ -24,14 +24,130 @@ const defaultConfig = { }, }; -export default class Sidebar extends Host { +/** + * Create the JSON-serializable subset of annotator configuration that should + * be passed to the sidebar application. + */ +function createSidebarConfig(config) { + const sidebarConfig = { ...config }; + + // Some config settings are not JSON-stringifiable (e.g. JavaScript + // functions) and will be omitted when the config is JSON-stringified. + // Add a JSON-stringifiable option for each of these so that the sidebar can + // at least know whether the callback functions were provided or not. + if (sidebarConfig.services?.length > 0) { + const service = sidebarConfig.services[0]; + if (service.onLoginRequest) { + service.onLoginRequestProvided = true; + } + if (service.onLogoutRequest) { + service.onLogoutRequestProvided = true; + } + if (service.onSignupRequest) { + service.onSignupRequestProvided = true; + } + if (service.onProfileRequest) { + service.onProfileRequestProvided = true; + } + if (service.onHelpRequest) { + service.onHelpRequestProvided = true; + } + } + + // Remove several annotator-only properties. + // + // nb. We don't currently strip all the annotator-only properties here. + // That's OK because validation / filtering happens in the sidebar app itself. + // It just results in unnecessary content in the sidebar iframe's URL string. + ['sidebarAppUrl', 'pluginClasses'].forEach(key => delete sidebarConfig[key]); + + return sidebarConfig; +} + +/** + * Create the iframe that will load the sidebar application. + * + * @return {HTMLIFrameElement} + */ +function createSidebarIframe(config) { + const sidebarConfig = createSidebarConfig(config); + const configParam = + 'config=' + encodeURIComponent(JSON.stringify(sidebarConfig)); + const sidebarAppSrc = config.sidebarAppUrl + '#' + configParam; + + const sidebarFrame = document.createElement('iframe'); + sidebarFrame.setAttribute('name', 'hyp_sidebar_frame'); + + // Enable media in annotations to be shown fullscreen + sidebarFrame.setAttribute('allowfullscreen', ''); + + sidebarFrame.setAttribute('seamless', ''); + sidebarFrame.src = sidebarAppSrc; + sidebarFrame.title = 'Hypothesis annotation viewer'; + sidebarFrame.className = 'h-sidebar-iframe'; + + return sidebarFrame; +} + +/** + * The `Sidebar` class creates the sidebar application iframe and its container, + * as well as the adjacent controls. + */ +export default class Sidebar extends Guest { constructor(element, config) { if (config.theme === 'clean' || config.externalContainerSelector) { delete config.pluginClasses.BucketBar; } + let externalContainer = null; + + if (config.externalContainerSelector) { + externalContainer = document.querySelector( + config.externalContainerSelector + ); + } + + let externalFrame; + let frame; + + if (externalContainer) { + externalFrame = externalContainer; + } else { + frame = document.createElement('div'); + frame.style.display = 'none'; + frame.className = 'annotator-frame annotator-outer'; + + if (config.theme === 'clean') { + frame.classList.add('annotator-frame--drop-shadow-enabled'); + } + + element.appendChild(frame); + } + + const sidebarFrame = createSidebarIframe(config); + super(element, { ...defaultConfig, ...config }); + this.externalFrame = externalFrame; + this.frame = frame; + (frame || externalFrame).appendChild(sidebarFrame); + + this.subscribe('panelReady', () => { + // Show the UI + if (this.frame) { + this.frame.style.display = ''; + } + }); + + this.subscribe('beforeAnnotationCreated', annotation => { + // When a new non-highlight annotation is created, focus + // the sidebar so that the text editor can be focused as + // soon as the annotation card appears + if (!annotation.$highlight) { + /** @type {Window} */ (sidebarFrame.contentWindow).focus(); + } + }); + if ( config.openSidebar || config.annotations || @@ -95,6 +211,7 @@ export default class Sidebar extends Host { destroy() { this._hammerManager?.destroy(); + this.frame?.remove(); super.destroy(); } diff --git a/src/annotator/test/host-test.js b/src/annotator/test/host-test.js deleted file mode 100644 index b14206906a1..00000000000 --- a/src/annotator/test/host-test.js +++ /dev/null @@ -1,127 +0,0 @@ -import Host from '../host'; - -describe('Host', () => { - const sandbox = sinon.createSandbox(); - const hostConfig = { pluginClasses: {} }; - - let CrossFrame; - let fakeCrossFrame; - - const createHost = (config = {}, element = null) => { - config = Object.assign( - { sidebarAppUrl: '/base/annotator/test/empty.html' }, - hostConfig, - config - ); - if (!element) { - element = document.createElement('div'); - } - return new Host(element, config); - }; - - beforeEach(() => { - // Disable any Host logging. - sandbox.stub(console, 'log'); - - fakeCrossFrame = {}; - fakeCrossFrame.onConnect = sandbox.stub().returns(fakeCrossFrame); - fakeCrossFrame.on = sandbox.stub().returns(fakeCrossFrame); - fakeCrossFrame.call = sandbox.spy(); - - CrossFrame = sandbox.stub(); - CrossFrame.returns(fakeCrossFrame); - hostConfig.pluginClasses.CrossFrame = CrossFrame; - }); - - afterEach(() => { - sandbox.restore(); - }); - - describe('widget visibility', () => { - it('starts hidden', () => { - const host = createHost(); - assert.equal(host.frame.style.display, 'none'); - }); - - it('becomes visible when the "panelReady" event fires', () => { - const host = createHost(); - host.publish('panelReady'); - assert.equal(host.frame.style.display, ''); - }); - }); - - describe('focus', () => { - let element; - let frame; - let host; - - beforeEach(() => { - element = document.createElement('div'); - document.body.appendChild(element); - host = createHost({}, element); - frame = element.querySelector('[name=hyp_sidebar_frame]'); - sinon.spy(frame.contentWindow, 'focus'); - }); - - afterEach(() => { - frame.contentWindow.focus.restore(); - element.parentNode.removeChild(element); - }); - - it('focuses the sidebar when a new annotation is created', () => { - host.publish('beforeAnnotationCreated', [ - { - $highlight: false, - }, - ]); - assert.called(frame.contentWindow.focus); - }); - - it('does not focus the sidebar when a new highlight is created', () => { - host.publish('beforeAnnotationCreated', [ - { - $highlight: true, - }, - ]); - assert.notCalled(frame.contentWindow.focus); - }); - }); - - describe('config', () => { - it('disables highlighting if showHighlights: false is given', done => { - const host = createHost({ showHighlights: false }); - host.subscribe('panelReady', () => { - assert.isFalse(host.visibleHighlights); - done(); - }); - host.publish('panelReady'); - }); - - function getConfigString(host) { - return host.frame.children[0].src; - } - - function configFragment(config) { - return '#config=' + encodeURIComponent(JSON.stringify(config)); - } - - it('passes config to the sidebar iframe', () => { - const appURL = new URL( - '/base/annotator/test/empty.html', - window.location.href - ); - const host = createHost({ annotations: '1234' }); - assert.equal( - getConfigString(host), - appURL + configFragment({ annotations: '1234' }) - ); - }); - - it('adds drop shadow if the clean theme is enabled', () => { - const host = createHost({ theme: 'clean' }); - assert.isTrue( - host.frame.classList.contains('annotator-frame--drop-shadow-enabled') - ); - }); - }); -}); diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js index eac4b1bf3e4..f5c00bdddcd 100644 --- a/src/annotator/test/sidebar-test.js +++ b/src/annotator/test/sidebar-test.js @@ -27,11 +27,15 @@ describe('Sidebar', () => { window.requestAnimationFrame.restore(); }); - const createSidebar = config => { - if (!config) { - config = {}; - } - config = Object.assign({}, sidebarConfig, config); + const createSidebar = (config = {}) => { + config = Object.assign( + { + // Dummy sidebar app. + sidebarAppUrl: '/base/annotator/test/empty.html', + }, + sidebarConfig, + config + ); const element = document.createElement('div'); const sidebar = new Sidebar(element, config); @@ -93,6 +97,81 @@ describe('Sidebar', () => { $imports.$restore(); }); + describe('sidebar container frame', () => { + it('starts hidden', () => { + const sidebar = createSidebar(); + assert.equal(sidebar.frame.style.display, 'none'); + }); + + it('has a shadow if the clean theme is enabled', () => { + const sidebar = createSidebar({ theme: 'clean' }); + assert.isTrue( + sidebar.frame.classList.contains('annotator-frame--drop-shadow-enabled') + ); + }); + + it('becomes visible when the "panelReady" event fires', () => { + const sidebar = createSidebar(); + sidebar.publish('panelReady'); + assert.equal(sidebar.frame.style.display, ''); + }); + }); + + function getConfigString(sidebar) { + return sidebar.frame.querySelector('iframe').src; + } + + function configFragment(config) { + return '#config=' + encodeURIComponent(JSON.stringify(config)); + } + + it('creates sidebar iframe and passes configuration to it', () => { + const appURL = new URL( + '/base/annotator/test/empty.html', + window.location.href + ); + const sidebar = createSidebar({ annotations: '1234' }); + assert.equal( + getConfigString(sidebar), + appURL + configFragment({ annotations: '1234' }) + ); + }); + + context('when a new annotation is created', () => { + function stubIframeWindow(sidebar) { + const iframe = sidebar.frame.querySelector('iframe'); + const fakeIframeWindow = { focus: sinon.stub() }; + sinon.stub(iframe, 'contentWindow').get(() => fakeIframeWindow); + return iframe; + } + + it('focuses the sidebar if the annotation is not a highlight', () => { + const sidebar = createSidebar(); + const iframe = stubIframeWindow(sidebar); + + sidebar.publish('beforeAnnotationCreated', [ + { + $highlight: false, + }, + ]); + + assert.called(iframe.contentWindow.focus); + }); + + it('does not focus the sidebar if the annotation is a highlight', () => { + const sidebar = createSidebar(); + const iframe = stubIframeWindow(sidebar); + + sidebar.publish('beforeAnnotationCreated', [ + { + $highlight: true, + }, + ]); + + assert.notCalled(iframe.contentWindow.focus); + }); + }); + describe('toolbar buttons', () => { it('shows or hides sidebar when toolbar button is clicked', () => { const sidebar = createSidebar({});