From 2a662377d4a0c63e376ed2c8402b7216ebacf6c9 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Fri, 6 Jul 2018 14:30:40 -0400 Subject: [PATCH] Fix ScrollZoom handler setting tr.zoom = NaN (#6924) * Add failing test for https://github.com/mapbox/mapbox-gl-js/issues/6782 * Fix ScrollZoom handler setting tr.zoom = NaN Closes #6782 Closes #6486 Replaces #6921 --- src/ui/handler/scroll_zoom.js | 24 ++++++++++++++++-------- test/node_modules/mapbox-gl-js-test.js | 1 + test/unit/ui/handler/scroll_zoom.test.js | 20 +++++++++++++++++++- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/ui/handler/scroll_zoom.js b/src/ui/handler/scroll_zoom.js index e02d1159c28..fb06392245c 100644 --- a/src/ui/handler/scroll_zoom.js +++ b/src/ui/handler/scroll_zoom.js @@ -1,5 +1,6 @@ // @flow +import assert from 'assert'; import DOM from '../../util/dom'; import { ease as _ease, bindAll, bezier } from '../../util/util'; @@ -43,11 +44,11 @@ class ScrollZoomHandler { _lastWheelEvent: any; _lastWheelEventTime: number; - _startZoom: number; - _targetZoom: number; + _startZoom: ?number; + _targetZoom: ?number; _delta: number; - _easing: (number) => number; - _prevEase: {start: number, duration: number, easing: (number) => number}; + _easing: ?((number) => number); + _prevEase: ?{start: number, duration: number, easing: (number) => number}; _frameId: ?TaskID; @@ -228,11 +229,18 @@ class ScrollZoomHandler { this._delta = 0; } + const targetZoom = typeof this._targetZoom === 'number' ? + this._targetZoom : tr.zoom; + const startZoom = this._startZoom; + const easing = this._easing; + let finished = false; - if (this._type === 'wheel') { + if (this._type === 'wheel' && startZoom && easing) { + assert(easing && typeof startZoom === 'number'); + const t = Math.min((browser.now() - this._lastWheelEventTime) / 200, 1); - const k = this._easing(t); - tr.zoom = interpolate(this._startZoom, this._targetZoom, k); + const k = easing(t); + tr.zoom = interpolate(startZoom, targetZoom, k); if (t < 1) { if (!this._frameId) { this._frameId = this._map._requestRenderFrame(this._onScrollFrame); @@ -241,7 +249,7 @@ class ScrollZoomHandler { finished = true; } } else { - tr.zoom = this._targetZoom; + tr.zoom = targetZoom; finished = true; } diff --git a/test/node_modules/mapbox-gl-js-test.js b/test/node_modules/mapbox-gl-js-test.js index f923c19f2ef..ea6de53f1da 100644 --- a/test/node_modules/mapbox-gl-js-test.js +++ b/test/node_modules/mapbox-gl-js-test.js @@ -2,6 +2,7 @@ import tap from 'tap'; import sinon from 'sinon'; export const test = tap.test; +export const only = tap.only; tap.Test.prototype.addAssert('equalWithPrecision', 3, assertEqualWithPrecision); diff --git a/test/unit/ui/handler/scroll_zoom.test.js b/test/unit/ui/handler/scroll_zoom.test.js index cfad0edd770..6cecaf7551f 100644 --- a/test/unit/ui/handler/scroll_zoom.test.js +++ b/test/unit/ui/handler/scroll_zoom.test.js @@ -113,7 +113,25 @@ test('ScrollZoomHandler', (t) => { t.end(); }); - test('does not zoom if preventDefault is called on the wheel event', (t) => { + t.test('Gracefully handle wheel events that cancel each other out before the first scroll frame', (t) => { + // See also https://github.com/mapbox/mapbox-gl-js/issues/6782 + const map = createMap(t); + map._renderTaskQueue.run(); + + simulate.wheel(map.getCanvas(), {type: 'wheel', deltaY: -1}); + simulate.wheel(map.getCanvas(), {type: 'wheel', deltaY: -1}); + now += 1; + simulate.wheel(map.getCanvas(), {type: 'wheel', deltaY: 2}); + + map._renderTaskQueue.run(); + + now += 400; + map._renderTaskQueue.run(); + + t.end(); + }); + + t.test('does not zoom if preventDefault is called on the wheel event', (t) => { const map = createMap(t); map.on('wheel', e => e.preventDefault());