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

Update sidebar left margin 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 #2, because it gives a more consistent behaviour
for both sidebars, PDF and non-PDF.

In addition, I added a mechanism to register and unregister events, to
avoid resource leaks.
parent d4096622
...@@ -39,7 +39,7 @@ export default class PdfSidebar extends Sidebar { ...@@ -39,7 +39,7 @@ export default class PdfSidebar extends Sidebar {
this.sideBySideActive = false; this.sideBySideActive = false;
this.subscribe('sidebarLayoutChanged', state => this.fitSideBySide(state)); this.subscribe('sidebarLayoutChanged', state => this.fitSideBySide(state));
this.window.addEventListener('resize', () => this.fitSideBySide()); this._registerEvent(window, 'resize', () => this.fitSideBySide());
} }
/** /**
......
...@@ -18,8 +18,15 @@ import BucketBar from './plugin/bucket-bar'; ...@@ -18,8 +18,15 @@ import BucketBar from './plugin/bucket-bar';
* @prop {number} height * @prop {number} height
*/ */
/**
* @typedef RegisteredListener
* @prop {Window|HTMLElement} eventTarget
* @prop {string} eventType
* @prop {(event: any) => void} listener
*/
// Minimum width to which the frame can be resized. // Minimum width to which the frame can be resized.
const MIN_RESIZE = 280; export const MIN_RESIZE = 280;
/** /**
* Create the iframe that will load the sidebar application. * Create the iframe that will load the sidebar application.
...@@ -93,6 +100,9 @@ export default class Sidebar extends Guest { ...@@ -93,6 +100,9 @@ export default class Sidebar extends Guest {
const sidebarFrame = createSidebarIframe(config); const sidebarFrame = createSidebarIframe(config);
(frame || externalFrame).appendChild(sidebarFrame); (frame || externalFrame).appendChild(sidebarFrame);
/** @type {RegisteredListener[]} */
this.registeredListeners = [];
this.externalFrame = externalFrame; this.externalFrame = externalFrame;
this.frame = frame; this.frame = frame;
this.hypothesisSidebar = hypothesisSidebar; this.hypothesisSidebar = hypothesisSidebar;
...@@ -123,7 +133,7 @@ export default class Sidebar extends Guest { ...@@ -123,7 +133,7 @@ export default class Sidebar extends Guest {
} }
if (this.plugins.BucketBar) { if (this.plugins.BucketBar) {
this.plugins.BucketBar.element.addEventListener('click', () => this._registerEvent(this.plugins.BucketBar.element, 'click', () =>
this.show() this.show()
); );
} }
...@@ -147,6 +157,8 @@ export default class Sidebar extends Guest { ...@@ -147,6 +157,8 @@ export default class Sidebar extends Guest {
this.toolbarWidth = 0; this.toolbarWidth = 0;
} }
this._registerEvent(window, 'resize', () => this._onResize());
this._gestureState = { this._gestureState = {
// Initial position at the start of a drag/pan resize event (in pixels). // Initial position at the start of a drag/pan resize event (in pixels).
initial: /** @type {number|null} */ (null), initial: /** @type {number|null} */ (null),
...@@ -175,12 +187,30 @@ export default class Sidebar extends Guest { ...@@ -175,12 +187,30 @@ export default class Sidebar extends Guest {
} }
destroy() { destroy() {
this._unregisterEvents();
this._hammerManager?.destroy(); this._hammerManager?.destroy();
this.frame?.remove(); this.frame?.remove();
this.hypothesisSidebar?.remove(); this.hypothesisSidebar?.remove();
super.destroy(); super.destroy();
} }
/**
* @param {Window|HTMLElement} eventTarget
* @param {string} eventType
* @param {(event: any) => void} listener
*/
_registerEvent(eventTarget, eventType, listener) {
eventTarget.addEventListener(eventType, listener);
this.registeredListeners.push({ eventTarget, eventType, listener });
}
_unregisterEvents() {
this.registeredListeners.forEach(({ eventTarget, eventType, listener }) => {
eventTarget.removeEventListener(eventType, listener);
});
this.registeredListeners = [];
}
_setupSidebarEvents() { _setupSidebarEvents() {
annotationCounts(document.body, this.crossframe); annotationCounts(document.body, this.crossframe);
sidebarTrigger(document.body, () => this.show()); sidebarTrigger(document.body, () => this.show());
...@@ -222,11 +252,13 @@ export default class Sidebar extends Guest { ...@@ -222,11 +252,13 @@ export default class Sidebar extends Guest {
const toggleButton = this.toolbar.sidebarToggleButton; const toggleButton = this.toolbar.sidebarToggleButton;
if (toggleButton) { if (toggleButton) {
// Prevent any default gestures on the handle. // 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) this._hammerManager = new Hammer.Manager(toggleButton).on(
// eslint-disable-next-line no-restricted-properties 'panstart panend panleft panright',
.on('panstart panend panleft panright', this._onPan.bind(this)); /* istanbul ignore next */
event => this._onPan(event)
);
this._hammerManager.add( this._hammerManager.add(
new Hammer.Pan({ direction: Hammer.DIRECTION_HORIZONTAL }) new Hammer.Pan({ direction: Hammer.DIRECTION_HORIZONTAL })
); );
...@@ -315,6 +347,19 @@ export default class Sidebar extends Guest { ...@@ -315,6 +347,19 @@ export default class Sidebar extends Guest {
this.publish('sidebarLayoutChanged', [layoutState]); this.publish('sidebarLayoutChanged', [layoutState]);
} }
/**
* On window resize events, update the marginLeft of the sidebar by calling hide/show methods.
*/
_onResize() {
if (this.toolbar.sidebarOpen === true) {
if (window.innerWidth < MIN_RESIZE) {
this.hide();
} else {
this.show();
}
}
}
_onPan(event) { _onPan(event) {
const frame = this.frame; const frame = this.frame;
if (!frame) { if (!frame) {
......
import events from '../../shared/bridge-events'; import events from '../../shared/bridge-events';
import Sidebar from '../sidebar'; import Sidebar, { MIN_RESIZE } from '../sidebar';
import { $imports } from '../sidebar'; import { $imports } from '../sidebar';
const DEFAULT_WIDTH = 350; const DEFAULT_WIDTH = 350;
...@@ -192,7 +192,7 @@ describe('Sidebar', () => { ...@@ -192,7 +192,7 @@ describe('Sidebar', () => {
describe('toolbar buttons', () => { describe('toolbar buttons', () => {
it('shows or hides sidebar when toolbar button is clicked', () => { it('shows or hides sidebar when toolbar button is clicked', () => {
const sidebar = createSidebar({}); const sidebar = createSidebar();
sinon.stub(sidebar, 'show'); sinon.stub(sidebar, 'show');
sinon.stub(sidebar, 'hide'); sinon.stub(sidebar, 'hide');
...@@ -204,7 +204,7 @@ describe('Sidebar', () => { ...@@ -204,7 +204,7 @@ describe('Sidebar', () => {
}); });
it('shows or hides highlights when toolbar button is clicked', () => { it('shows or hides highlights when toolbar button is clicked', () => {
const sidebar = createSidebar({}); const sidebar = createSidebar();
sinon.stub(sidebar, 'setAllVisibleHighlights'); sinon.stub(sidebar, 'setAllVisibleHighlights');
FakeToolbarController.args[0][1].setHighlightsVisible(true); FakeToolbarController.args[0][1].setHighlightsVisible(true);
...@@ -216,7 +216,7 @@ describe('Sidebar', () => { ...@@ -216,7 +216,7 @@ describe('Sidebar', () => {
}); });
it('creates an annotation when toolbar button is clicked', () => { it('creates an annotation when toolbar button is clicked', () => {
const sidebar = createSidebar({}); const sidebar = createSidebar();
sinon.stub(sidebar, 'createAnnotation'); sinon.stub(sidebar, 'createAnnotation');
FakeToolbarController.args[0][1].createAnnotation(); FakeToolbarController.args[0][1].createAnnotation();
...@@ -330,7 +330,7 @@ describe('Sidebar', () => { ...@@ -330,7 +330,7 @@ describe('Sidebar', () => {
}); });
it('does not crash if there is no services', () => { it('does not crash if there is no services', () => {
createSidebar({}); // No config.services createSidebar(); // No config.services
emitEvent(events.LOGIN_REQUESTED); emitEvent(events.LOGIN_REQUESTED);
}); });
...@@ -390,7 +390,7 @@ describe('Sidebar', () => { ...@@ -390,7 +390,7 @@ describe('Sidebar', () => {
let sidebar; let sidebar;
beforeEach(() => { beforeEach(() => {
sidebar = createSidebar({}); sidebar = createSidebar();
}); });
describe('panstart event', () => { describe('panstart event', () => {
...@@ -487,7 +487,7 @@ describe('Sidebar', () => { ...@@ -487,7 +487,7 @@ describe('Sidebar', () => {
}); });
it('does not show the sidebar if not configured to.', () => { it('does not show the sidebar if not configured to.', () => {
const sidebar = createSidebar({}); const sidebar = createSidebar();
const show = sandbox.stub(sidebar, 'show'); const show = sandbox.stub(sidebar, 'show');
sidebar.publish('panelReady'); sidebar.publish('panelReady');
assert.notCalled(show); assert.notCalled(show);
...@@ -558,7 +558,7 @@ describe('Sidebar', () => { ...@@ -558,7 +558,7 @@ describe('Sidebar', () => {
describe('#setAllVisibleHighlights', () => describe('#setAllVisibleHighlights', () =>
it('sets the state through crossframe and emits', () => { it('sets the state through crossframe and emits', () => {
const sidebar = createSidebar({}); const sidebar = createSidebar();
sidebar.setAllVisibleHighlights(true); sidebar.setAllVisibleHighlights(true);
assert.calledWith(fakeCrossFrame.call, 'setVisibleHighlights', true); assert.calledWith(fakeCrossFrame.call, 'setVisibleHighlights', true);
})); }));
...@@ -569,10 +569,59 @@ describe('Sidebar', () => { ...@@ -569,10 +569,59 @@ describe('Sidebar', () => {
}); });
it('shows toolbar controls when using the default theme', () => { it('shows toolbar controls when using the default theme', () => {
createSidebar({}); createSidebar();
assert.equal(fakeToolbar.useMinimalControls, false); assert.equal(fakeToolbar.useMinimalControls, false);
}); });
describe('window resize events', () => {
it('hides the sidebar if window width is < MIN_RESIZE', () => {
const sidebar = createSidebar({ openSidebar: true });
sidebar.publish('panelReady');
window.innerWidth = MIN_RESIZE - 1;
window.dispatchEvent(new Event('resize'));
assert.equal(fakeToolbar.sidebarOpen, false);
});
it('invokes the "show" method when window is resized', () => {
// Calling the 'show' methods adjust the marginLeft at different screen sizes
const sidebar = createSidebar({ openSidebar: true });
sidebar.publish('panelReady');
sinon.stub(sidebar, 'show');
// Make the window very small
window.innerWidth = MIN_RESIZE;
window.dispatchEvent(new Event('resize'));
assert.calledOnce(sidebar.show);
// Make the window very large
window.innerWidth = MIN_RESIZE * 10;
window.dispatchEvent(new Event('resize'));
assert.calledTwice(sidebar.show);
});
});
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', () => { describe('layout change notifier', () => {
let layoutChangeHandlerSpy; let layoutChangeHandlerSpy;
......
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