Commit 3c849a81 authored by Robert Knight's avatar Robert Knight

Remove sidebar toggle button swipe gesture handling

Remove logic for handling swipe gestures on the sidebar's toggle button.
Trying to perform a swipe on a target as small as the toggle button is
difficult and it doesn't offer advantages over just tapping the button
to open and close. Behavior that _might_ be useful is being able to
two-finger swipe on the whole sidebar to open or close it, but this conflicts
with the browser's own swipe gesture handling. Just remove it entirely
for now.

 - Remove swipe gesture handling

 - Remove unnecessary use of jQuery

 - Ensure that the `_setupGestures` code path is exercised by tests
parent 43b7fef8
import $ from 'jquery';
import Hammer from 'hammerjs'; import Hammer from 'hammerjs';
import annotationCounts from './annotation-counts'; import annotationCounts from './annotation-counts';
...@@ -90,6 +89,11 @@ export default class Sidebar extends Host { ...@@ -90,6 +89,11 @@ export default class Sidebar extends Host {
this._setupSidebarEvents(); this._setupSidebarEvents();
} }
destroy() {
this._hammerManager?.destroy();
super.destroy();
}
_setupSidebarEvents() { _setupSidebarEvents() {
annotationCounts(document.body, this.crossframe); annotationCounts(document.body, this.crossframe);
sidebarTrigger(document.body, () => this.show()); sidebarTrigger(document.body, () => this.show());
...@@ -117,27 +121,17 @@ export default class Sidebar extends Host { ...@@ -117,27 +121,17 @@ export default class Sidebar extends Host {
} }
_setupGestures() { _setupGestures() {
const toggle = $(this.toolbar.sidebarToggleButton); const toggleButton = this.toolbar.sidebarToggleButton;
if (toggleButton) {
if (toggle[0]) {
// Prevent any default gestures on the handle. // Prevent any default gestures on the handle.
toggle.on('touchmove', e => e.preventDefault()); toggleButton.addEventListener('touchmove', e => e.preventDefault());
// Set up Hammer instance and handlers. this._hammerManager = new Hammer.Manager(toggleButton)
const manager = new Hammer.Manager(toggle[0])
// eslint-disable-next-line no-restricted-properties
.on('panstart panend panleft panright', this._onPan.bind(this))
// eslint-disable-next-line no-restricted-properties // eslint-disable-next-line no-restricted-properties
.on('swipeleft swiperight', this._onSwipe.bind(this)); .on('panstart panend panleft panright', this._onPan.bind(this));
this._hammerManager.add(
// Set up gesture recognition.
const pan = manager.add(
new Hammer.Pan({ direction: Hammer.DIRECTION_HORIZONTAL }) new Hammer.Pan({ direction: Hammer.DIRECTION_HORIZONTAL })
); );
const swipe = manager.add(
new Hammer.Swipe({ direction: Hammer.DIRECTION_HORIZONTAL })
);
swipe.recognizeWith(pan);
} }
} }
...@@ -271,17 +265,6 @@ export default class Sidebar extends Host { ...@@ -271,17 +265,6 @@ export default class Sidebar extends Host {
} }
} }
_onSwipe(event) {
switch (event.type) {
case 'swipeleft':
this.show();
break;
case 'swiperight':
this.hide();
break;
}
}
show() { show() {
this.crossframe.call('sidebarOpened'); this.crossframe.call('sidebarOpened');
......
...@@ -15,6 +15,9 @@ describe('Sidebar', () => { ...@@ -15,6 +15,9 @@ describe('Sidebar', () => {
let fakeCrossFrame; let fakeCrossFrame;
const sidebarConfig = { pluginClasses: {} }; const sidebarConfig = { pluginClasses: {} };
// `Sidebar` instances created by current test.
let sidebars;
let FakeToolbarController; let FakeToolbarController;
let fakeToolbar; let fakeToolbar;
...@@ -32,7 +35,11 @@ describe('Sidebar', () => { ...@@ -32,7 +35,11 @@ describe('Sidebar', () => {
} }
config = Object.assign({}, sidebarConfig, config); config = Object.assign({}, sidebarConfig, config);
const element = document.createElement('div'); const element = document.createElement('div');
return new Sidebar(element, config); const sidebar = new Sidebar(element, config);
sidebars.push(sidebar);
return sidebar;
}; };
const createExternalContainer = () => { const createExternalContainer = () => {
...@@ -44,8 +51,6 @@ describe('Sidebar', () => { ...@@ -44,8 +51,6 @@ describe('Sidebar', () => {
}; };
beforeEach(() => { beforeEach(() => {
sandbox.stub(Sidebar.prototype, '_setupGestures');
fakeCrossFrame = {}; fakeCrossFrame = {};
fakeCrossFrame.onConnect = sandbox.stub().returns(fakeCrossFrame); fakeCrossFrame.onConnect = sandbox.stub().returns(fakeCrossFrame);
fakeCrossFrame.on = sandbox.stub().returns(fakeCrossFrame); fakeCrossFrame.on = sandbox.stub().returns(fakeCrossFrame);
...@@ -75,6 +80,8 @@ describe('Sidebar', () => { ...@@ -75,6 +80,8 @@ describe('Sidebar', () => {
sidebarConfig.pluginClasses.CrossFrame = CrossFrame; sidebarConfig.pluginClasses.CrossFrame = CrossFrame;
sidebarConfig.pluginClasses.BucketBar = BucketBar; sidebarConfig.pluginClasses.BucketBar = BucketBar;
sidebars = [];
$imports.$mock({ $imports.$mock({
'./toolbar': { './toolbar': {
ToolbarController: FakeToolbarController, ToolbarController: FakeToolbarController,
...@@ -83,6 +90,7 @@ describe('Sidebar', () => { ...@@ -83,6 +90,7 @@ describe('Sidebar', () => {
}); });
afterEach(() => { afterEach(() => {
sidebars.forEach(s => s.destroy());
sandbox.restore(); sandbox.restore();
$imports.$restore(); $imports.$restore();
}); });
...@@ -361,26 +369,6 @@ describe('Sidebar', () => { ...@@ -361,26 +369,6 @@ describe('Sidebar', () => {
}); });
}); });
describe('swipe gestures', () => {
let sidebar;
beforeEach(() => {
sidebar = createSidebar({});
});
it('opens the sidebar on swipeleft', () => {
const show = sandbox.stub(sidebar, 'show');
sidebar._onSwipe({ type: 'swipeleft' });
assert.calledOnce(show);
});
it('closes the sidebar on swiperight', () => {
const hide = sandbox.stub(sidebar, 'hide');
sidebar._onSwipe({ type: 'swiperight' });
assert.calledOnce(hide);
});
});
describe('destruction', () => { describe('destruction', () => {
let sidebar; let sidebar;
......
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