From 54bae0168f7e0a2e2032184f7ad94aaea785b746 Mon Sep 17 00:00:00 2001 From: David Brooks Date: Tue, 14 May 2019 12:22:58 +1200 Subject: [PATCH 1/5] Handle mouse wheel zoom events as if they've come from a trackpad. --- src/ol/interaction/MouseWheelZoom.js | 66 ++++------------------------ 1 file changed, 8 insertions(+), 58 deletions(-) diff --git a/src/ol/interaction/MouseWheelZoom.js b/src/ol/interaction/MouseWheelZoom.js index 1ed762bc17..443dc004e9 100644 --- a/src/ol/interaction/MouseWheelZoom.js +++ b/src/ol/interaction/MouseWheelZoom.js @@ -8,15 +8,6 @@ import Interaction, {zoomByDelta} from './Interaction.js'; import {clamp} from '../math.js'; -/** - * @enum {string} - */ -export const Mode = { - TRACKPAD: 'trackpad', - WHEEL: 'wheel' -}; - - /** * @typedef {Object} Options * @property {import("../events/condition.js").Condition} [condition] A function that @@ -107,12 +98,6 @@ class MouseWheelZoom extends Interaction { */ this.timeoutId_; - /** - * @private - * @type {Mode|undefined} - */ - this.mode_ = undefined; - /** * Trackpad events separated by this delay will be considered separate * interactions. @@ -206,51 +191,16 @@ class MouseWheelZoom extends Interaction { this.startTime_ = now; } - if (!this.mode_ || now - this.startTime_ > this.trackpadEventGap_) { - this.mode_ = Math.abs(delta) < 4 ? - Mode.TRACKPAD : - Mode.WHEEL; - } - - if (this.mode_ === Mode.TRACKPAD) { - const view = map.getView(); - if (this.trackpadTimeoutId_) { - clearTimeout(this.trackpadTimeoutId_); - } else { - view.beginInteraction(); - } - this.trackpadTimeoutId_ = setTimeout(this.endInteraction_.bind(this), this.trackpadEventGap_); - view.adjustZoom(-delta / this.trackpadDeltaPerZoom_, this.lastAnchor_); - this.startTime_ = now; - return false; - } - - this.totalDelta_ += delta; - - const timeLeft = Math.max(this.timeout_ - (now - this.startTime_), 0); - - clearTimeout(this.timeoutId_); - this.timeoutId_ = setTimeout(this.handleWheelZoom_.bind(this, map), timeLeft); - - return false; - } - - /** - * @private - * @param {import("../PluggableMap.js").default} map Map. - */ - handleWheelZoom_(map) { const view = map.getView(); - if (view.getAnimating()) { - view.cancelAnimations(); + if (this.trackpadTimeoutId_) { + clearTimeout(this.trackpadTimeoutId_); + } else { + view.beginInteraction(); } - const delta = clamp(this.totalDelta_, -this.maxDelta_, this.maxDelta_); - zoomByDelta(view, -delta, this.lastAnchor_, this.duration_); - this.mode_ = undefined; - this.totalDelta_ = 0; - this.lastAnchor_ = null; - this.startTime_ = undefined; - this.timeoutId_ = undefined; + this.trackpadTimeoutId_ = setTimeout(this.endInteraction_.bind(this), this.trackpadEventGap_); + view.adjustZoom(-delta / this.trackpadDeltaPerZoom_, this.lastAnchor_); + this.startTime_ = now; + return false; } /** From 77658e5750025dfd3045c891de4a1c9ff4573a00 Mon Sep 17 00:00:00 2001 From: David Brooks Date: Tue, 28 May 2019 09:53:36 +1200 Subject: [PATCH 2/5] Ensure changes to zoom wheel handling pass tests. --- src/ol/interaction/MouseWheelZoom.js | 3 +-- test/spec/ol/interaction/mousewheelzoom.test.js | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ol/interaction/MouseWheelZoom.js b/src/ol/interaction/MouseWheelZoom.js index 443dc004e9..7546e6e399 100644 --- a/src/ol/interaction/MouseWheelZoom.js +++ b/src/ol/interaction/MouseWheelZoom.js @@ -4,8 +4,7 @@ import {always, focus} from '../events/condition.js'; import EventType from '../events/EventType.js'; import {DEVICE_PIXEL_RATIO, FIREFOX} from '../has.js'; -import Interaction, {zoomByDelta} from './Interaction.js'; -import {clamp} from '../math.js'; +import Interaction from './Interaction.js'; /** diff --git a/test/spec/ol/interaction/mousewheelzoom.test.js b/test/spec/ol/interaction/mousewheelzoom.test.js index ee1858bf5d..ee9b7fd87a 100644 --- a/test/spec/ol/interaction/mousewheelzoom.test.js +++ b/test/spec/ol/interaction/mousewheelzoom.test.js @@ -2,8 +2,8 @@ import Map from '../../../../src/ol/Map.js'; import MapBrowserEvent from '../../../../src/ol/MapBrowserEvent.js'; import View from '../../../../src/ol/View.js'; import Event from '../../../../src/ol/events/Event.js'; -import {DEVICE_PIXEL_RATIO, FIREFOX} from '../../../../src/ol/has.js'; -import MouseWheelZoom, {Mode} from '../../../../src/ol/interaction/MouseWheelZoom.js'; +import {DEVICE_PIXEL_RATIO, FIREFOX, SAFARI} from '../../../../src/ol/has.js'; +import MouseWheelZoom from '../../../../src/ol/interaction/MouseWheelZoom.js'; describe('ol.interaction.MouseWheelZoom', function() { @@ -66,7 +66,7 @@ describe('ol.interaction.MouseWheelZoom', function() { if (FIREFOX) { it('works on Firefox in DOM_DELTA_PIXEL mode (trackpad)', function(done) { map.once('postrender', function() { - expect(interaction.mode_).to.be(Mode.TRACKPAD); + expect(interaction.lastDelta_).to.be(1); done(); }); const event = new MapBrowserEvent('wheel', map, { @@ -84,7 +84,7 @@ describe('ol.interaction.MouseWheelZoom', function() { if (!FIREFOX) { it('works in DOM_DELTA_PIXEL mode (trackpad)', function(done) { map.once('postrender', function() { - expect(interaction.mode_).to.be(Mode.TRACKPAD); + expect(interaction.lastDelta_).to.be(1); done(); }); const event = new MapBrowserEvent('wheel', map, { From 03fcf1ca706f0aa2a4ee975b133679eb68f46c64 Mon Sep 17 00:00:00 2001 From: David Brooks Date: Wed, 29 May 2019 14:08:03 +1200 Subject: [PATCH 3/5] Get all mouse wheel tests passing (#9564). --- .../ol/interaction/mousewheelzoom.test.js | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/spec/ol/interaction/mousewheelzoom.test.js b/test/spec/ol/interaction/mousewheelzoom.test.js index ee9b7fd87a..f016a2adf8 100644 --- a/test/spec/ol/interaction/mousewheelzoom.test.js +++ b/test/spec/ol/interaction/mousewheelzoom.test.js @@ -63,6 +63,11 @@ describe('ol.interaction.MouseWheelZoom', function() { describe('handleEvent()', function() { + let view; + beforeEach(function() { + view = map.getView(); + }); + if (FIREFOX) { it('works on Firefox in DOM_DELTA_PIXEL mode (trackpad)', function(done) { map.once('postrender', function() { @@ -149,8 +154,65 @@ describe('ol.interaction.MouseWheelZoom', function() { map.handleMapBrowserEvent(event); }); + it('works in DOM_DELTA_LINE mode (wheel)', function(done) { + map.once('postrender', function() { + expect(view.getResolution()).to.be(2); + expect(view.getCenter()).to.eql([0, 0]); + done(); + }); + + const event = new MapBrowserEvent('wheel', map, { + type: 'wheel', + deltaMode: WheelEvent.DOM_DELTA_LINE, + deltaY: 7.5, + target: map.getViewport(), + preventDefault: Event.prototype.preventDefault + }); + event.coordinate = [0, 0]; + + map.handleMapBrowserEvent(event); }); + if (SAFARI) { + it('works on Safari (wheel)', function(done) { + map.once('postrender', function() { + expect(view.getResolution()).to.be(2); + expect(view.getCenter()).to.eql([0, 0]); + done(); + }); + + const event = new MapBrowserEvent('mousewheel', map, { + type: 'mousewheel', + wheelDeltaY: -900, + target: map.getViewport(), + preventDefault: Event.prototype.preventDefault + }); + event.coordinate = [0, 0]; + + map.handleMapBrowserEvent(event); + }); + } + + if (!SAFARI) { + it('works on other browsers (wheel)', function(done) { + map.once('postrender', function() { + expect(view.getResolution()).to.be(2); + expect(view.getCenter()).to.eql([0, 0]); + done(); + }); + + const event = new MapBrowserEvent('mousewheel', map, { + type: 'mousewheel', + wheelDeltaY: -300, + target: map.getViewport(), + preventDefault: Event.prototype.preventDefault + }); + event.coordinate = [0, 0]; + + map.handleMapBrowserEvent(event); + }); + } + }); }); From c8e340a623d3b7170184bb7fe879e05f8c1dd1ac Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 5 Feb 2020 12:58:21 +0100 Subject: [PATCH 4/5] Rename variables now that trackpads are not special any more --- src/ol/interaction/MouseWheelZoom.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/ol/interaction/MouseWheelZoom.js b/src/ol/interaction/MouseWheelZoom.js index 7546e6e399..37a4e4022f 100644 --- a/src/ol/interaction/MouseWheelZoom.js +++ b/src/ol/interaction/MouseWheelZoom.js @@ -92,29 +92,23 @@ class MouseWheelZoom extends Interaction { this.startTime_ = undefined; /** - * @private - * @type {?} - */ - this.timeoutId_; - - /** - * Trackpad events separated by this delay will be considered separate + * Events separated by this delay will be considered separate * interactions. * @type {number} */ - this.trackpadEventGap_ = 400; + this.eventGap_ = 400; /** * @type {?} */ - this.trackpadTimeoutId_; + this.timeoutId_; /** * The number of delta values per zoom level * @private * @type {number} */ - this.trackpadDeltaPerZoom_ = 300; + this.deltaPerZoom_ = 300; } @@ -136,7 +130,7 @@ class MouseWheelZoom extends Interaction { * @private */ endInteraction_() { - this.trackpadTimeoutId_ = undefined; + this.timeoutId_ = undefined; const view = this.getMap().getView(); view.endInteraction(undefined, this.lastDelta_ ? (this.lastDelta_ > 0 ? 1 : -1) : 0, this.lastAnchor_); } @@ -191,13 +185,13 @@ class MouseWheelZoom extends Interaction { } const view = map.getView(); - if (this.trackpadTimeoutId_) { - clearTimeout(this.trackpadTimeoutId_); + if (this.timeoutId_) { + clearTimeout(this.timeoutId_); } else { view.beginInteraction(); } - this.trackpadTimeoutId_ = setTimeout(this.endInteraction_.bind(this), this.trackpadEventGap_); - view.adjustZoom(-delta / this.trackpadDeltaPerZoom_, this.lastAnchor_); + this.timeoutId_ = setTimeout(this.endInteraction_.bind(this), this.eventGap_); + view.adjustZoom(-delta / this.deltaPerZoom_, this.lastAnchor_); this.startTime_ = now; return false; } From cc21f92bdb52dc30efa41ce0cc68f08435384fcd Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 5 Feb 2020 12:58:35 +0100 Subject: [PATCH 5/5] Restore test coverage, fix tests --- .../ol/interaction/mousewheelzoom.test.js | 134 ++++++------------ 1 file changed, 41 insertions(+), 93 deletions(-) diff --git a/test/spec/ol/interaction/mousewheelzoom.test.js b/test/spec/ol/interaction/mousewheelzoom.test.js index f016a2adf8..8eea3ea3c3 100644 --- a/test/spec/ol/interaction/mousewheelzoom.test.js +++ b/test/spec/ol/interaction/mousewheelzoom.test.js @@ -2,7 +2,7 @@ import Map from '../../../../src/ol/Map.js'; import MapBrowserEvent from '../../../../src/ol/MapBrowserEvent.js'; import View from '../../../../src/ol/View.js'; import Event from '../../../../src/ol/events/Event.js'; -import {DEVICE_PIXEL_RATIO, FIREFOX, SAFARI} from '../../../../src/ol/has.js'; +import {DEVICE_PIXEL_RATIO, FIREFOX} from '../../../../src/ol/has.js'; import MouseWheelZoom from '../../../../src/ol/interaction/MouseWheelZoom.js'; @@ -32,13 +32,13 @@ describe('ol.interaction.MouseWheelZoom', function() { describe('timeout duration', function() { let clock; beforeEach(function() { - sinon.spy(interaction, 'handleWheelZoom_'); + sinon.spy(interaction, 'endInteraction_'); clock = sinon.useFakeTimers(); }); afterEach(function() { clock.restore(); - interaction.handleWheelZoom_.restore(); + interaction.endInteraction_.restore(); }); it('works with the default value', function(done) { @@ -49,12 +49,12 @@ describe('ol.interaction.MouseWheelZoom', function() { }); map.handleMapBrowserEvent(event); - clock.tick(50); - // default timeout is 80 ms, not called yet - expect(interaction.handleWheelZoom_.called).to.be(false); + clock.tick(100); + // default timeout is 400 ms, not called yet + expect(interaction.endInteraction_.called).to.be(false); - clock.tick(30); - expect(interaction.handleWheelZoom_.called).to.be(true); + clock.tick(300); + expect(interaction.endInteraction_.called).to.be(true); done(); }); @@ -104,55 +104,6 @@ describe('ol.interaction.MouseWheelZoom', function() { }); } - describe('spying on view.animateInternal()', function() { - let view; - beforeEach(function() { - view = map.getView(); - sinon.spy(view, 'animateInternal'); - }); - - afterEach(function() { - view.animateInternal.restore(); - }); - - it('works in DOM_DELTA_LINE mode (wheel)', function(done) { - map.once('postrender', function() { - const call = view.animateInternal.getCall(0); - expect(call.args[0].resolution).to.be(2); - expect(call.args[0].anchor).to.eql([0, 0]); - done(); - }); - - const event = new MapBrowserEvent('wheel', map, { - type: 'wheel', - deltaMode: WheelEvent.DOM_DELTA_LINE, - deltaY: 3.714599609375, - target: map.getViewport(), - preventDefault: Event.prototype.preventDefault - }); - event.coordinate = [0, 0]; - - map.handleMapBrowserEvent(event); - }); - - it('works on all browsers (wheel)', function(done) { - map.once('postrender', function() { - const call = view.animateInternal.getCall(0); - expect(call.args[0].resolution).to.be(2); - expect(call.args[0].anchor).to.eql([0, 0]); - done(); - }); - - const event = new MapBrowserEvent('wheel', map, { - type: 'wheel', - deltaY: 120, - target: map.getViewport(), - preventDefault: Event.prototype.preventDefault - }); - event.coordinate = [0, 0]; - - map.handleMapBrowserEvent(event); - }); it('works in DOM_DELTA_LINE mode (wheel)', function(done) { map.once('postrender', function() { @@ -173,45 +124,42 @@ describe('ol.interaction.MouseWheelZoom', function() { map.handleMapBrowserEvent(event); }); - if (SAFARI) { - it('works on Safari (wheel)', function(done) { - map.once('postrender', function() { - expect(view.getResolution()).to.be(2); - expect(view.getCenter()).to.eql([0, 0]); - done(); - }); - - const event = new MapBrowserEvent('mousewheel', map, { - type: 'mousewheel', - wheelDeltaY: -900, - target: map.getViewport(), - preventDefault: Event.prototype.preventDefault - }); - event.coordinate = [0, 0]; - - map.handleMapBrowserEvent(event); + it('works on all browsers (wheel)', function(done) { + map.once('postrender', function() { + expect(view.getResolution()).to.be(2); + expect(view.getCenter()).to.eql([0, 0]); + done(); }); - } - if (!SAFARI) { - it('works on other browsers (wheel)', function(done) { - map.once('postrender', function() { - expect(view.getResolution()).to.be(2); - expect(view.getCenter()).to.eql([0, 0]); - done(); - }); - - const event = new MapBrowserEvent('mousewheel', map, { - type: 'mousewheel', - wheelDeltaY: -300, - target: map.getViewport(), - preventDefault: Event.prototype.preventDefault - }); - event.coordinate = [0, 0]; - - map.handleMapBrowserEvent(event); + const event = new MapBrowserEvent('wheel', map, { + type: 'wheel', + deltaY: 300, // trackpadDeltaPerZoom_ + target: map.getViewport(), + preventDefault: Event.prototype.preventDefault }); - } + event.coordinate = [0, 0]; + + map.handleMapBrowserEvent(event); + }); + + it('works in DOM_DELTA_LINE mode (wheel)', function(done) { + map.once('postrender', function() { + expect(view.getResolution()).to.be(2); + expect(view.getCenter()).to.eql([0, 0]); + done(); + }); + + const event = new MapBrowserEvent('wheel', map, { + type: 'wheel', + deltaMode: WheelEvent.DOM_DELTA_LINE, + deltaY: 7.5, // trackpadDeltaPerZoom_ / 40 + target: map.getViewport(), + preventDefault: Event.prototype.preventDefault + }); + event.coordinate = [0, 0]; + + map.handleMapBrowserEvent(event); + }); });