From 1b5c4076d38fadef2af27e27a0c2bd89066a4868 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Mon, 4 Nov 2024 15:33:09 +0000 Subject: [PATCH] Replace hammer.js with local code Hammer.js is used to facilitate resizing the sidebar by dragging the sidebar button. For this use case, we can avoid a dependency which is not actively maintained, get more visibility into what the code is doing and make testing easier by using a small amount of our own code. In the process the tests for drag resizing in the sidebar were refactored to avoid referencing private fields of `Sidebar` and mocking `getComputedStyle`. The user-facing behavior should be unchanged. --- package.json | 2 - src/annotator/sidebar.tsx | 78 ++++++------- src/annotator/test/sidebar-test.js | 117 +++++++++++-------- src/annotator/util/drag-handler.tsx | 94 +++++++++++++++ src/annotator/util/test/drag-handler-test.js | 84 +++++++++++++ yarn.lock | 16 --- 6 files changed, 284 insertions(+), 107 deletions(-) create mode 100644 src/annotator/util/drag-handler.tsx create mode 100644 src/annotator/util/test/drag-handler-test.js diff --git a/package.json b/package.json index 361e5f4a1cf..adc7214ae9b 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,6 @@ "@types/chai": "^5.0.0", "@types/dompurify": "^3.0.0", "@types/escape-html": "^1.0.1", - "@types/hammerjs": "^2.0.41", "@types/katex": "^0.16.0", "@types/retry": "^0.12.1", "@types/scroll-into-view": "^1.16.0", @@ -69,7 +68,6 @@ "globals": "^15.11.0", "gulp": "^5.0.0", "gulp-changed": "^5.0.1", - "hammerjs": "^2.0.4", "karma": "^6.0.1", "karma-chrome-launcher": "^3.1.0", "karma-coverage-istanbul-reporter": "^3.0.2", diff --git a/src/annotator/sidebar.tsx b/src/annotator/sidebar.tsx index 6858da913a6..3ba6c4920ed 100644 --- a/src/annotator/sidebar.tsx +++ b/src/annotator/sidebar.tsx @@ -1,6 +1,5 @@ import type { ToastMessage } from '@hypothesis/frontend-shared'; import classnames from 'classnames'; -import * as Hammer from 'hammerjs'; import { render } from 'preact'; import { addConfigFragment } from '../shared/config-fragment'; @@ -26,6 +25,8 @@ import { createAppConfig } from './config/app'; import { FeatureFlags } from './features'; import { sidebarTrigger } from './sidebar-trigger'; import { ToolbarController } from './toolbar'; +import { DragHandler } from './util/drag-handler'; +import type { DragHandlerEvent } from './util/drag-handler'; import type { Emitter, EventBus } from './util/emitter'; import { createShadowRoot } from './util/shadow-root'; @@ -90,7 +91,7 @@ function createSidebarIframe(config: SidebarConfig): HTMLIFrameElement { return sidebarFrame; } -type GestureState = { +type DragResizeState = { /** Initial position at the start of a drag/pan resize event (in pixels). */ initial: number | null; /** Final position at end of drag resize event. */ @@ -104,10 +105,10 @@ type GestureState = { export class Sidebar implements Destroyable { private _emitter: Emitter; private _config: SidebarContainerConfig & SidebarConfig; + private _dragResizeHandler: DragHandler; + private _dragResizeState: DragResizeState; private _listeners: ListenerCollection; - private _gestureState: GestureState; private _layoutState: SidebarLayout; - private _hammerManager: HammerManager | undefined; private _hypothesisSidebar: HTMLElement | undefined; private _messagesElement: HTMLElement | undefined; private _toolbarWidth: number; @@ -287,11 +288,14 @@ export class Sidebar implements Destroyable { this._listeners.add(window, 'resize', () => this._onResize()); - this._gestureState = { + this._dragResizeState = { initial: null, final: null, }; - this._setupGestures(); + this._dragResizeHandler = new DragHandler({ + target: this.toolbar.sidebarToggleButton, + onDrag: event => this._onDragSidebarToggleButton(event), + }); this.close(); // Publisher-provided callback functions @@ -323,7 +327,7 @@ export class Sidebar implements Destroyable { this._sidebarRPC.destroy(); this.bucketBar?.destroy(); this._listeners.removeAll(); - this._hammerManager?.destroy(); + this._dragResizeHandler.destroy(); if (this._hypothesisSidebar) { // Explicitly unmounting the "messages" element, to make sure effects are clean-up render(null, this._messagesElement!); @@ -482,23 +486,8 @@ export class Sidebar implements Destroyable { }); } - _resetGestureState() { - this._gestureState = { initial: null, final: null }; - } - - _setupGestures() { - const toggleButton = this.toolbar.sidebarToggleButton; - if (toggleButton) { - this._hammerManager = new Hammer.Manager(toggleButton); - this._hammerManager.on( - 'panstart panend panleft panright', - /* istanbul ignore next */ - event => this._onPan(event), - ); - this._hammerManager.add( - new Hammer.Pan({ direction: Hammer.DIRECTION_HORIZONTAL }), - ); - } + _resetDragResizeState() { + this._dragResizeState = { initial: null, final: null }; } // Schedule any changes needed to update the sidebar layout. @@ -513,10 +502,11 @@ export class Sidebar implements Destroyable { this._renderFrame = undefined; if ( - this._gestureState.final !== this._gestureState.initial && + typeof this._dragResizeState.final === 'number' && + this._dragResizeState.final !== this._dragResizeState.initial && this.iframeContainer ) { - const margin: number = this._gestureState.final!; + const margin = this._dragResizeState.final; const width = -margin; this.iframeContainer.style.marginLeft = `${margin}px`; if (width >= MIN_RESIZE) { @@ -591,7 +581,7 @@ export class Sidebar implements Destroyable { } /** - * On window resize events, update the marginLeft of the sidebar by calling hide/show methods. + * On window resize events, update the marginLeft of the sidebar by calling hide/show methods. */ _onResize() { if (this.toolbar.sidebarOpen === true) { @@ -603,15 +593,24 @@ export class Sidebar implements Destroyable { } } - _onPan(event: HammerInput) { + /** Return true if the user is currently resizing the sidebar. */ + isResizing() { + return this._dragResizeState.initial !== null; + } + + /** + * Event handler invoked when user drags the sidebar toggle button in order + * to resize the sidebar. + */ + _onDragSidebarToggleButton(event: DragHandlerEvent) { const frame = this.iframeContainer; if (!frame) { return; } switch (event.type) { - case 'panstart': - this._resetGestureState(); + case 'dragstart': + this._resetDragResizeState(); // Disable animated transition of sidebar position frame.classList.add('sidebar-no-transition'); @@ -619,12 +618,12 @@ export class Sidebar implements Destroyable { // Disable pointer events on the iframe. frame.style.pointerEvents = 'none'; - this._gestureState.initial = parseInt( + this._dragResizeState.initial = parseInt( getComputedStyle(frame).marginLeft, ); break; - case 'panend': + case 'dragend': frame.classList.remove('sidebar-no-transition'); // Re-enable pointer events on the iframe. @@ -632,24 +631,23 @@ export class Sidebar implements Destroyable { // Snap open or closed. if ( - this._gestureState.final === null || - this._gestureState.final <= -MIN_RESIZE + this._dragResizeState.final === null || + this._dragResizeState.final <= -MIN_RESIZE ) { this.open(); } else { this.close(); } - this._resetGestureState(); + this._resetDragResizeState(); break; - case 'panleft': - case 'panright': { - if (typeof this._gestureState.initial !== 'number') { + case 'dragmove': { + if (typeof this._dragResizeState.initial !== 'number') { return; } - const margin = this._gestureState.initial; + const margin = this._dragResizeState.initial; const delta = event.deltaX; - this._gestureState.final = Math.min(Math.round(margin + delta), 0); + this._dragResizeState.final = Math.min(Math.round(margin + delta), 0); this._updateLayout(); break; } diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js index 57fcdeda701..2cdf4eb00db 100644 --- a/src/annotator/test/sidebar-test.js +++ b/src/annotator/test/sidebar-test.js @@ -20,6 +20,8 @@ describe('Sidebar', () => { let containers; let sidebars; + let FakeDragHandler; + let fakeDragHandler; let FakePortRPC; let fakePortRPCs; let FakeBucketBar; @@ -30,6 +32,8 @@ describe('Sidebar', () => { let fakeEmitter; before(() => { + // Make `requestAnimationFrame` invoke its callback synchronously. rAF is + // used to debounce some internal actions. sinon.stub(window, 'requestAnimationFrame').yields(); }); @@ -119,6 +123,12 @@ describe('Sidebar', () => { return externalFrame; }; + // Simulate a drag event on the sidebar toggle button. + const fireDragEvent = event => { + const { onDrag } = FakeDragHandler.getCall(0).args[0]; + onDrag(event); + }; + beforeEach(() => { sidebars = []; containers = []; @@ -141,6 +151,11 @@ describe('Sidebar', () => { }; FakeBucketBar = sinon.stub().returns(fakeBucketBar); + fakeDragHandler = { + destroy: sinon.stub(), + }; + FakeDragHandler = sinon.stub().returns(fakeDragHandler); + fakeToolbar = { getWidth: sinon.stub().returns(100), useMinimalControls: false, @@ -169,6 +184,7 @@ describe('Sidebar', () => { './toolbar': { ToolbarController: FakeToolbarController, }, + './util/drag-handler': { DragHandler: FakeDragHandler }, }); }); @@ -598,67 +614,80 @@ describe('Sidebar', () => { }); }); - describe('pan gestures', () => { + describe('when the sidebar toggle button is dragged', () => { let sidebar; beforeEach(() => { sidebar = createSidebar(); }); - describe('panstart event', () => { - it('disables pointer events and transitions on the widget', () => { - sidebar._onPan({ type: 'panstart' }); + /** Simulate the start of a drag of the sidebar's toggle button. */ + function startDrag() { + // Set the initial size of the sidebar to the minimum size. If a drag + // resize would make it any smaller, it will snap closed. + sidebar.iframeContainer.style.marginLeft = `-${MIN_RESIZE}px`; + fireDragEvent({ type: 'dragstart' }); + } + + describe('when a drag starts', () => { + it('begins resize', () => { + startDrag(); + assert.isTrue(sidebar.isResizing()); + }); + it('disables pointer events and transitions on the widget', () => { + startDrag(); assert.isTrue( sidebar.iframeContainer.classList.contains('sidebar-no-transition'), ); assert.equal(sidebar.iframeContainer.style.pointerEvents, 'none'); }); + }); - it('captures the left margin as the gesture initial state', () => { - sandbox - .stub(window, 'getComputedStyle') - .returns({ marginLeft: '100px' }); - sidebar._onPan({ type: 'panstart' }); - assert.equal(sidebar._gestureState.initial, '100'); + describe('when drag ends', () => { + it('ends resize', () => { + startDrag(); + fireDragEvent({ type: 'dragend' }); + assert.isFalse(sidebar.isResizing()); }); - }); - describe('panend event', () => { it('enables pointer events and transitions on the widget', () => { - sidebar._gestureState = { final: 0 }; - sidebar._onPan({ type: 'panend' }); + startDrag(); + fireDragEvent({ type: 'dragend' }); assert.isFalse( sidebar.iframeContainer.classList.contains('sidebar-no-transition'), ); assert.equal(sidebar.iframeContainer.style.pointerEvents, ''); }); - it('calls `open` if the widget is fully visible', () => { - sidebar._gestureState = { final: -500 }; - const open = sandbox.stub(sidebar, 'open'); - sidebar._onPan({ type: 'panend' }); - assert.calledOnce(open); + it('opens sidebar if final width is above threshold', () => { + startDrag(); + fireDragEvent({ type: 'dragmove', deltaX: 0 }); + fireDragEvent({ type: 'dragend' }); + assert.isTrue(sidebar.toolbar.sidebarOpen); }); - it('calls `close` if the widget is not fully visible', () => { - sidebar._gestureState = { final: -100 }; - const close = sandbox.stub(sidebar, 'close'); - sidebar._onPan({ type: 'panend' }); - assert.calledOnce(close); + it('closes sidebar if final width is below threshold', () => { + startDrag(); + fireDragEvent({ type: 'dragmove', deltaX: 50 }); + fireDragEvent({ type: 'dragend' }); + assert.isFalse(sidebar.toolbar.sidebarOpen); }); }); - describe('panleft and panright events', () => - it('shrinks or grows the widget to match the delta', () => { - sidebar._gestureState = { initial: -100 }; + describe('when toolbar button is dragged', () => { + it('shrinks or grows the widget to match the delta', async () => { + startDrag(); - sidebar._onPan({ type: 'panleft', deltaX: -50 }); - assert.equal(sidebar._gestureState.final, -150); + fireDragEvent({ type: 'dragmove', deltaX: -50 }); + const expected = `-${MIN_RESIZE + 50}px`; + assert.equal(sidebar.iframeContainer.style.marginLeft, expected); - sidebar._onPan({ type: 'panright', deltaX: 100 }); - assert.equal(sidebar._gestureState.final, 0); - })); + fireDragEvent({ type: 'dragmove', deltaX: -20 }); + const expected2 = `-${MIN_RESIZE + 20}px`; + assert.equal(sidebar.iframeContainer.style.marginLeft, expected2); + }); + }); }); describe('when the sidebar application has loaded', () => { @@ -946,21 +975,14 @@ describe('Sidebar', () => { ); }); - it('notifies when sidebar is panned left', () => { - sidebar._gestureState = { initial: -DEFAULT_WIDTH }; - sidebar._onPan({ type: 'panleft', deltaX: -50 }); + it('notifies when sidebar is drag-resized', async () => { + sidebar.iframeContainer.style.marginLeft = `-${DEFAULT_WIDTH}px`; + fireDragEvent({ type: 'dragstart' }); + fireDragEvent({ type: 'dragmove', deltaX: -50 }); assertLayoutValues(layoutChangeHandlerSpy.lastCall.args[0], { width: DEFAULT_WIDTH + 50 + fakeToolbar.getWidth(), }); }); - - it('notifies when sidebar is panned right', () => { - sidebar._gestureState = { initial: -DEFAULT_WIDTH }; - sidebar._onPan({ type: 'panright', deltaX: 50 }); - assertLayoutValues(layoutChangeHandlerSpy.lastCall.args[0], { - width: DEFAULT_WIDTH - 50 + fakeToolbar.getWidth(), - }); - }); }); describe('with the frame in an external container', () => { @@ -1018,12 +1040,9 @@ describe('Sidebar', () => { assert.notExists(sidebar.iframe.parentElement); }); - it('ignores pan events', () => { - sandbox - .stub(window, 'getComputedStyle') - .returns({ marginLeft: '100px' }); - sidebar._onPan({ type: 'panstart' }); - assert.isNull(sidebar._gestureState.initial); + it('ignores sidebar drag events', () => { + fireDragEvent({ type: 'dragstart' }); + assert.isFalse(sidebar.isResizing()); }); }); }); diff --git a/src/annotator/util/drag-handler.tsx b/src/annotator/util/drag-handler.tsx new file mode 100644 index 00000000000..1c24c5a52fd --- /dev/null +++ b/src/annotator/util/drag-handler.tsx @@ -0,0 +1,94 @@ +import { ListenerCollection } from '../../shared/listener-collection'; +import type { Destroyable } from '../../types/annotator'; + +/** + * Events emitted by {@link DragHandler}. + * + * This is named `DragHandlerEvent` to avoid confusion with {@link DragEvent}. + */ +export type DragHandlerEvent = { + type: 'dragstart' | 'dragend' | 'dragmove'; + + /** Distance that the pointer has moved by since the start of the drag. */ + deltaX: number; +}; + +export type DragOptions = { + /** Element where the pointer must be pressed to start the drag. */ + target: HTMLElement; + + /** Callback to invoke when drag events occur. */ + onDrag: (event: DragHandlerEvent) => void; + + /** + * Threshold that pointer must move from where it is initially pressed before + * a drag starts. + */ + threshold?: number; +}; + +/** + * Utility which recognizes drag/pan gestures on a control and reports events + * when a drag is in progress. + */ +export class DragHandler implements Destroyable { + private _listeners: ListenerCollection; + + /** Pointer position in the viewport at the start of the drag operation. */ + private _startX: number | null; + + private _dragActive: boolean; + + private _threshold: number; + + /** + * Construct a drag handler which triggers drag events when the user presses + * `target` and moves the pointer. + */ + constructor({ target, threshold = 10, onDrag }: DragOptions) { + // Disable the browser's own pan/scroll gestures on the target. Otherwise + // the drag action will not work on mobile. + target.style.touchAction = 'none'; + + this._listeners = new ListenerCollection(); + + this._startX = null; + this._dragActive = false; + this._threshold = threshold; + + this._listeners.add(target, 'pointerdown', event => { + this._startX = event.clientX; + }); + + const onCancel = (event: PointerEvent) => { + if (this._startX !== null && this._dragActive) { + const deltaX = event.clientX - this._startX; + onDrag({ type: 'dragend', deltaX }); + } + this._startX = null; + this._dragActive = false; + }; + this._listeners.add(window, 'pointercancel', onCancel); + this._listeners.add(window, 'pointerup', onCancel); + + this._listeners.add(window, 'pointermove', event => { + if (this._startX === null) { + return; + } + + const deltaX = event.clientX - this._startX; + if (!this._dragActive && Math.abs(deltaX) >= this._threshold) { + this._dragActive = true; + onDrag({ type: 'dragstart', deltaX }); + } + + if (this._dragActive) { + onDrag({ type: 'dragmove', deltaX }); + } + }); + } + + destroy() { + this._listeners.removeAll(); + } +} diff --git a/src/annotator/util/test/drag-handler-test.js b/src/annotator/util/test/drag-handler-test.js new file mode 100644 index 00000000000..3857288c409 --- /dev/null +++ b/src/annotator/util/test/drag-handler-test.js @@ -0,0 +1,84 @@ +import { DragHandler } from '../drag-handler'; + +describe('DragHandler', () => { + let handler; + let target; + let threshold; + let onDrag; + + beforeEach(() => { + target = document.createElement('button'); + document.body.append(target); + + threshold = 10; + + onDrag = sinon.stub(); + handler = new DragHandler({ target, onDrag, threshold }); + }); + + afterEach(() => { + handler.destroy(); + target.remove(); + }); + + const firePointerDown = (clientX = 0) => { + target.dispatchEvent( + new PointerEvent('pointerdown', { clientX, bubbles: true }), + ); + }; + + // Pointer move and up events are dispatched at the body, but they could be + // dispatched at any element in the window. + + const firePointerMove = clientX => { + document.body.dispatchEvent( + new PointerEvent('pointermove', { clientX, bubbles: true }), + ); + }; + + const firePointerUp = () => { + document.body.dispatchEvent( + new PointerEvent('pointerup', { bubbles: true }), + ); + }; + + const beginDrag = distance => { + firePointerDown(); + firePointerMove(distance); + }; + + it('disables browser pan gestures on the target', () => { + assert.equal(target.style.touchAction, 'none'); + }); + + it('fires "dragstart" when target is pressed and moved by at least `threshold` pixels', () => { + firePointerDown(); + assert.notCalled(onDrag); + + firePointerMove(threshold + 5); + assert.calledWith(onDrag, { type: 'dragstart', deltaX: threshold + 5 }); + }); + + ['pointercancel', 'pointerup'].forEach(eventType => { + it('fires "dragend" when pointer is released', () => { + beginDrag(threshold + 5); + document.body.dispatchEvent( + new PointerEvent(eventType, { bubbles: true }), + ); + assert.calledWith(onDrag, { type: 'dragend', deltaX: 0 }); + }); + }); + + it('fires "dragmove" when pointer moves while a drag is active', () => { + beginDrag(threshold + 5); + onDrag.resetHistory(); + firePointerMove(20); + assert.calledWith(onDrag, { type: 'dragmove', deltaX: 20 }); + }); + + it('does not fire "dragmove" or "dragend" if a drag is not active', () => { + firePointerUp(); + firePointerMove(10); + assert.notCalled(onDrag); + }); +}); diff --git a/yarn.lock b/yarn.lock index 71dadf5861f..b1b3c76207b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4427,13 +4427,6 @@ __metadata: languageName: node linkType: hard -"@types/hammerjs@npm:^2.0.41": - version: 2.0.46 - resolution: "@types/hammerjs@npm:2.0.46" - checksum: caba6ec788d19905c71092670b58514b3d1f5eee5382bf9205e8df688d51e7857b7994e2dd7aed57fac8977bdf0e456d67fbaf23440a4385b8ce25fe2af1ec39 - languageName: node - linkType: hard - "@types/json-schema@npm:^7.0.15": version: 7.0.15 resolution: "@types/json-schema@npm:7.0.15" @@ -8373,13 +8366,6 @@ __metadata: languageName: node linkType: hard -"hammerjs@npm:^2.0.4": - version: 2.0.8 - resolution: "hammerjs@npm:2.0.8" - checksum: b092da7d1565a165d7edb53ef0ce212837a8b11f897aa3cf81a7818b66686b0ab3f4747fbce8fc8a41d1376594639ce3a054b0fd4889ca8b5b136a29ca500e27 - languageName: node - linkType: hard - "has-bigints@npm:^1.0.1": version: 1.0.1 resolution: "has-bigints@npm:1.0.1" @@ -8694,7 +8680,6 @@ __metadata: "@types/chai": ^5.0.0 "@types/dompurify": ^3.0.0 "@types/escape-html": ^1.0.1 - "@types/hammerjs": ^2.0.41 "@types/katex": ^0.16.0 "@types/retry": ^0.12.1 "@types/scroll-into-view": ^1.16.0 @@ -8732,7 +8717,6 @@ __metadata: globals: ^15.11.0 gulp: ^5.0.0 gulp-changed: ^5.0.1 - hammerjs: ^2.0.4 karma: ^6.0.1 karma-chrome-launcher: ^3.1.0 karma-coverage-istanbul-reporter: ^3.0.2