Fix ScrollZoom handler setting tr.zoom = NaN (#6924)

* Add failing test for #6782 * Fix ScrollZoom handler setting tr.zoom = NaN Closes #6782 Closes #6486 Replaces #6921
mapbox · Jul 6, 2018 · 2a66237 · 2a66237
1 parent 01f0857
commit 2a66237
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 9 deletions.
diff --git 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
diff --git 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());