From 399ac618c767d47f035d75a59d22b6db93459f10 Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Thu, 21 Apr 2022 18:56:11 +0200
Subject: [PATCH] LLS: fix jumpy maximised map (#8387)

* add maxzoom to map fit bounds

Signed-off-by: Kerry Archibald <kerrya@element.io>

* take snapshot of bounds at center on dialog open

Signed-off-by: Kerry Archibald <kerrya@element.io>
---
 .../views/beacon/BeaconViewDialog.tsx         | 18 ++++++++---
 src/components/views/location/Map.tsx         |  2 +-
 .../views/beacon/BeaconViewDialog-test.tsx    | 30 +++++++++++++++++++
 test/components/views/location/Map-test.tsx   |  2 +-
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/src/components/views/beacon/BeaconViewDialog.tsx b/src/components/views/beacon/BeaconViewDialog.tsx
index 9dc1352f105..e6c4a423fe9 100644
--- a/src/components/views/beacon/BeaconViewDialog.tsx
+++ b/src/components/views/beacon/BeaconViewDialog.tsx
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { useState } from 'react';
+import React, { useState, useRef } from 'react';
 import { MatrixClient } from 'matrix-js-sdk/src/client';
 import {
     Beacon,
@@ -56,6 +56,17 @@ const getBoundsCenter = (bounds: Bounds): string | undefined => {
     });
 };
 
+const useInitialMapPosition = (liveBeacons: Beacon[], focusBeacon?: Beacon): {
+    bounds?: Bounds; centerGeoUri: string;
+} => {
+    const bounds = useRef<Bounds | undefined>(getBeaconBounds(liveBeacons));
+    const centerGeoUri = useRef<string>(
+        focusBeacon?.latestLocationState?.uri ||
+        getBoundsCenter(bounds.current),
+    );
+    return { bounds: bounds.current, centerGeoUri: centerGeoUri.current };
+};
+
 /**
  * Dialog to view live beacons maximised
  */
@@ -69,8 +80,7 @@ const BeaconViewDialog: React.FC<IProps> = ({
 
     const [isSidebarOpen, setSidebarOpen] = useState(false);
 
-    const bounds = getBeaconBounds(liveBeacons);
-    const centerGeoUri = focusBeacon?.latestLocationState?.uri || getBoundsCenter(bounds);
+    const { bounds, centerGeoUri } = useInitialMapPosition(liveBeacons, focusBeacon);
 
     return (
         <BaseDialog
@@ -79,7 +89,7 @@ const BeaconViewDialog: React.FC<IProps> = ({
             fixedWidth={false}
         >
             <MatrixClientContext.Provider value={matrixClient}>
-                { !!bounds ? <Map
+                { !!liveBeacons?.length ? <Map
                     id='mx_BeaconViewDialog'
                     bounds={bounds}
                     centerGeoUri={centerGeoUri}
diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx
index fc3bfab3eb7..b3e109f4538 100644
--- a/src/components/views/location/Map.tsx
+++ b/src/components/views/location/Map.tsx
@@ -64,7 +64,7 @@ const useMapWithStyle = ({ id, centerGeoUri, onError, interactive, bounds }) =>
                     [bounds.west, bounds.south],
                     [bounds.east, bounds.north],
                 );
-                map.fitBounds(lngLatBounds, { padding: 100 });
+                map.fitBounds(lngLatBounds, { padding: 100, maxZoom: 15 });
             } catch (error) {
                 logger.error('Invalid map bounds', error);
             }
diff --git a/test/components/views/beacon/BeaconViewDialog-test.tsx b/test/components/views/beacon/BeaconViewDialog-test.tsx
index 0f23036b8b1..8d0fb30e307 100644
--- a/test/components/views/beacon/BeaconViewDialog-test.tsx
+++ b/test/components/views/beacon/BeaconViewDialog-test.tsx
@@ -24,6 +24,7 @@ import {
     RoomMember,
     getBeaconInfoIdentifier,
 } from 'matrix-js-sdk/src/matrix';
+import maplibregl from 'maplibre-gl';
 
 import BeaconViewDialog from '../../../../src/components/views/beacon/BeaconViewDialog';
 import {
@@ -58,6 +59,8 @@ describe('<BeaconViewDialog />', () => {
         getVisibleRooms: jest.fn().mockReturnValue([]),
     });
 
+    const mockMap = new maplibregl.Map();
+
     // make fresh rooms every time
     // as we update room state
     const setupRoom = (stateEvents: MatrixEvent[] = []): Room => {
@@ -88,6 +91,8 @@ describe('<BeaconViewDialog />', () => {
 
     beforeEach(() => {
         jest.spyOn(OwnBeaconStore.instance, 'getLiveBeaconIds').mockRestore();
+
+        jest.clearAllMocks();
     });
 
     it('renders a map with markers', () => {
@@ -151,6 +156,31 @@ describe('<BeaconViewDialog />', () => {
         expect(component.find('BeaconMarker').length).toEqual(2);
     });
 
+    it('does not update bounds or center on changing beacons', () => {
+        const room = setupRoom([defaultEvent]);
+        const beacon = room.currentState.beacons.get(getBeaconInfoIdentifier(defaultEvent));
+        beacon.addLocations([location1]);
+        const component = getComponent();
+        expect(component.find('BeaconMarker').length).toEqual(1);
+
+        const anotherBeaconEvent = makeBeaconInfoEvent(bobId,
+            roomId,
+            { isLive: true },
+            '$bob-room1-1',
+        );
+
+        act(() => {
+            // emits RoomStateEvent.BeaconLiveness
+            room.currentState.setStateEvents([anotherBeaconEvent]);
+        });
+
+        component.setProps({});
+
+        // two markers now!
+        expect(mockMap.setCenter).toHaveBeenCalledTimes(1);
+        expect(mockMap.fitBounds).toHaveBeenCalledTimes(1);
+    });
+
     it('renders a fallback when no live beacons remain', () => {
         const onFinished = jest.fn();
         const room = setupRoom([defaultEvent]);
diff --git a/test/components/views/location/Map-test.tsx b/test/components/views/location/Map-test.tsx
index a1e1680b18a..72826a9cdf2 100644
--- a/test/components/views/location/Map-test.tsx
+++ b/test/components/views/location/Map-test.tsx
@@ -125,7 +125,7 @@ describe('<Map />', () => {
             const bounds = { north: 51, south: 50, east: 42, west: 41 };
             getComponent({ bounds });
             expect(mockMap.fitBounds).toHaveBeenCalledWith(new maplibregl.LngLatBounds([bounds.west, bounds.south],
-                [bounds.east, bounds.north]), { padding: 100 });
+                [bounds.east, bounds.north]), { padding: 100, maxZoom: 15 });
         });
 
         it('handles invalid bounds', () => {