From 94385169f1470d4f7c022a22becda66f672a97d6 Mon Sep 17 00:00:00 2001 From: Kerry Date: Mon, 11 Apr 2022 10:29:24 +0200 Subject: [PATCH] Live location sharing - smart location marker (#8232) * extract location markers into generic Marker Signed-off-by: Kerry Archibald * wrap marker in smartmarker Signed-off-by: Kerry Archibald * test smartmarker Signed-off-by: Kerry Archibald * remove skinned-sdk Signed-off-by: Kerry Archibald * lint Signed-off-by: Kerry Archibald * better types for LocationBodyContent Signed-off-by: Kerry Archibald --- __mocks__/maplibre-gl.js | 3 +- .../views/location/LocationViewDialog.tsx | 4 +- src/components/views/location/Marker.tsx | 5 +- src/components/views/location/SmartMarker.tsx | 83 +++++++++++++++++++ .../views/messages/MLocationBody.tsx | 32 ++++--- src/utils/location/map.ts | 62 ++++++++++---- .../views/location/SmartMarker-test.tsx | 80 ++++++++++++++++++ .../__snapshots__/Marker-test.tsx.snap | 4 +- .../__snapshots__/SmartMarker-test.tsx.snap | 61 ++++++++++++++ 9 files changed, 294 insertions(+), 40 deletions(-) create mode 100644 src/components/views/location/SmartMarker.tsx create mode 100644 test/components/views/location/SmartMarker-test.tsx create mode 100644 test/components/views/location/__snapshots__/SmartMarker-test.tsx.snap diff --git a/__mocks__/maplibre-gl.js b/__mocks__/maplibre-gl.js index 716db4b665..687f769a70 100644 --- a/__mocks__/maplibre-gl.js +++ b/__mocks__/maplibre-gl.js @@ -16,10 +16,11 @@ const MockGeolocateInstance = new MockGeolocateControl(); const MockMarker = {} MockMarker.setLngLat = jest.fn().mockReturnValue(MockMarker); MockMarker.addTo = jest.fn().mockReturnValue(MockMarker); +MockMarker.remove = jest.fn().mockReturnValue(MockMarker); module.exports = { Map: jest.fn().mockReturnValue(MockMapInstance), GeolocateControl: jest.fn().mockReturnValue(MockGeolocateInstance), Marker: jest.fn().mockReturnValue(MockMarker), LngLat, - NavigationControl + NavigationControl, }; diff --git a/src/components/views/location/LocationViewDialog.tsx b/src/components/views/location/LocationViewDialog.tsx index 236bd754d9..a3363e8f22 100644 --- a/src/components/views/location/LocationViewDialog.tsx +++ b/src/components/views/location/LocationViewDialog.tsx @@ -22,7 +22,7 @@ import BaseDialog from "../dialogs/BaseDialog"; import { IDialogProps } from "../dialogs/IDialogProps"; import { LocationBodyContent } from '../messages/MLocationBody'; import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; -import { parseGeoUri, locationEventGeoUri, createMap } from '../../../utils/location'; +import { parseGeoUri, locationEventGeoUri, createMapWithCoords } from '../../../utils/location'; interface IProps extends IDialogProps { matrixClient: MatrixClient; @@ -54,7 +54,7 @@ export default class LocationViewDialog extends React.Component this.props.matrixClient.on(ClientEvent.ClientWellKnown, this.updateStyleUrl); - this.map = createMap( + this.map = createMapWithCoords( this.coords, true, this.getBodyId(), diff --git a/src/components/views/location/Marker.tsx b/src/components/views/location/Marker.tsx index 7978e0d533..bacade71cf 100644 --- a/src/components/views/location/Marker.tsx +++ b/src/components/views/location/Marker.tsx @@ -33,9 +33,10 @@ interface Props { /** * Generic location marker */ -const Marker: React.FC = ({ id, roomMember, useMemberColor }) => { +const Marker = React.forwardRef(({ id, roomMember, useMemberColor }, ref) => { const memberColorClass = useMemberColor && roomMember ? getUserNameColorClass(roomMember.userId) : ''; return
= ({ id, roomMember, useMemberColor }) => { }
; -}; +}); export default Marker; diff --git a/src/components/views/location/SmartMarker.tsx b/src/components/views/location/SmartMarker.tsx new file mode 100644 index 0000000000..29207eb6e3 --- /dev/null +++ b/src/components/views/location/SmartMarker.tsx @@ -0,0 +1,83 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React, { useCallback, useEffect, useState } from 'react'; +import maplibregl from 'maplibre-gl'; +import { RoomMember } from 'matrix-js-sdk/src/matrix'; + +import { createMarker, parseGeoUri } from '../../../utils/location'; +import Marker from './Marker'; + +const useMapMarker = ( + map: maplibregl.Map, + geoUri: string, +): { marker?: maplibregl.Marker, onElementRef: (el: HTMLDivElement) => void } => { + const [marker, setMarker] = useState(); + + const onElementRef = useCallback((element: HTMLDivElement) => { + if (marker || !element) { + return; + } + const coords = parseGeoUri(geoUri); + const newMarker = createMarker(coords, element); + newMarker.addTo(map); + setMarker(newMarker); + }, [marker, geoUri, map]); + + useEffect(() => { + if (marker) { + const coords = parseGeoUri(geoUri); + marker.setLngLat({ lon: coords.longitude, lat: coords.latitude }); + } + }, [marker, geoUri]); + + useEffect(() => () => { + if (marker) { + marker.remove(); + } + }, [marker]); + + return { + marker, + onElementRef, + }; +}; + +interface SmartMarkerProps { + map: maplibregl.Map; + geoUri: string; + id?: string; + // renders MemberAvatar when provided + roomMember?: RoomMember; + // use member text color as background + useMemberColor?: boolean; +} + +/** + * Generic location marker + */ +const SmartMarker: React.FC = ({ id, map, geoUri, roomMember, useMemberColor }) => { + const { onElementRef } = useMapMarker(map, geoUri); + + return ; +}; + +export default SmartMarker; diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index 3a447c7944..94e27def21 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -30,7 +30,7 @@ import Modal from '../../../Modal'; import { parseGeoUri, locationEventGeoUri, - createMap, + createMapWithCoords, getLocationShareErrorMessage, LocationShareError, } from '../../../utils/location'; @@ -75,7 +75,7 @@ export default class MLocationBody extends React.Component { this.context.on(ClientEvent.ClientWellKnown, this.updateStyleUrl); - this.map = createMap( + this.map = createMapWithCoords( this.coords, false, this.bodyId, @@ -138,18 +138,6 @@ export function isSelfLocation(locationContent: ILocationContent): boolean { return assetType == LocationAssetType.Self; } -interface ILocationBodyContentProps { - mxEvent: MatrixEvent; - bodyId: string; - markerId: string; - error: Error; - tooltip?: string; - onClick?: (event: React.MouseEvent) => void; - zoomButtons?: boolean; - onZoomIn?: () => void; - onZoomOut?: () => void; -} - export const LocationBodyFallbackContent: React.FC<{ event: MatrixEvent, error: Error }> = ({ error, event }) => { const errorType = error?.message as LocationShareError; const message = `${_t('Unable to load map')}: ${getLocationShareErrorMessage(errorType)}`; @@ -167,8 +155,18 @@ export const LocationBodyFallbackContent: React.FC<{ event: MatrixEvent, error: ; }; -export function LocationBodyContent(props: ILocationBodyContentProps): - React.ReactElement { +interface LocationBodyContentProps { + mxEvent: MatrixEvent; + bodyId: string; + markerId: string; + error: Error; + tooltip?: string; + onClick?: (event: React.MouseEvent) => void; + zoomButtons?: boolean; + onZoomIn?: () => void; + onZoomOut?: () => void; +} +export const LocationBodyContent: React.FC = (props) => { const mapDiv =
; -} +}; interface IZoomButtonsProps { onZoomIn: () => void; diff --git a/src/utils/location/map.ts b/src/utils/location/map.ts index f1e87acc28..5627bf5c0e 100644 --- a/src/utils/location/map.ts +++ b/src/utils/location/map.ts @@ -24,6 +24,46 @@ import { findMapStyleUrl } from "./findMapStyleUrl"; import { LocationShareError } from "./LocationShareErrors"; export const createMap = ( + interactive: boolean, + bodyId: string, + onError: (error: Error) => void, +): maplibregl.Map => { + try { + const styleUrl = findMapStyleUrl(); + + const map = new maplibregl.Map({ + container: bodyId, + style: styleUrl, + zoom: 15, + interactive, + }); + + map.on('error', (e) => { + logger.error( + "Failed to load map: check map_style_url in config.json has a " + + "valid URL and API key", + e.error, + ); + onError(new Error(LocationShareError.MapStyleUrlNotReachable)); + }); + + return map; + } catch (e) { + logger.error("Failed to render map", e); + throw e; + } +}; + +export const createMarker = (coords: GeolocationCoordinates, element: HTMLElement): maplibregl.Marker => { + const marker = new maplibregl.Marker({ + element, + anchor: 'bottom', + offset: [0, -1], + }).setLngLat({ lon: coords.longitude, lat: coords.latitude }); + return marker; +}; + +export const createMapWithCoords = ( coords: GeolocationCoordinates, interactive: boolean, bodyId: string, @@ -31,24 +71,14 @@ export const createMap = ( onError: (error: Error) => void, ): maplibregl.Map => { try { - const styleUrl = findMapStyleUrl(); + const map = createMap(interactive, bodyId, onError); + const coordinates = new maplibregl.LngLat(coords.longitude, coords.latitude); + // center on coordinates + map.setCenter(coordinates); - const map = new maplibregl.Map({ - container: bodyId, - style: styleUrl, - center: coordinates, - zoom: 15, - interactive, - }); - - new maplibregl.Marker({ - element: document.getElementById(markerId), - anchor: 'bottom', - offset: [0, -1], - }) - .setLngLat(coordinates) - .addTo(map); + const marker = createMarker(coords, document.getElementById(markerId)); + marker.addTo(map); map.on('error', (e) => { logger.error( diff --git a/test/components/views/location/SmartMarker-test.tsx b/test/components/views/location/SmartMarker-test.tsx new file mode 100644 index 0000000000..5fc48085b4 --- /dev/null +++ b/test/components/views/location/SmartMarker-test.tsx @@ -0,0 +1,80 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from 'react'; +import { mount } from 'enzyme'; +import { mocked } from 'jest-mock'; +import maplibregl from 'maplibre-gl'; + +import SmartMarker from '../../../../src/components/views/location/SmartMarker'; + +jest.mock('../../../../src/utils/location/findMapStyleUrl', () => ({ + findMapStyleUrl: jest.fn().mockReturnValue('tileserver.com'), +})); + +describe('', () => { + const mockMap = new maplibregl.Map(); + const mockMarker = new maplibregl.Marker(); + + const defaultProps = { + map: mockMap, + geoUri: 'geo:43.2,54.6', + }; + const getComponent = (props = {}) => + mount(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('creates a marker on mount', () => { + const component = getComponent(); + expect(component).toMatchSnapshot(); + + component.setProps({}); + + // marker added only once + expect(maplibregl.Marker).toHaveBeenCalledTimes(1); + // set to correct position + expect(mockMarker.setLngLat).toHaveBeenCalledWith({ lon: 54.6, lat: 43.2 }); + // added to map + expect(mockMarker.addTo).toHaveBeenCalledWith(mockMap); + }); + + it('updates marker position on change', () => { + const component = getComponent({ geoUri: 'geo:40,50' }); + + component.setProps({ geoUri: 'geo:41,51' }); + component.setProps({ geoUri: 'geo:42,52' }); + + // marker added only once + expect(maplibregl.Marker).toHaveBeenCalledTimes(1); + // set positions + expect(mocked(mockMarker.setLngLat)).toHaveBeenCalledWith({ lat: 40, lon: 50 }); + expect(mocked(mockMarker.setLngLat)).toHaveBeenCalledWith({ lat: 41, lon: 51 }); + expect(mocked(mockMarker.setLngLat)).toHaveBeenCalledWith({ lat: 42, lon: 52 }); + }); + + it('removes marker on unmount', () => { + const component = getComponent(); + expect(component).toMatchSnapshot(); + + component.setProps({}); + + component.unmount(); + expect(mockMarker.remove).toHaveBeenCalled(); + }); +}); diff --git a/test/components/views/location/__snapshots__/Marker-test.tsx.snap b/test/components/views/location/__snapshots__/Marker-test.tsx.snap index 8030f6448e..b7596d1af8 100644 --- a/test/components/views/location/__snapshots__/Marker-test.tsx.snap +++ b/test/components/views/location/__snapshots__/Marker-test.tsx.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[` renders with location icon when no room member 1`] = ` -
renders with location icon when no room member 1`] = ` />
- + `; diff --git a/test/components/views/location/__snapshots__/SmartMarker-test.tsx.snap b/test/components/views/location/__snapshots__/SmartMarker-test.tsx.snap new file mode 100644 index 0000000000..5b6bcf6a95 --- /dev/null +++ b/test/components/views/location/__snapshots__/SmartMarker-test.tsx.snap @@ -0,0 +1,61 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` creates a marker on mount 1`] = ` + + +
+
+
+
+
+ + +`; + +exports[` removes marker on unmount 1`] = ` + + +
+
+
+
+
+ + +`;