map.isMoving() match move events fix #9647 (#9679)

* map.isMoving() match move events fix #9647

Previously isMoving() would return true if any interaction handler was
active. Handlers are sometimes active even if they haven't changed the
map yet. This resulted in the isMoving() returning true when the map
hasn't moved.

This makes isMoving aligned with movestart/move/moveend events.

Since move events may be fired after several mouse events have been
batched, the camera changes a mouseevent will *later* cause won't be
reflected in isMoving() when that mouseevent is fired.

* lint

* fix test

Author: ansis

src/ui/handler_manager.js

@@ -259,6 +259,10 @@ class HandlerManager {
         return !!this._eventsInProgress.rotate;
     }
 
+    isMoving() {
+        return Boolean(isMoving(this._eventsInProgress)) || this.isZooming();
+    }
+
     _blockedByActive(activeHandlers: { [string]: Handler }, allowed: Array<string>, myName: string) {
         for (const name in activeHandlers) {
             if (name === myName) continue;
@@ -430,17 +434,23 @@ class HandlerManager {
         const wasMoving = isMoving(this._eventsInProgress);
         const nowMoving = isMoving(newEventsInProgress);
 
-        if (!wasMoving && nowMoving) {
-            this._fireEvent('movestart', nowMoving.originalEvent);
-        }
+        const startEvents = {};
 
         for (const eventName in newEventsInProgress) {
             const {originalEvent} = newEventsInProgress[eventName];
-            const isStart = !this._eventsInProgress[eventName];
-            this._eventsInProgress[eventName] = newEventsInProgress[eventName];
-            if (isStart) {
-                this._fireEvent(`${eventName}start`, originalEvent);
+            if (!this._eventsInProgress[eventName]) {
+                startEvents[`${eventName}start`] = originalEvent;
             }
+            this._eventsInProgress[eventName] = newEventsInProgress[eventName];
+        }
+
+        // fire start events only after this._eventsInProgress has been updated
+        if (!wasMoving && nowMoving) {
+            this._fireEvent('movestart', nowMoving.originalEvent);
+        }
+
+        for (const name in startEvents) {
+            this._fireEvent(name, startEvents[name]);
         }
 
         if (newEventsInProgress.rotate) this._bearingChanged = true;
@@ -454,16 +464,22 @@ class HandlerManager {
             this._fireEvent(eventName, originalEvent);
         }
 
+        const endEvents = {};
+
         let originalEndEvent;
         for (const eventName in this._eventsInProgress) {
             const {handlerName, originalEvent} = this._eventsInProgress[eventName];
             if (!this._handlersById[handlerName].isActive()) {
                 delete this._eventsInProgress[eventName];
                 originalEndEvent = deactivatedHandlers[handlerName] || originalEvent;
-                this._fireEvent(`${eventName}end`, originalEndEvent);
+                endEvents[`${eventName}end`] = originalEndEvent;
             }
         }
 
+        for (const name in endEvents) {
+            this._fireEvent(name, endEvents[name]);
+        }
+
         const stillMoving = isMoving(this._eventsInProgress);
         if ((wasMoving || nowMoving) && !stillMoving) {
             this._updatingCamera = true;

src/ui/map.js

@@ -853,7 +853,7 @@ class Map extends Camera {
      * var isMoving = map.isMoving();
      */
     isMoving(): boolean {
-        return this._moving || this.handlers.isActive();
+        return this._moving || this.handlers.isMoving();
     }
 
     /**

test/unit/ui/handler/drag_rotate.test.js

@@ -273,9 +273,11 @@ test('DragRotateHandler ensures that map.isMoving() returns true during drag', (
 
     simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2});
     simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 10, clientY: 10});
+    map._renderTaskQueue.run();
     t.ok(map.isMoving());
 
     simulate.mouseup(map.getCanvas(),   {buttons: 0, button: 2});
+    map._renderTaskQueue.run();
     t.ok(!map.isMoving());
 
     map.remove();

test/unit/ui/map/isMoving.test.js

@@ -36,12 +36,18 @@ test('Map#isMoving returns true during a camera zoom animation', (t) => {
 test('Map#isMoving returns true when drag panning', (t) => {
     const map = createMap(t);
 
+    map.on('movestart', () => {
+        t.equal(map.isMoving(), true);
+    });
     map.on('dragstart', () => {
         t.equal(map.isMoving(), true);
     });
 
     map.on('dragend', () => {
         t.equal(map.isMoving(), false);
+    });
+    map.on('moveend', () => {
+        t.equal(map.isMoving(), false);
         map.remove();
         t.end();
     });
@@ -62,12 +68,18 @@ test('Map#isMoving returns true when drag rotating', (t) => {
     // Prevent inertial rotation.
     t.stub(browser, 'now').returns(0);
 
+    map.on('movestart', () => {
+        t.equal(map.isMoving(), true);
+    });
     map.on('rotatestart', () => {
         t.equal(map.isMoving(), true);
     });
 
     map.on('rotateend', () => {
         t.equal(map.isMoving(), false);
+    });
+    map.on('moveend', () => {
+        t.equal(map.isMoving(), false);
         map.remove();
         t.end();
     });

test/unit/ui/map_events.test.js

@@ -601,3 +601,32 @@ test(`Map#on click fires subsequent click event if there is no corresponding mou
     map.remove();
     t.end();
 });
+
+test("Map#isMoving() returns false in mousedown/mouseup/click with no movement", (t) => {
+    const map = createMap(t, {interactive: true, clickTolerance: 4});
+    let mousedown, mouseup, click;
+    map.on('mousedown', () => { mousedown = map.isMoving(); });
+    map.on('mouseup', () => { mouseup = map.isMoving(); });
+    map.on('click', () => { click = map.isMoving(); });
+
+    const canvas = map.getCanvas();
+    const MouseEvent = window(canvas).MouseEvent;
+
+    canvas.dispatchEvent(new MouseEvent('mousedown', {bubbles: true, clientX: 100, clientY: 100}));
+    t.equal(mousedown, false);
+    map._renderTaskQueue.run();
+    t.equal(mousedown, false);
+
+    canvas.dispatchEvent(new MouseEvent('mouseup', {bubbles: true, clientX: 100, clientY: 100}));
+    t.equal(mouseup, false);
+    map._renderTaskQueue.run();
+    t.equal(mouseup, false);
+
+    canvas.dispatchEvent(new MouseEvent('click', {bubbles: true, clientX: 100, clientY: 100}));
+    t.equal(click, false);
+    map._renderTaskQueue.run();
+    t.equal(click, false);
+
+    map.remove();
+    t.end();
+});