Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
LLS: fix jumpy maximised map (#8387)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
Kerry authored Apr 21, 2022
1 parent 86419b1 commit 399ac61
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
18 changes: 14 additions & 4 deletions src/components/views/beacon/BeaconViewDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
*/
Expand All @@ -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
Expand All @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/location/Map.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
30 changes: 30 additions & 0 deletions test/components/views/beacon/BeaconViewDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -88,6 +91,8 @@ describe('<BeaconViewDialog />', () => {

beforeEach(() => {
jest.spyOn(OwnBeaconStore.instance, 'getLiveBeaconIds').mockRestore();

jest.clearAllMocks();
});

it('renders a map with markers', () => {
Expand Down Expand Up @@ -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]);
Expand Down
2 changes: 1 addition & 1 deletion test/components/views/location/Map-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 399ac61

Please sign in to comment.