fix(maps): leave zoom in place if the bounds did not change (#3050)

* fix map-zoom after browser-zoom

* use .equals and add tests

Co-authored-by: Haroen Viaene <hello@haroen.me>

Author: anikolskiy

packages/react-instantsearch-dom-maps/src/GoogleMaps.js

@@ -67,17 +67,21 @@ class GoogleMaps extends Component {
     }
 
     if (boundingBox) {
-      this.lockUserInteration(() => {
-        this.instance.fitBounds(
-          new google.maps.LatLngBounds(
-            boundingBox.southWest,
-            boundingBox.northEast
-          ),
-          boundingBoxPadding
+      this.lockUserInteraction(() => {
+        const oldBounds = this.instance.getBounds();
+
+        // south west and north east are swapped
+        const newBounds = new google.maps.LatLngBounds(
+          boundingBox.southWest,
+          boundingBox.northEast
         );
+
+        if (!newBounds.equals(oldBounds)) {
+          this.instance.fitBounds(newBounds, boundingBoxPadding);
+        }
       });
     } else {
-      this.lockUserInteration(() => {
+      this.lockUserInteraction(() => {
         this.instance.setZoom(initialZoom);
         this.instance.setCenter(initialPosition);
       });
@@ -124,7 +128,7 @@ class GoogleMaps extends Component {
     );
   };
 
-  lockUserInteration(functionThatAltersTheMapPosition) {
+  lockUserInteraction(functionThatAltersTheMapPosition) {
     this.isUserInteraction = false;
     functionThatAltersTheMapPosition();
     this.isUserInteraction = true;

packages/react-instantsearch-dom-maps/src/__tests__/GoogleMaps.js

@@ -358,11 +358,6 @@ describe('GoogleMaps', () => {
         mapInstance,
       });
 
-      google.maps.LatLngBounds.mockImplementation((sw, ne) => ({
-        northEast: ne,
-        southWest: sw,
-      }));
-
       const props = {
         ...defaultProps,
         google,
@@ -384,6 +379,20 @@ describe('GoogleMaps', () => {
       wrapper.setProps({
         boundingBoxPadding: 0,
         boundingBox: {
+          northEast: {
+            lat: 14,
+            lng: 14,
+          },
+          southWest: {
+            lat: 10,
+            lng: 10,
+          },
+        },
+      });
+
+      expect(mapInstance.fitBounds).toHaveBeenCalledTimes(1);
+      expect(mapInstance.fitBounds).toHaveBeenCalledWith(
+        expect.objectContaining({
           northEast: {
             lat: 10,
             lng: 10,
@@ -392,12 +401,41 @@ describe('GoogleMaps', () => {
             lat: 14,
             lng: 14,
           },
-        },
+        }),
+        0
+      );
+
+      expect(mapInstance.setZoom).toHaveBeenCalledTimes(1); // cDM
+      expect(mapInstance.setCenter).toHaveBeenCalledTimes(1); // cDM
+    });
+
+    it('expect not to call fitBounds on didUpdate when boundingBox equal to previous', () => {
+      const mapInstance = createFakeMapInstance();
+      const google = createFakeGoogleReference({
+        mapInstance,
       });
 
-      expect(mapInstance.fitBounds).toHaveBeenCalledTimes(1);
-      expect(mapInstance.fitBounds).toHaveBeenCalledWith(
-        {
+      const props = {
+        ...defaultProps,
+        google,
+      };
+
+      const wrapper = shallow(
+        <GoogleMaps {...props}>
+          <div>This is the children</div>
+        </GoogleMaps>
+      );
+
+      simulateMapReadyEvent(google);
+
+      expect(mapInstance.fitBounds).toHaveBeenCalledTimes(0);
+
+      expect(mapInstance.setZoom).toHaveBeenCalledTimes(1); // cDM
+      expect(mapInstance.setCenter).toHaveBeenCalledTimes(1); // cDM
+
+      wrapper.setProps({
+        boundingBoxPadding: 0,
+        boundingBox: {
           northEast: {
             lat: 10,
             lng: 10,
@@ -407,11 +445,49 @@ describe('GoogleMaps', () => {
             lng: 14,
           },
         },
+      });
+
+      expect(mapInstance.fitBounds).toHaveBeenCalledTimes(1);
+      expect(mapInstance.fitBounds).toHaveBeenCalledWith(
+        expect.objectContaining({
+          northEast: {
+            lat: 14,
+            lng: 14,
+          },
+          southWest: {
+            lat: 10,
+            lng: 10,
+          },
+        }),
         0
       );
 
       expect(mapInstance.setZoom).toHaveBeenCalledTimes(1); // cDM
       expect(mapInstance.setCenter).toHaveBeenCalledTimes(1); // cDM
+
+      mapInstance.getBounds.mockImplementation(
+        () =>
+          new google.maps.LatLngBounds(
+            { lat: 14, lng: 14 },
+            { lat: 10, lng: 10 }
+          )
+      );
+
+      wrapper.setProps({
+        boundingBoxPadding: 0,
+        boundingBox: {
+          northEast: {
+            lat: 10,
+            lng: 10,
+          },
+          southWest: {
+            lat: 14,
+            lng: 14,
+          },
+        },
+      });
+
+      expect(mapInstance.fitBounds).toHaveBeenCalledTimes(1);
     });
 
     it('expect to call setCenter & setZoom when boundingBox is not provided', () => {

packages/react-instantsearch-dom-maps/test/mockGoogleMaps.js

@@ -15,6 +15,22 @@ export class FakeOverlayView {
   }));
 }
 
+export const MockLatLngBounds = jest.fn((ne, sw) => ({
+  northEast: ne,
+  southWest: sw,
+  equals(oldBounds) {
+    if (!oldBounds) {
+      return false;
+    }
+    return (
+      oldBounds.northEast.lat === this.northEast.lat &&
+      oldBounds.northEast.lng === this.northEast.lng &&
+      oldBounds.southWest.lat === this.southWest.lat &&
+      oldBounds.southWest.lng === this.southWest.lng
+    );
+  },
+}));
+
 export const createFakeMapInstance = () => ({
   addListener: jest.fn(() => ({
     remove: jest.fn(),
@@ -23,10 +39,9 @@ export const createFakeMapInstance = () => ({
   setCenter: jest.fn(),
   getZoom: jest.fn(),
   setZoom: jest.fn(),
-  getBounds: jest.fn(() => ({
-    getNorthEast: jest.fn(),
-    getSouthWest: jest.fn(),
-  })),
+  getBounds: jest.fn(
+    () => new MockLatLngBounds({ lat: 0, lng: 0 }, { lat: 0, lng: 0 })
+  ),
   getProjection: jest.fn(() => ({
     fromPointToLatLng: jest.fn(() => ({
       lat: jest.fn(),
@@ -58,9 +73,7 @@ export const createFakeGoogleReference = ({
 } = {}) => ({
   maps: {
     LatLng: jest.fn(x => x),
-    LatLngBounds: jest.fn(() => ({
-      extend: jest.fn().mockReturnThis(),
-    })),
+    LatLngBounds: MockLatLngBounds,
     Map: jest.fn(() => mapInstance),
     Marker: jest.fn(() => markerInstance),
     ControlPosition: {