From 126384d377804566bb2bce5b6962695f57a07aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= Date: Mon, 18 Jan 2021 17:56:33 +0100 Subject: [PATCH] Hide sidebar when browser window is resized Currently, if the client's sidebar is opened it doesn't behave graciously when the browser window is resized. Sometimes the sidebar appears floating on the middle of the window. We discussed several alternative solutions: when the sidebar is opened and the window is resized, then... 1. close the sidebar and the user opens it manually 2. maintain the sidebar open, but scale it properly 3. close the sidebar and open it after X milliseconds of the last resize event 4. close the sidebar if the width of the window is reduced, but maintain it open if the window's width is increased We implemented solution #1. However, this issue is still visible on the PDF-sidebar. It would be great if the both sidebars (PDF and non-PDF) have the same behaviour. In addition, I added a mechanism to register and unregister events, to avoid resource leaks. --- src/annotator/pdf-sidebar.js | 5 ++- src/annotator/sidebar.js | 60 +++++++++++++++++++++++++++--- src/annotator/test/sidebar-test.js | 48 +++++++++++++++++++----- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/annotator/pdf-sidebar.js b/src/annotator/pdf-sidebar.js index 717365ef2f0..256a94e25f0 100644 --- a/src/annotator/pdf-sidebar.js +++ b/src/annotator/pdf-sidebar.js @@ -20,7 +20,7 @@ const MIN_PDF_WIDTH = 680; export default class PdfSidebar extends Sidebar { constructor(element, config) { - super(element, { ...defaultConfig, ...config }); + super(element, { ...defaultConfig, ...config }, false); this._lastSidebarLayoutState = { expanded: false, @@ -36,7 +36,8 @@ export default class PdfSidebar extends Sidebar { this.sideBySideActive = false; this.subscribe('sidebarLayoutChanged', state => this.fitSideBySide(state)); - this.window.addEventListener('resize', () => this.fitSideBySide()); + + this._registerEvent(window, 'resize', () => this.fitSideBySide()); } /** diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js index 473b4062e74..b2474de2f71 100644 --- a/src/annotator/sidebar.js +++ b/src/annotator/sidebar.js @@ -16,6 +16,16 @@ import { ToolbarController } from './toolbar'; * @prop {number} height */ +/** + * @typedef {Window|HTMLElement} WindowOrHTMLElement + * @typedef {Parameters} ArgAddEventListener + * @typedef {{ + * object: WindowOrHTMLElement, + * eventType: ArgAddEventListener[0], + * listener:ArgAddEventListener[1] + * }} RegisteredListener + */ + // Minimum width to which the frame can be resized. const MIN_RESIZE = 280; @@ -53,7 +63,11 @@ function createSidebarIframe(config) { * as well as the adjacent controls. */ export default class Sidebar extends Guest { - constructor(element, config) { + /** + * @param {HTMLElement} element + * @param {Record} config + */ + constructor(element, config, hideOnResize = true) { if (config.theme === 'clean' || config.externalContainerSelector) { delete config.pluginClasses.BucketBar; } @@ -87,6 +101,9 @@ export default class Sidebar extends Guest { super(element, { ...defaultConfig, ...config }); + /** @type {RegisteredListener[]} */ + this.registeredListeners = []; + this.externalFrame = externalFrame; this.frame = frame; (frame || externalFrame).appendChild(sidebarFrame); @@ -117,7 +134,7 @@ export default class Sidebar extends Guest { } if (this.plugins.BucketBar) { - this.plugins.BucketBar.element.addEventListener('click', () => + this._registerEvent(this.plugins.BucketBar.element, 'click', () => this.show() ); } @@ -141,6 +158,10 @@ export default class Sidebar extends Guest { this.toolbarWidth = 0; } + if (hideOnResize) { + this._registerEvent(window, 'resize', () => this._onResize()); + } + this._gestureState = { // Initial position at the start of a drag/pan resize event (in pixels). initial: /** @type {number|null} */ (null), @@ -169,11 +190,29 @@ export default class Sidebar extends Guest { } destroy() { + this._unregisterEvents(); this._hammerManager?.destroy(); this.frame?.remove(); super.destroy(); } + /** + * @param {WindowOrHTMLElement} object + * @param {ArgAddEventListener[0]} eventType + * @param {ArgAddEventListener[1]} listener + */ + _registerEvent(object, eventType, listener) { + object.addEventListener(eventType, listener); + this.registeredListeners.push({ object, eventType, listener }); + } + + _unregisterEvents() { + this.registeredListeners.forEach(({ object, eventType, listener }) => { + object.removeEventListener(eventType, listener); + }); + this.registeredListeners = []; + } + _setupSidebarEvents() { annotationCounts(document.body, this.crossframe); sidebarTrigger(document.body, () => this.show()); @@ -215,11 +254,11 @@ export default class Sidebar extends Guest { const toggleButton = this.toolbar.sidebarToggleButton; if (toggleButton) { // Prevent any default gestures on the handle. - toggleButton.addEventListener('touchmove', e => e.preventDefault()); + this._registerEvent(toggleButton, 'touchmove', e => e.preventDefault()); - this._hammerManager = new Hammer.Manager(toggleButton) - // eslint-disable-next-line no-restricted-properties - .on('panstart panend panleft panright', this._onPan.bind(this)); + this._hammerManager = new Hammer.Manager( + toggleButton + ).on('panstart panend panleft panright', () => this._onPan()); this._hammerManager.add( new Hammer.Pan({ direction: Hammer.DIRECTION_HORIZONTAL }) ); @@ -308,6 +347,15 @@ export default class Sidebar extends Guest { this.publish('sidebarLayoutChanged', [layoutState]); } + /** + * Minimize the sidebar on window resize event + */ + _onResize() { + if (this.toolbar.sidebarOpen === true) { + this.hide(); + } + } + _onPan(event) { const frame = this.frame; if (!frame) { diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js index de2d8cbaf94..636018bb239 100644 --- a/src/annotator/test/sidebar-test.js +++ b/src/annotator/test/sidebar-test.js @@ -174,7 +174,7 @@ describe('Sidebar', () => { describe('toolbar buttons', () => { it('shows or hides sidebar when toolbar button is clicked', () => { - const sidebar = createSidebar({}); + const sidebar = createSidebar(); sinon.stub(sidebar, 'show'); sinon.stub(sidebar, 'hide'); @@ -186,7 +186,7 @@ describe('Sidebar', () => { }); it('shows or hides highlights when toolbar button is clicked', () => { - const sidebar = createSidebar({}); + const sidebar = createSidebar(); sinon.stub(sidebar, 'setAllVisibleHighlights'); FakeToolbarController.args[0][1].setHighlightsVisible(true); @@ -198,7 +198,7 @@ describe('Sidebar', () => { }); it('creates an annotation when toolbar button is clicked', () => { - const sidebar = createSidebar({}); + const sidebar = createSidebar(); sinon.stub(sidebar, 'createAnnotation'); FakeToolbarController.args[0][1].createAnnotation(); @@ -312,7 +312,7 @@ describe('Sidebar', () => { }); it('does not crash if there is no services', () => { - createSidebar({}); // No config.services + createSidebar(); // No config.services emitEvent(events.LOGIN_REQUESTED); }); @@ -372,7 +372,7 @@ describe('Sidebar', () => { let sidebar; beforeEach(() => { - sidebar = createSidebar({}); + sidebar = createSidebar(); }); describe('panstart event', () => { @@ -469,7 +469,7 @@ describe('Sidebar', () => { }); it('does not show the sidebar if not configured to.', () => { - const sidebar = createSidebar({}); + const sidebar = createSidebar(); const show = sandbox.stub(sidebar, 'show'); sidebar.publish('panelReady'); assert.notCalled(show); @@ -480,7 +480,7 @@ describe('Sidebar', () => { let sidebar; beforeEach(() => { - sidebar = createSidebar({}); + sidebar = createSidebar(); }); it('the sidebar is destroyed and the frame is detached', () => { @@ -534,7 +534,7 @@ describe('Sidebar', () => { describe('#setAllVisibleHighlights', () => it('sets the state through crossframe and emits', () => { - const sidebar = createSidebar({}); + const sidebar = createSidebar(); sidebar.setAllVisibleHighlights(true); assert.calledWith(fakeCrossFrame.call, 'setVisibleHighlights', true); })); @@ -545,10 +545,40 @@ describe('Sidebar', () => { }); it('shows toolbar controls when using the default theme', () => { - createSidebar({}); + createSidebar(); assert.equal(fakeToolbar.useMinimalControls, false); }); + it('hides the sidebar if window is resized', () => { + const sidebar = createSidebar(); + sidebar.show(); + assert.isNotEmpty(sidebar.frame.style.marginLeft); + + window.dispatchEvent(new Event('resize')); + assert.isEmpty(sidebar.frame.style.marginLeft); + }); + + describe('register/unregister events', () => { + it('triggers registered event listener', () => { + const sidebar = createSidebar(); + const listener = sinon.stub(); + sidebar._registerEvent(window, 'resize', listener); + + window.dispatchEvent(new Event('resize')); + assert.calledOnce(listener); + }); + + it('unregisters event listeners', () => { + const sidebar = createSidebar(); + const listener = sinon.stub(); + sidebar._registerEvent(window, 'resize', listener); + sidebar.destroy(); + + window.dispatchEvent(new Event('resize')); + assert.notCalled(listener); + }); + }); + describe('layout change notifier', () => { let layoutChangeHandlerSpy;