From e5a7e83b486096ea31b882969974b347aadb390b Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Sat, 4 Feb 2023 19:58:19 +0100 Subject: [PATCH 01/13] Add option to find own location in map views --- .../views/beacon/BeaconViewDialog.tsx | 4 ++ .../views/location/LocationPicker.tsx | 21 +------ .../views/location/LocationViewDialog.tsx | 1 + src/components/views/location/Map.tsx | 55 +++++++++++++++++-- src/utils/location/index.ts | 1 + src/utils/location/positionFailureMessage.ts | 36 ++++++++++++ 6 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 src/utils/location/positionFailureMessage.ts diff --git a/src/components/views/beacon/BeaconViewDialog.tsx b/src/components/views/beacon/BeaconViewDialog.tsx index 17ba972ef9c..a2fa704e8e8 100644 --- a/src/components/views/beacon/BeaconViewDialog.tsx +++ b/src/components/views/beacon/BeaconViewDialog.tsx @@ -125,6 +125,9 @@ const BeaconViewDialog: React.FC = ({ initialFocusedBeacon, roomId, matr setFocusedBeaconState({ beacon, ts: Date.now() }); }; + const hasOwnBeacon = + liveBeacons.filter((beacon) => beacon?.beaconInfoOwner === matrixClient.getUserId()).length > 0; + return ( @@ -136,6 +139,7 @@ const BeaconViewDialog: React.FC = ({ initialFocusedBeacon, roomId, matr interactive onError={setMapDisplayError} className="mx_BeaconViewDialog_map" + allowGeolocate={!hasOwnBeacon} > {({ map }: { map: maplibregl.Map }) => ( <> diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 838fa619dae..57a65d04193 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -23,10 +23,9 @@ import { ClientEvent, IClientWellKnown } from "matrix-js-sdk/src/client"; import { _t } from "../../../languageHandler"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; import Modal from "../../../Modal"; -import SdkConfig from "../../../SdkConfig"; import { tileServerFromWellKnown } from "../../../utils/WellKnownUtils"; import { GenericPosition, genericPositionFromGeolocation, getGeoUri } from "../../../utils/beacon"; -import { LocationShareError, findMapStyleUrl } from "../../../utils/location"; +import { LocationShareError, findMapStyleUrl, positionFailureMessage } from "../../../utils/location"; import ErrorDialog from "../dialogs/ErrorDialog"; import AccessibleButton from "../elements/AccessibleButton"; import { MapError } from "./MapError"; @@ -266,21 +265,3 @@ class LocationPicker extends React.Component { } export default LocationPicker; - -function positionFailureMessage(code: number): string { - const brand = SdkConfig.get().brand; - switch (code) { - case 1: - return _t( - "%(brand)s was denied permission to fetch your location. " + - "Please allow location access in your browser settings.", - { brand }, - ); - case 2: - return _t("Failed to fetch your location. Please try again later."); - case 3: - return _t("Timed out trying to fetch your location. Please try again later."); - case 4: - return _t("Unknown error fetching location. Please try again later."); - } -} diff --git a/src/components/views/location/LocationViewDialog.tsx b/src/components/views/location/LocationViewDialog.tsx index 48a4b78e1be..3e706d3df25 100644 --- a/src/components/views/location/LocationViewDialog.tsx +++ b/src/components/views/location/LocationViewDialog.tsx @@ -68,6 +68,7 @@ export default class LocationViewDialog extends React.Component onError={this.onError} interactive className="mx_LocationViewDialog_map" + allowGeolocate={true} > {({ map }) => ( <> diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx index 87666f022b8..a6376039db5 100644 --- a/src/components/views/location/Map.tsx +++ b/src/components/views/location/Map.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { ReactNode, useContext, useEffect } from "react"; +import React, { ReactNode, useContext, useEffect, useState } from "react"; import classNames from "classnames"; import * as maplibregl from "maplibre-gl"; import { ClientEvent, IClientWellKnown } from "matrix-js-sdk/src/matrix"; @@ -22,10 +22,13 @@ import { logger } from "matrix-js-sdk/src/logger"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; import { useEventEmitterState } from "../../../hooks/useEventEmitter"; -import { parseGeoUri } from "../../../utils/location"; +import { parseGeoUri, positionFailureMessage } from "../../../utils/location"; import { tileServerFromWellKnown } from "../../../utils/WellKnownUtils"; import { useMap } from "../../../utils/location/useMap"; import { Bounds } from "../../../utils/beacon/bounds"; +import Modal from "../../../Modal"; +import ErrorDialog from "../dialogs/ErrorDialog"; +import { _t } from "../../../languageHandler"; const useMapWithStyle = ({ id, @@ -33,12 +36,14 @@ const useMapWithStyle = ({ onError, interactive, bounds, + allowGeolocate, }: { id: string; centerGeoUri?: string; + onError(error: Error): void; interactive?: boolean; bounds?: Bounds; - onError(error: Error): void; + allowGeolocate: boolean; }): { map: maplibregl.Map; bodyId: string; @@ -86,12 +91,41 @@ const useMapWithStyle = ({ } }, [map, bounds]); + const [geolocate] = useState( + allowGeolocate + ? new maplibregl.GeolocateControl({ + positionOptions: { + enableHighAccuracy: true, + }, + trackUserLocation: false, + }) + : null, + ); + + useEffect(() => { + if (map && geolocate) { + map.addControl(geolocate); + geolocate.on("error", onGeolocateError); + return () => { + geolocate.off("error", onGeolocateError); + }; + } + }, [map, geolocate]); + return { map, bodyId, }; }; +const onGeolocateError = (e: GeolocationPositionError): void => { + logger.error("Could not fetch location", e); + Modal.createDialog(ErrorDialog, { + title: _t("Could not fetch location"), + description: positionFailureMessage(e.code), + }); +}; + interface MapProps { id: string; interactive?: boolean; @@ -105,13 +139,24 @@ interface MapProps { centerGeoUri?: string; bounds?: Bounds; className?: string; + allowGeolocate?: boolean; onClick?: () => void; onError?: (error: Error) => void; children?: (renderProps: { map: maplibregl.Map }) => ReactNode; } -const Map: React.FC = ({ bounds, centerGeoUri, children, className, id, interactive, onError, onClick }) => { - const { map, bodyId } = useMapWithStyle({ centerGeoUri, onError, id, interactive, bounds }); +const Map: React.FC = ({ + bounds, + centerGeoUri, + children, + className, + allowGeolocate, + id, + interactive, + onError, + onClick, +}) => { + const { map, bodyId } = useMapWithStyle({ centerGeoUri, onError, id, interactive, bounds, allowGeolocate }); const onMapClick = (event: React.MouseEvent): void => { // Eat click events when clicking the attribution button diff --git a/src/utils/location/index.ts b/src/utils/location/index.ts index f94c6a12dd3..035fe526942 100644 --- a/src/utils/location/index.ts +++ b/src/utils/location/index.ts @@ -20,3 +20,4 @@ export * from "./locationEventGeoUri"; export * from "./LocationShareErrors"; export * from "./map"; export * from "./parseGeoUri"; +export * from "./positionFailureMessage"; diff --git a/src/utils/location/positionFailureMessage.ts b/src/utils/location/positionFailureMessage.ts new file mode 100644 index 00000000000..a1e2711208f --- /dev/null +++ b/src/utils/location/positionFailureMessage.ts @@ -0,0 +1,36 @@ +/* +Copyright 2023 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 { _t } from "../../languageHandler"; +import SdkConfig from "../../SdkConfig"; + +export const positionFailureMessage = (code: number): string => { + const brand = SdkConfig.get().brand; + switch (code) { + case 1: + return _t( + "%(brand)s was denied permission to fetch your location. " + + "Please allow location access in your browser settings.", + { brand }, + ); + case 2: + return _t("Failed to fetch your location. Please try again later."); + case 3: + return _t("Timed out trying to fetch your location. Please try again later."); + case 4: + return _t("Unknown error fetching location. Please try again later."); + } +}; From 73fdef2174a5d01d9f2c449b2bcc9ed63e3dcbc5 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Sat, 4 Feb 2023 20:06:20 +0100 Subject: [PATCH 02/13] Regenerate strings --- src/i18n/strings/en_EN.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index c1aeeab2de5..3e7ff552d09 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -787,6 +787,10 @@ "Reset bearing to north": "Reset bearing to north", "Zoom in": "Zoom in", "Zoom out": "Zoom out", + "%(brand)s was denied permission to fetch your location. Please allow location access in your browser settings.": "%(brand)s was denied permission to fetch your location. Please allow location access in your browser settings.", + "Failed to fetch your location. Please try again later.": "Failed to fetch your location. Please try again later.", + "Timed out trying to fetch your location. Please try again later.": "Timed out trying to fetch your location. Please try again later.", + "Unknown error fetching location. Please try again later.": "Unknown error fetching location. Please try again later.", "Are you sure you want to exit during this export?": "Are you sure you want to exit during this export?", "Unnamed Room": "Unnamed Room", "Generating a ZIP": "Generating a ZIP", @@ -2449,10 +2453,6 @@ "Click to move the pin": "Click to move the pin", "Click to drop a pin": "Click to drop a pin", "Share location": "Share location", - "%(brand)s was denied permission to fetch your location. Please allow location access in your browser settings.": "%(brand)s was denied permission to fetch your location. Please allow location access in your browser settings.", - "Failed to fetch your location. Please try again later.": "Failed to fetch your location. Please try again later.", - "Timed out trying to fetch your location. Please try again later.": "Timed out trying to fetch your location. Please try again later.", - "Unknown error fetching location. Please try again later.": "Unknown error fetching location. Please try again later.", "You don't have permission to share locations": "You don't have permission to share locations", "You need to have the right permissions in order to share locations in this room.": "You need to have the right permissions in order to share locations in this room.", "We couldn't send your location": "We couldn't send your location", From a3d09937d6044f668fba99f2d90a43c83bb4d187 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Sat, 4 Feb 2023 20:22:03 +0100 Subject: [PATCH 03/13] Fix types --- src/components/views/location/Map.tsx | 8 ++++---- src/utils/location/positionFailureMessage.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx index a6376039db5..34b9d4f104e 100644 --- a/src/components/views/location/Map.tsx +++ b/src/components/views/location/Map.tsx @@ -40,12 +40,12 @@ const useMapWithStyle = ({ }: { id: string; centerGeoUri?: string; - onError(error: Error): void; + onError?(error: Error): void; interactive?: boolean; bounds?: Bounds; - allowGeolocate: boolean; + allowGeolocate?: boolean; }): { - map: maplibregl.Map; + map: maplibregl.Map | undefined; bodyId: string; } => { const bodyId = `mx_Map_${id}`; @@ -122,7 +122,7 @@ const onGeolocateError = (e: GeolocationPositionError): void => { logger.error("Could not fetch location", e); Modal.createDialog(ErrorDialog, { title: _t("Could not fetch location"), - description: positionFailureMessage(e.code), + description: positionFailureMessage(e.code) ?? "", }); }; diff --git a/src/utils/location/positionFailureMessage.ts b/src/utils/location/positionFailureMessage.ts index a1e2711208f..8114ee96278 100644 --- a/src/utils/location/positionFailureMessage.ts +++ b/src/utils/location/positionFailureMessage.ts @@ -17,7 +17,7 @@ limitations under the License. import { _t } from "../../languageHandler"; import SdkConfig from "../../SdkConfig"; -export const positionFailureMessage = (code: number): string => { +export const positionFailureMessage = (code: number): string | undefined => { const brand = SdkConfig.get().brand; switch (code) { case 1: From 93682e9d13960f214dba77814384e3d1f5366e46 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Sat, 4 Feb 2023 20:29:50 +0100 Subject: [PATCH 04/13] Further type fixes --- src/utils/location/useMap.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/location/useMap.ts b/src/utils/location/useMap.ts index c4637a9a367..5cd78970691 100644 --- a/src/utils/location/useMap.ts +++ b/src/utils/location/useMap.ts @@ -21,7 +21,7 @@ import { createMap } from "./map"; interface UseMapProps { bodyId: string; - onError: (error: Error) => void; + onError?: (error: Error) => void; interactive?: boolean; } @@ -39,7 +39,7 @@ export const useMap = ({ interactive, bodyId, onError }: UseMapProps): MapLibreM try { setMap(createMap(interactive, bodyId, onError)); } catch (error) { - onError(error); + onError?.(error); } return () => { if (map) { From 2b97de7a479f919686dc87a66a126254afc1e799 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Sat, 4 Feb 2023 20:38:27 +0100 Subject: [PATCH 05/13] One more type fix --- test/components/views/messages/MBeaconBody-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/views/messages/MBeaconBody-test.tsx b/test/components/views/messages/MBeaconBody-test.tsx index a466f9be9ee..1e8af0e5617 100644 --- a/test/components/views/messages/MBeaconBody-test.tsx +++ b/test/components/views/messages/MBeaconBody-test.tsx @@ -436,7 +436,7 @@ describe("", () => { beforeEach(() => { // mock map utils to raise MapStyleUrlNotConfigured error jest.spyOn(mapUtilHooks, "useMap").mockImplementation(({ onError }) => { - onError(new Error(LocationShareError.MapStyleUrlNotConfigured)); + onError?.(new Error(LocationShareError.MapStyleUrlNotConfigured)); return mockMap; }); }); From fdac94a841a82211270bb14ada7c15e953cb165d Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Sat, 4 Feb 2023 20:58:53 +0100 Subject: [PATCH 06/13] Update snapshot --- .../LocationViewDialog-test.tsx.snap | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap index a2284ceee87..a19d6b0ff45 100644 --- a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap +++ b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap @@ -2,6 +2,7 @@ exports[` renders map correctly 1`] = ` renders map correctly 1`] = ` MockAttributionControl {}, "top-right", ], + [ + MockGeolocateControl { + "_events": { + "error": [Function], + }, + "_eventsCount": 1, + "_maxListeners": undefined, + "trigger": [MockFunction], + Symbol(kCapture): false, + }, + ], ], "results": [ { "type": "return", "value": undefined, }, + { + "type": "return", + "value": undefined, + }, ], }, "fitBounds": [MockFunction], @@ -97,12 +113,27 @@ exports[` renders map correctly 1`] = ` MockAttributionControl {}, "top-right", ], + [ + MockGeolocateControl { + "_events": { + "error": [Function], + }, + "_eventsCount": 1, + "_maxListeners": undefined, + "trigger": [MockFunction], + Symbol(kCapture): false, + }, + ], ], "results": [ { "type": "return", "value": undefined, }, + { + "type": "return", + "value": undefined, + }, ], }, "fitBounds": [MockFunction], From 06d8755f6b2347804fe2dfa52aa7a811133dcf22 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 7 Feb 2023 16:51:58 +1300 Subject: [PATCH 07/13] add tests for geolocate self on map views --- .../views/location/LocationViewDialog.tsx | 2 +- src/utils/location/positionFailureMessage.ts | 5 ++ .../location/LocationViewDialog-test.tsx | 3 +- test/components/views/location/Map-test.tsx | 78 ++++++++++++++++++- .../location/positionFailureMessage-test.ts | 38 +++++++++ 5 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 test/utils/location/positionFailureMessage-test.ts diff --git a/src/components/views/location/LocationViewDialog.tsx b/src/components/views/location/LocationViewDialog.tsx index 3e706d3df25..76f8001a980 100644 --- a/src/components/views/location/LocationViewDialog.tsx +++ b/src/components/views/location/LocationViewDialog.tsx @@ -68,7 +68,7 @@ export default class LocationViewDialog extends React.Component onError={this.onError} interactive className="mx_LocationViewDialog_map" - allowGeolocate={true} + allowGeolocate > {({ map }) => ( <> diff --git a/src/utils/location/positionFailureMessage.ts b/src/utils/location/positionFailureMessage.ts index 8114ee96278..a5c1e6e60b7 100644 --- a/src/utils/location/positionFailureMessage.ts +++ b/src/utils/location/positionFailureMessage.ts @@ -17,6 +17,11 @@ limitations under the License. import { _t } from "../../languageHandler"; import SdkConfig from "../../SdkConfig"; +/** + * Get a localised error message for GeolocationPositionError error codes + * @param code - error code from GeolocationPositionError + * @returns + */ export const positionFailureMessage = (code: number): string | undefined => { const brand = SdkConfig.get().brand; switch (code) { diff --git a/test/components/views/location/LocationViewDialog-test.tsx b/test/components/views/location/LocationViewDialog-test.tsx index 576e5f32179..41149299ee9 100644 --- a/test/components/views/location/LocationViewDialog-test.tsx +++ b/test/components/views/location/LocationViewDialog-test.tsx @@ -49,9 +49,10 @@ describe("", () => { it("renders marker correctly for self share", () => { const selfShareEvent = makeLocationEvent("geo:51.5076,-0.1276", LocationAssetType.Self); const member = new RoomMember(roomId, userId); - // @ts-ignore cheat assignment to property + // @ts-ignore cheat assignment to property selfShareEvent.sender = member; const component = getComponent({ mxEvent: selfShareEvent }); + // @ts-ignore fix when moving to rtl expect(component.find("SmartMarker").props()["roomMember"]).toEqual(member); }); }); diff --git a/test/components/views/location/Map-test.tsx b/test/components/views/location/Map-test.tsx index 1d92e098354..ab9f86cf98b 100644 --- a/test/components/views/location/Map-test.tsx +++ b/test/components/views/location/Map-test.tsx @@ -16,15 +16,18 @@ limitations under the License. import React from "react"; import { act } from "react-dom/test-utils"; +import { fireEvent, getByTestId, render } from "@testing-library/react"; import * as maplibregl from "maplibre-gl"; import { ClientEvent } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; -import { fireEvent, getByTestId, render } from "@testing-library/react"; +import { mocked } from "jest-mock"; import Map from "../../../../src/components/views/location/Map"; -import { getMockClientWithEventEmitter } from "../../../test-utils"; +import { getMockClientWithEventEmitter, getMockGeolocationPositionError } from "../../../test-utils"; import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; import { TILE_SERVER_WK_KEY } from "../../../../src/utils/WellKnownUtils"; +import Modal from "../../../../src/Modal"; +import ErrorDialog from "../../../../src/components/views/dialogs/ErrorDialog"; describe("", () => { const defaultProps = { @@ -52,6 +55,11 @@ describe("", () => { }); jest.spyOn(logger, "error").mockRestore(); + mocked(maplibregl.GeolocateControl).mockClear(); + }); + + afterEach(() => { + jest.spyOn(logger, "error").mockRestore(); }); const mapOptions = { container: {} as unknown as HTMLElement, style: "" }; @@ -201,4 +209,70 @@ describe("", () => { expect(onClick).toHaveBeenCalled(); }); }); + + describe("geolocate", () => { + it("does not add a geolocate control when allowGeolocate is falsy", () => { + getComponent({ allowGeolocate: false }); + + // didn't create a geolocation control + expect(maplibregl.GeolocateControl).not.toHaveBeenCalled(); + }); + + it("creates a geolocate control and adds it to the map when allowGeolocate is truthy", () => { + getComponent({ allowGeolocate: true }); + + // didn't create a geolocation control + expect(maplibregl.GeolocateControl).toHaveBeenCalledWith({ + positionOptions: { + enableHighAccuracy: true, + }, + trackUserLocation: false, + }); + + // mocked maplibregl shares mock for each mocked instance + // so we can assert the geolocate control was added using this static mock + const mockGeolocate = new maplibregl.GeolocateControl({}); + expect(mockMap.addControl).toHaveBeenCalledWith(mockGeolocate); + }); + + it("logs and opens a dialog on a geolocation error", () => { + const mockGeolocate = new maplibregl.GeolocateControl({}); + jest.spyOn(mockGeolocate, "on"); + const logSpy = jest.spyOn(logger, "error").mockImplementation(() => {}); + jest.spyOn(Modal, "createDialog"); + + const { rerender } = getComponent({ allowGeolocate: true }); + + // wait for component to settle + getComponent({ allowGeolocate: true }, rerender); + expect(mockGeolocate.on).toHaveBeenCalledWith("error", expect.any(Function)); + const error = getMockGeolocationPositionError(1, "Test"); + + // @ts-ignore pretend to have geolocate emit an error + mockGeolocate.emit("error", error); + + expect(logSpy).toHaveBeenCalledWith("Could not fetch location", error); + + expect(Modal.createDialog).toHaveBeenCalledWith(ErrorDialog, { + title: "Could not fetch location", + description: + "Element was denied permission to fetch your location. Please allow location access in your browser settings.", + }); + }); + + it("unsubscribes from geolocate errors on destroy", () => { + const mockGeolocate = new maplibregl.GeolocateControl({}); + jest.spyOn(mockGeolocate, "on"); + jest.spyOn(mockGeolocate, "off"); + jest.spyOn(Modal, "createDialog"); + + const { unmount } = getComponent({ allowGeolocate: true }); + + expect(mockGeolocate.on).toHaveBeenCalled(); + + unmount(); + + expect(mockGeolocate.off).toHaveBeenCalled(); + }); + }); }); diff --git a/test/utils/location/positionFailureMessage-test.ts b/test/utils/location/positionFailureMessage-test.ts new file mode 100644 index 00000000000..643ebd50990 --- /dev/null +++ b/test/utils/location/positionFailureMessage-test.ts @@ -0,0 +1,38 @@ +/* +Copyright 2023 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 { positionFailureMessage } from "../../../src/utils/location/positionFailureMessage"; + +describe("positionFailureMessage()", () => { + // error codes from GeolocationPositionError + // see: https://developer.mozilla.org/en-US/docs/Web/API/GeolocationPositionError + // 1: PERMISSION_DENIED + // 2: POSITION_UNAVAILABLE + // 3: TIMEOUT + type TestCase = [number, string | undefined]; + it.each([ + [ + 1, + "Element was denied permission to fetch your location. Please allow location access in your browser settings.", + ], + [2, "Failed to fetch your location. Please try again later."], + [3, "Timed out trying to fetch your location. Please try again later."], + [4, "Unknown error fetching location. Please try again later."], + [5, undefined], + ])("returns correct message for error code %s", (code, expectedMessage) => { + expect(positionFailureMessage(code)).toEqual(expectedMessage); + }); +}); From e5e2a825fbd3929d51bab466f0f9c8d4734b4457 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Tue, 7 Feb 2023 20:46:10 +0100 Subject: [PATCH 08/13] Make geolocate update with allowGeolocate --- src/components/views/location/Map.tsx | 29 +++++++++++-------- .../location/LocationViewDialog-test.tsx | 2 +- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx index 34b9d4f104e..c1dbcab337e 100644 --- a/src/components/views/location/Map.tsx +++ b/src/components/views/location/Map.tsx @@ -91,26 +91,31 @@ const useMapWithStyle = ({ } }, [map, bounds]); - const [geolocate] = useState( - allowGeolocate - ? new maplibregl.GeolocateControl({ - positionOptions: { - enableHighAccuracy: true, - }, - trackUserLocation: false, - }) - : null, - ); + const [geolocate, setGeolocate] = useState(null); useEffect(() => { - if (map && geolocate) { + if (!map) { + return; + } + if (allowGeolocate && !geolocate) { + const geolocate = new maplibregl.GeolocateControl({ + positionOptions: { + enableHighAccuracy: true, + }, + trackUserLocation: false, + }); + setGeolocate(geolocate); map.addControl(geolocate); geolocate.on("error", onGeolocateError); return () => { geolocate.off("error", onGeolocateError); }; } - }, [map, geolocate]); + if (!allowGeolocate && geolocate) { + map.removeControl(geolocate); + setGeolocate(null); + } + }, [map, geolocate, allowGeolocate]); return { map, diff --git a/test/components/views/location/LocationViewDialog-test.tsx b/test/components/views/location/LocationViewDialog-test.tsx index 41149299ee9..c9f6f490994 100644 --- a/test/components/views/location/LocationViewDialog-test.tsx +++ b/test/components/views/location/LocationViewDialog-test.tsx @@ -49,7 +49,7 @@ describe("", () => { it("renders marker correctly for self share", () => { const selfShareEvent = makeLocationEvent("geo:51.5076,-0.1276", LocationAssetType.Self); const member = new RoomMember(roomId, userId); - // @ts-ignore cheat assignment to property + // @ts-ignore cheat assignment to property selfShareEvent.sender = member; const component = getComponent({ mxEvent: selfShareEvent }); // @ts-ignore fix when moving to rtl From 69268856cfd9113b6458f190d32fbcb2fa4f03d1 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Wed, 8 Feb 2023 08:38:12 +0100 Subject: [PATCH 09/13] Update snapshot --- .../__snapshots__/LocationViewDialog-test.tsx.snap | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap index a19d6b0ff45..29d29e96dd9 100644 --- a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap +++ b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap @@ -32,10 +32,8 @@ exports[` renders map correctly 1`] = ` ], [ MockGeolocateControl { - "_events": { - "error": [Function], - }, - "_eventsCount": 1, + "_events": {}, + "_eventsCount": 0, "_maxListeners": undefined, "trigger": [MockFunction], Symbol(kCapture): false, @@ -115,10 +113,8 @@ exports[` renders map correctly 1`] = ` ], [ MockGeolocateControl { - "_events": { - "error": [Function], - }, - "_eventsCount": 1, + "_events": {}, + "_eventsCount": 0, "_maxListeners": undefined, "trigger": [MockFunction], Symbol(kCapture): false, From 039824c0ee01ae265523859db0bb4b2d3f6bb4e7 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 10 Feb 2023 08:25:59 +0100 Subject: [PATCH 10/13] Move error handling to separate effect --- src/components/views/location/Map.tsx | 13 ++++++---- .../PictureInPictureDragger-test.tsx.snap | 24 ------------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx index c1dbcab337e..22a4bda63e8 100644 --- a/src/components/views/location/Map.tsx +++ b/src/components/views/location/Map.tsx @@ -106,10 +106,6 @@ const useMapWithStyle = ({ }); setGeolocate(geolocate); map.addControl(geolocate); - geolocate.on("error", onGeolocateError); - return () => { - geolocate.off("error", onGeolocateError); - }; } if (!allowGeolocate && geolocate) { map.removeControl(geolocate); @@ -117,6 +113,15 @@ const useMapWithStyle = ({ } }, [map, geolocate, allowGeolocate]); + useEffect(() => { + if (geolocate) { + geolocate.on("error", onGeolocateError); + return () => { + geolocate.off("error", onGeolocateError); + }; + } + }, [geolocate]); + return { map, bodyId, diff --git a/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap b/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap index 0e382ea7f0a..4f95f5e41f7 100644 --- a/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap +++ b/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap @@ -30,27 +30,3 @@ exports[`PictureInPictureDragger when rendering the dragger with PiP content 1 a `; - -exports[`PictureInPictureDragger when rendering the dragger with PiP content 1 and rerendering PiP content 1 should not change the PiP content: pip-content-1 1`] = ` -
- -
-`; - -exports[`PictureInPictureDragger when rendering the dragger with PiP content 1 should render the PiP content: pip-content-1 1`] = ` -
- -
-`; From 886ecbcc588675d664eb92e16464b1954578eb8a Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 10 Feb 2023 08:48:37 +0100 Subject: [PATCH 11/13] Update snapshots --- .../__snapshots__/LocationViewDialog-test.tsx.snap | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap index c88bced2202..115dc412034 100644 --- a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap +++ b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap @@ -32,8 +32,10 @@ exports[` renders map correctly 1`] = ` ], [ MockGeolocateControl { - "_events": {}, - "_eventsCount": 0, + "_events": { + "error": [Function], + }, + "_eventsCount": 1, "_maxListeners": undefined, "trigger": [MockFunction], Symbol(kCapture): false, @@ -113,8 +115,10 @@ exports[` renders map correctly 1`] = ` ], [ MockGeolocateControl { - "_events": {}, - "_eventsCount": 0, + "_events": { + "error": [Function], + }, + "_eventsCount": 1, "_maxListeners": undefined, "trigger": [MockFunction], Symbol(kCapture): false, From 985d9983a354feb09d7ab1bd25f160a1ce450aa9 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Fri, 10 Feb 2023 10:14:34 +0100 Subject: [PATCH 12/13] Revert accidental changes in PictureInPictureDragger-test.tsx.snap --- .../PictureInPictureDragger-test.tsx.snap | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap b/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap index 4f95f5e41f7..0e382ea7f0a 100644 --- a/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap +++ b/test/components/structures/__snapshots__/PictureInPictureDragger-test.tsx.snap @@ -30,3 +30,27 @@ exports[`PictureInPictureDragger when rendering the dragger with PiP content 1 a `; + +exports[`PictureInPictureDragger when rendering the dragger with PiP content 1 and rerendering PiP content 1 should not change the PiP content: pip-content-1 1`] = ` +
+ +
+`; + +exports[`PictureInPictureDragger when rendering the dragger with PiP content 1 should render the PiP content: pip-content-1 1`] = ` +
+ +
+`; From de864f15bcafd70d4e392662cc0054001a79c55a Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 13 Feb 2023 20:25:43 +0100 Subject: [PATCH 13/13] Fix strict mode issues --- src/utils/location/map.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/location/map.ts b/src/utils/location/map.ts index 8ba3d68c48f..34d3d01478b 100644 --- a/src/utils/location/map.ts +++ b/src/utils/location/map.ts @@ -24,7 +24,7 @@ import { parseGeoUri } from "./parseGeoUri"; import { findMapStyleUrl } from "./findMapStyleUrl"; import { LocationShareError } from "./LocationShareErrors"; -export const createMap = (interactive: boolean, bodyId: string, onError: (error: Error) => void): maplibregl.Map => { +export const createMap = (interactive: boolean, bodyId: string, onError?: (error: Error) => void): maplibregl.Map => { try { const styleUrl = findMapStyleUrl(); @@ -54,7 +54,7 @@ export const createMap = (interactive: boolean, bodyId: string, onError: (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)); + onError?.(new Error(LocationShareError.MapStyleUrlNotReachable)); }); return map;