From d3c81cf4a5cab3ea18a3fae16aa9482fbefa40ba Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 10:42:19 +0100 Subject: [PATCH 01/23] center icon better Signed-off-by: Kerry Archibald --- src/components/views/messages/MLocationBody.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index 0d3ce4eebdb..4103c6a3f65 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -131,7 +131,7 @@ export default class MLocationBody extends React.Component { export function isSelfLocation(locationContent: ILocationContent): boolean { const asset = ASSET_NODE_TYPE.findIn(locationContent) as { type: string }; const assetType = asset?.type ?? LocationAssetType.Self; - return assetType == LocationAssetType.Self; + return false && assetType == LocationAssetType.Self; } interface ILocationBodyContentProps { From 9c0c90db28c2594bf4d2aee4fb3dd78e21b424be Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 10:46:49 +0100 Subject: [PATCH 02/23] remove debug Signed-off-by: Kerry Archibald --- src/components/views/messages/MLocationBody.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index 4103c6a3f65..0d3ce4eebdb 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -131,7 +131,7 @@ export default class MLocationBody extends React.Component { export function isSelfLocation(locationContent: ILocationContent): boolean { const asset = ASSET_NODE_TYPE.findIn(locationContent) as { type: string }; const assetType = asset?.type ?? LocationAssetType.Self; - return false && assetType == LocationAssetType.Self; + return assetType == LocationAssetType.Self; } interface ILocationBodyContentProps { From 267e63e9c944272937876aa5eaed37174caabe60 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 10:55:29 +0100 Subject: [PATCH 03/23] retrigger all builds Signed-off-by: Kerry Archibald From 259ce9a51924dd662b68c38a3716b9c2e2129fde Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 11:16:11 +0100 Subject: [PATCH 04/23] set assetType on share event Signed-off-by: Kerry Archibald --- src/components/views/location/LocationShareMenu.tsx | 9 +++++---- src/components/views/location/ShareType.tsx | 9 ++------- src/components/views/location/shareLocation.ts | 11 ++++++++++- .../views/location/LocationShareMenu-test.tsx | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/components/views/location/LocationShareMenu.tsx b/src/components/views/location/LocationShareMenu.tsx index ab332517942..11ddf2c7dbc 100644 --- a/src/components/views/location/LocationShareMenu.tsx +++ b/src/components/views/location/LocationShareMenu.tsx @@ -23,8 +23,9 @@ import ContextMenu, { AboveLeftOf } from '../../structures/ContextMenu'; import LocationPicker, { ILocationPickerProps } from "./LocationPicker"; import { shareLocation } from './shareLocation'; import SettingsStore from '../../../settings/SettingsStore'; -import ShareType, { LocationShareType } from './ShareType'; import ShareDialogButtons from './ShareDialogButtons'; +import ShareType from './ShareType'; +import { LocationShareType } from './shareLocation'; type Props = Omit & { onFinished: (ev?: SyntheticEvent) => void; @@ -68,13 +69,13 @@ const LocationShareMenu: React.FC = ({ managed={false} >
- { shareType ? : - } + } setShareType(undefined)} onCancel={onFinished} />
; diff --git a/src/components/views/location/ShareType.tsx b/src/components/views/location/ShareType.tsx index 8e16660dcb6..949274d8600 100644 --- a/src/components/views/location/ShareType.tsx +++ b/src/components/views/location/ShareType.tsx @@ -25,6 +25,7 @@ import AccessibleButton from '../elements/AccessibleButton'; import Heading from '../typography/Heading'; import { Icon as LocationIcon } from '../../../../res/img/element-icons/location.svg'; import { Icon as LiveLocationIcon } from '../../../../res/img/location/live-location.svg'; +import { LocationShareType } from './shareLocation'; const UserAvatar = () => { const matrixClient = useContext(MatrixClientContext); @@ -48,12 +49,6 @@ const UserAvatar = () => { ; }; -// TODO this will be defined somewhere better -export enum LocationShareType { - Own = 'Own', - Pin = 'Pin', - Live = 'Live' -} type ShareTypeOptionProps = HTMLAttributes & { label: string, shareType: LocationShareType }; const ShareTypeOption: React.FC = ({ onClick, label, shareType, ...rest @@ -62,7 +57,7 @@ const ShareTypeOption: React.FC = ({ className='mx_ShareType_option' onClick={onClick} // not yet implemented - disabled={shareType !== LocationShareType.Own} + disabled={shareType === LocationShareType.Live} {...rest}> { shareType === LocationShareType.Own && } { shareType === LocationShareType.Pin && diff --git a/src/components/views/location/shareLocation.ts b/src/components/views/location/shareLocation.ts index bd98f2ea8b2..f2b182daf0a 100644 --- a/src/components/views/location/shareLocation.ts +++ b/src/components/views/location/shareLocation.ts @@ -24,10 +24,18 @@ import { _t } from "../../../languageHandler"; import Modal from "../../../Modal"; import QuestionDialog from "../dialogs/QuestionDialog"; import SdkConfig from "../../../SdkConfig"; +import { LocationAssetType } from "matrix-js-sdk/src/@types/location"; + +export enum LocationShareType { + Own = 'Own', + Pin = 'Pin', + Live = 'Live' +} export const shareLocation = ( client: MatrixClient, roomId: string, + shareType: LocationShareType, relation: IEventRelation | undefined, openMenu: () => void, ) => async (uri: string, ts: number) => { @@ -35,7 +43,8 @@ export const shareLocation = ( try { const text = textForLocation(uri, ts, null); const threadId = relation?.rel_type === RelationType.Thread ? relation.event_id : null; - await client.sendMessage(roomId, threadId, makeLocationContent(text, uri, ts, null)); + const assetType = shareType === LocationShareType.Pin ? LocationAssetType.Pin : LocationAssetType.Self; + await client.sendMessage(roomId, threadId, makeLocationContent(text, uri, ts, null, assetType)); } catch (e) { logger.error("We couldn't send your location", e); diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 8ffd80bf294..0a80fea23cd 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -27,7 +27,7 @@ import MatrixClientContext from '../../../../src/contexts/MatrixClientContext'; import { ChevronFace } from '../../../../src/components/structures/ContextMenu'; import SettingsStore from '../../../../src/settings/SettingsStore'; import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; -import { LocationShareType } from '../../../../src/components/views/location/ShareType'; +import { LocationShareType } from '../../../../src/components/views/location/shareLocation'; import { findByTestId } from '../../../test-utils'; jest.mock('../../../../src/components/views/messages/MLocationBody', () => ({ From 095e40fdb6b23eb28e80c47693e45773c8e28e9b Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 11:27:55 +0100 Subject: [PATCH 05/23] use pin marker on map for pin drop share Signed-off-by: Kerry Archibald --- res/css/views/location/_LocationPicker.scss | 12 ++++++++---- .../views/location/LocationPicker.tsx | 18 ++++++++++++------ .../views/location/LocationShareMenu.tsx | 1 + 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index 76e56eedd9f..b8d5319754f 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -51,10 +51,9 @@ limitations under the License. background-color: $accent; filter: drop-shadow(0px 3px 5px rgba(0, 0, 0, 0.2)); - .mx_BaseAvatar { - margin-top: 2px; - margin-left: 2px; - } + display: flex; + align-items: center; + justify-content: center; } .mx_MLocationBody_pointer { @@ -103,3 +102,8 @@ limitations under the License. margin: auto; } } + +.mx_MLocationBody_markerIcon { + color: white; + height: 20px; +} \ No newline at end of file diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 150b8355e82..acc40819419 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -29,9 +29,12 @@ import Modal from '../../../Modal'; import ErrorDialog from '../dialogs/ErrorDialog'; import { findMapStyleUrl } from '../messages/MLocationBody'; import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; +import { LocationShareType } from './shareLocation'; +import { Icon as LocationIcon } from '../../../../res/img/element-icons/location.svg'; export interface ILocationPickerProps { sender: RoomMember; + shareType: LocationShareType; onChoose(uri: string, ts: number): unknown; onFinished(ev?: SyntheticEvent): void; } @@ -186,12 +189,15 @@ class LocationPicker extends React.Component {
- + {this.props.shareType === LocationShareType.Own ? + + : + }
= ({
{shareType ? From 48566237ebc13cd7959164c7300b456f5aba2b11 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 11:33:54 +0100 Subject: [PATCH 06/23] lint Signed-off-by: Kerry Archibald --- res/css/views/location/_LocationPicker.scss | 2 +- src/components/views/location/LocationPicker.tsx | 2 +- src/components/views/location/LocationShareMenu.tsx | 2 +- src/components/views/location/ShareType.tsx | 2 +- src/components/views/location/shareLocation.ts | 2 +- test/components/views/location/LocationShareMenu-test.tsx | 1 + 6 files changed, 6 insertions(+), 5 deletions(-) diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index b8d5319754f..e6fd16425eb 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -106,4 +106,4 @@ limitations under the License. .mx_MLocationBody_markerIcon { color: white; height: 20px; -} \ No newline at end of file +} diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index acc40819419..e87b57f63e7 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -189,7 +189,7 @@ class LocationPicker extends React.Component {
- {this.props.shareType === LocationShareType.Own ? + { this.props.shareType === LocationShareType.Own ? & { +type Props = Omit & { onFinished: (ev?: SyntheticEvent) => void; menuPosition: AboveLeftOf; openMenu: () => void; diff --git a/src/components/views/location/ShareType.tsx b/src/components/views/location/ShareType.tsx index 949274d8600..45cf43d24ef 100644 --- a/src/components/views/location/ShareType.tsx +++ b/src/components/views/location/ShareType.tsx @@ -57,7 +57,7 @@ const ShareTypeOption: React.FC = ({ className='mx_ShareType_option' onClick={onClick} // not yet implemented - disabled={shareType === LocationShareType.Live} + disabled={shareType === LocationShareType.Live} {...rest}> { shareType === LocationShareType.Own && } { shareType === LocationShareType.Pin && diff --git a/src/components/views/location/shareLocation.ts b/src/components/views/location/shareLocation.ts index f2b182daf0a..4ab8e169d02 100644 --- a/src/components/views/location/shareLocation.ts +++ b/src/components/views/location/shareLocation.ts @@ -19,12 +19,12 @@ import { MatrixClient } from "matrix-js-sdk/src/client"; import { makeLocationContent } from "matrix-js-sdk/src/content-helpers"; import { logger } from "matrix-js-sdk/src/logger"; import { IEventRelation } from "matrix-js-sdk/src/models/event"; +import { LocationAssetType } from "matrix-js-sdk/src/@types/location"; import { _t } from "../../../languageHandler"; import Modal from "../../../Modal"; import QuestionDialog from "../dialogs/QuestionDialog"; import SdkConfig from "../../../SdkConfig"; -import { LocationAssetType } from "matrix-js-sdk/src/@types/location"; export enum LocationShareType { Own = 'Own', diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 0a80fea23cd..27f7ee82a38 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -58,6 +58,7 @@ describe('', () => { getClientWellKnown: jest.fn().mockResolvedValue({ map_style_url: 'maps.com', }), + sendMessage: jest.fn(), }; const defaultProps = { From b4f3576a459144baf4f09f5a2f72f93a14bea823 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 12:11:45 +0100 Subject: [PATCH 07/23] test events Signed-off-by: Kerry Archibald --- __mocks__/maplibre-gl.js | 1 + .../views/location/LocationShareMenu.tsx | 4 +- .../views/location/LocationShareMenu-test.tsx | 88 +++++++++++++++++-- 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/__mocks__/maplibre-gl.js b/__mocks__/maplibre-gl.js index b1f114e8eff..8b347f2edb4 100644 --- a/__mocks__/maplibre-gl.js +++ b/__mocks__/maplibre-gl.js @@ -5,6 +5,7 @@ class MockMap extends EventEmitter { addControl = jest.fn(); removeControl = jest.fn(); } + class MockGeolocateControl extends EventEmitter { } diff --git a/src/components/views/location/LocationShareMenu.tsx b/src/components/views/location/LocationShareMenu.tsx index c7542056fb4..40adca86ef0 100644 --- a/src/components/views/location/LocationShareMenu.tsx +++ b/src/components/views/location/LocationShareMenu.tsx @@ -69,14 +69,14 @@ const LocationShareMenu: React.FC = ({ managed={false} >
- {shareType ? : - } + } setShareType(undefined)} onCancel={onFinished} />
; diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 27f7ee82a38..7542a40cf63 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -20,6 +20,7 @@ import { RoomMember } from 'matrix-js-sdk/src/models/room-member'; import { MatrixClient } from 'matrix-js-sdk/src/client'; import { mocked } from 'jest-mock'; import { act } from 'react-dom/test-utils'; +import { ASSET_NODE_TYPE, LocationAssetType } from 'matrix-js-sdk/src/@types/location'; import '../../../skinned-sdk'; import LocationShareMenu from '../../../../src/components/views/location/LocationShareMenu'; @@ -71,6 +72,17 @@ describe('', () => { roomId: '!room:server.org', sender: new RoomMember('!room:server.org', userId), }; + + const position = { + coords: { + latitude: -36.24484561954707, + longitude: 175.46884959563613, + accuracy: 10, + }, + timestamp: 1646305006802, + type: 'geolocate', + }; + const getComponent = (props = {}) => mount(, { wrappingComponent: MatrixClientContext.Provider, @@ -82,6 +94,8 @@ describe('', () => { (settingName) => settingName === "feature_location_share_pin_drop", ); + mockClient.sendMessage.mockClear(); + jest.spyOn(MatrixClientPeg, 'get').mockReturnValue(mockClient as unknown as MatrixClient); }); @@ -89,6 +103,21 @@ describe('', () => { findByTestId(component, `share-location-option-${shareType}`); const getBackButton = component => findByTestId(component, 'share-dialog-buttons-back'); const getCancelButton = component => findByTestId(component, 'share-dialog-buttons-cancel'); + const getSubmitButton = component => findByTestId(component, 'dialog-primary-button'); + const setLocation = (component) => { + // set the location + const locationPickerInstance = component.find('LocationPicker').instance(); + act(() => { + // @ts-ignore + locationPickerInstance.onGeolocate(position); + // make sure button gets enabled + component.setProps({}); + }); + }; + const setShareType = (component, shareType) => act(() => { + getShareTypeOption(component, shareType).at(0).simulate('click'); + component.setProps({}); + }); describe('when only Own share type is enabled', () => { beforeEach(() => { @@ -116,6 +145,28 @@ describe('', () => { expect(onFinished).toHaveBeenCalled(); }); + + it('creates static own location share event on submission', () => { + const onFinished = jest.fn(); + const component = getComponent({ onFinished }); + + setLocation(component); + + act(() => { + getSubmitButton(component).at(0).simulate('click'); + component.setProps({}); + }); + + expect(onFinished).toHaveBeenCalled(); + const [messageRoomId, relation, messageBody] = mockClient.sendMessage.mock.calls[0]; + expect(messageRoomId).toEqual(defaultProps.roomId); + expect(relation).toEqual(null); + expect(messageBody).toEqual(expect.objectContaining({ + [ASSET_NODE_TYPE.name]: { + type: LocationAssetType.Self, + }, + })); + }); }); describe('with pin drop share type enabled', () => { @@ -148,11 +199,7 @@ describe('', () => { it('selecting own location share type advances to location picker', () => { const component = getComponent(); - act(() => { - getShareTypeOption(component, LocationShareType.Own).at(0).simulate('click'); - }); - - component.setProps({}); + setShareType(component, LocationShareType.Own); expect(component.find('LocationPicker').length).toBeTruthy(); }); @@ -163,10 +210,7 @@ describe('', () => { const component = getComponent({ onFinished }); // advance to location picker - act(() => { - getShareTypeOption(component, LocationShareType.Own).at(0).simulate('click'); - component.setProps({}); - }); + setShareType(component, LocationShareType.Own); expect(component.find('LocationPicker').length).toBeTruthy(); @@ -178,5 +222,31 @@ describe('', () => { // back to share type expect(component.find('ShareType').length).toBeTruthy(); }); + + it('creates pin drop location share event on submission', () => { + // feature_location_share_pin_drop is set to enabled by default mocking + const onFinished = jest.fn(); + const component = getComponent({ onFinished }); + + // advance to location picker + setShareType(component, LocationShareType.Pin); + + setLocation(component); + + act(() => { + getSubmitButton(component).at(0).simulate('click'); + component.setProps({}); + }); + + expect(onFinished).toHaveBeenCalled(); + const [messageRoomId, relation, messageBody] = mockClient.sendMessage.mock.calls[0]; + expect(messageRoomId).toEqual(defaultProps.roomId); + expect(relation).toEqual(null); + expect(messageBody).toEqual(expect.objectContaining({ + [ASSET_NODE_TYPE.name]: { + type: LocationAssetType.Pin, + }, + })); + }); }); }); From 700d9c89a75f6f131c088c0c4391e519f8b490bf Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 12:27:37 +0100 Subject: [PATCH 08/23] pin drop helper text Signed-off-by: Kerry Archibald --- res/css/views/location/_LocationPicker.scss | 21 +++++++++++++++++++ .../views/location/LocationPicker.tsx | 11 +++++++++- src/i18n/strings/en_EN.json | 2 ++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index e6fd16425eb..5313662b0c3 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -107,3 +107,24 @@ limitations under the License. color: white; height: 20px; } + +.mx_LocationPicker_pinText { + position: absolute; + top: $spacing-16; + width: 100%; + box-sizing: border-box; + text-align: center; + height: 0; + pointer-events: none; + + span { + box-shadow: 0px 4px 15px rgba(0, 0, 0, 0.15); + border-radius: 8px; + padding: $spacing-8; + background-color: $background; + color: $primary-content; + + font-size: $font-12px; + } + +} \ No newline at end of file diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index e87b57f63e7..4d4f51ef40f 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -115,7 +115,10 @@ class LocationPicker extends React.Component { }); this.geolocate.on('error', this.onGeolocateError); - this.geolocate.on('geolocate', this.onGeolocate); + + if (this.props.shareType === LocationShareType.Own) { + this.geolocate.on('geolocate', this.onGeolocate); + } } catch (e) { logger.error("Failed to render map", e); this.setState({ error: e }); @@ -175,6 +178,12 @@ class LocationPicker extends React.Component { return (
+ {this.props.shareType === LocationShareType.Pin &&
+ + {this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin")} + +
+ } { error }
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 4f7f7cdf04e..cbbe06f2d79 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2173,6 +2173,8 @@ "toggle event": "toggle event", "Location": "Location", "Could not fetch location": "Could not fetch location", + "Click to move the pin": "Click to move the pin", + "Click to drop a pin": "Click to drop a pin", "Share location": "Share location", "Element was denied permission to fetch your location. Please allow location access in your browser settings.": "Element 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.", From a902773ac7db0715e632563128c8f11753489af6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 12:55:17 +0100 Subject: [PATCH 09/23] use generic location type Signed-off-by: Kerry Archibald --- .../views/location/LocationPicker.tsx | 84 +++++++++++++++---- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 4d4f51ef40f..7ed0e1d3f4f 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React, { SyntheticEvent } from 'react'; -import maplibregl from 'maplibre-gl'; +import maplibregl, { MapMouseEvent } from 'maplibre-gl'; import { logger } from "matrix-js-sdk/src/logger"; import { RoomMember } from 'matrix-js-sdk/src/models/room-member'; import { ClientEvent, IClientWellKnown } from 'matrix-js-sdk/src/client'; @@ -39,8 +39,15 @@ export interface ILocationPickerProps { onFinished(ev?: SyntheticEvent): void; } +interface IPosition { + latitude: number; + longitude: number; + altitude?: number; + accuracy?: number; + timestamp: number; +} interface IState { - position?: GeolocationPosition; + position?: IPosition; error: Error; } @@ -93,14 +100,6 @@ class LocationPicker extends React.Component { }); this.map.addControl(this.geolocate); - this.marker = new maplibregl.Marker({ - element: document.getElementById(this.getMarkerId()), - anchor: 'bottom', - offset: [0, -1], - }) - .setLngLat(new maplibregl.LngLat(0, 0)) - .addTo(this.map); - this.map.on('error', (e) => { logger.error( "Failed to load map: check map_style_url in config.json " @@ -117,8 +116,14 @@ class LocationPicker extends React.Component { this.geolocate.on('error', this.onGeolocateError); if (this.props.shareType === LocationShareType.Own) { + this.addMarkerToMap(); this.geolocate.on('geolocate', this.onGeolocate); } + + if (this.props.shareType === LocationShareType.Pin) { + this.map.on('click', this.onClick); + } + } catch (e) { logger.error("Failed to render map", e); this.setState({ error: e }); @@ -128,9 +133,20 @@ class LocationPicker extends React.Component { componentWillUnmount() { this.geolocate?.off('error', this.onGeolocateError); this.geolocate?.off('geolocate', this.onGeolocate); + this.map?.off('click', this.onClick); this.context.off(ClientEvent.ClientWellKnown, this.updateStyleUrl); } + private addMarkerToMap = () => { + this.marker = new maplibregl.Marker({ + element: document.getElementById(this.getMarkerId()), + anchor: 'bottom', + offset: [0, -1], + }) + .setLngLat(new maplibregl.LngLat(0, 0)) + .addTo(this.map); + } + private updateStyleUrl = (clientWellKnown: IClientWellKnown) => { const style = tileServerFromWellKnown(clientWellKnown)?.["map_style_url"]; if (style) { @@ -139,7 +155,7 @@ class LocationPicker extends React.Component { }; private onGeolocate = (position: GeolocationPosition) => { - this.setState({ position }); + this.setState({ position: genericPositionFromGeolocation(position) }); this.marker?.setLngLat( new maplibregl.LngLat( position.coords.longitude, @@ -148,6 +164,28 @@ class LocationPicker extends React.Component { ); }; + private onClick = (event: MapMouseEvent, ...rest) => { + console.log('hhh onClick', event, rest); + if (!this.marker) { + this.addMarkerToMap(); + } + this.marker?.setLngLat(event.lngLat); + this.setState({ + position: { + timestamp: Date.now(), + latitude: event.lngLat.lat, + longitude: event.lngLat.lng, + } + }) + // this.setState({ position }); + // this.marker?.setLngLat( + // new maplibregl.LngLat( + // position.coords.longitude, + // position.coords.latitude, + // ), + // ); + }; + private onGeolocateError = (e: GeolocationPositionError) => { this.props.onFinished(); logger.error("Could not fetch location", e); @@ -217,17 +255,27 @@ class LocationPicker extends React.Component { } } -export function getGeoUri(position: GeolocationPosition): string { - const lat = position.coords.latitude; - const lon = position.coords.longitude; +const genericPositionFromGeolocation = (geoPosition: GeolocationPosition): IPosition => { + const { + latitude, longitude, altitude, accuracy + } = geoPosition.coords; + return { + timestamp: geoPosition.timestamp, + latitude, longitude, altitude, accuracy + } +} + +export function getGeoUri(position: IPosition): string { + const lat = position.latitude; + const lon = position.longitude; const alt = ( - Number.isFinite(position.coords.altitude) - ? `,${position.coords.altitude}` + Number.isFinite(position.altitude) + ? `,${position.altitude}` : "" ); const acc = ( - Number.isFinite(position.coords.accuracy) - ? `;u=${ position.coords.accuracy }` + Number.isFinite(position.accuracy) + ? `;u=${position.accuracy}` : "" ); return `geo:${lat},${lon}${alt}${acc}`; From 99688d8e4a04512b425c9e6953037bbae13e4133 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 18:23:20 +0100 Subject: [PATCH 10/23] add navigationcontrol when in pin mode Signed-off-by: Kerry Archibald --- res/css/views/location/_LocationPicker.scss | 12 +-- .../views/location/LocationPicker.tsx | 34 ++++---- .../views/location/LocationPicker-test.tsx | 77 +++++++------------ 3 files changed, 49 insertions(+), 74 deletions(-) diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index 5313662b0c3..ecf2bf26b80 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -24,16 +24,19 @@ limitations under the License. height: 100%; border-radius: 8px; + .maplibregl-ctrl.maplibregl-ctrl-group, + .maplibregl-ctrl.maplibregl-ctrl-attrib { + margin-right: $spacing-16; + } + .maplibregl-ctrl.maplibregl-ctrl-group { // place below the close button // padding-16 + 24px close button + padding-10 margin-top: 50px; - margin-right: $spacing-16; } .maplibregl-ctrl-bottom-right { bottom: 68px; - margin-right: $spacing-16; } .maplibregl-user-location-accuracy-circle { @@ -123,8 +126,7 @@ limitations under the License. padding: $spacing-8; background-color: $background; color: $primary-content; - + font-size: $font-12px; } - -} \ No newline at end of file +} diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 7ed0e1d3f4f..b9ec5a66a56 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -121,9 +121,13 @@ class LocationPicker extends React.Component { } if (this.props.shareType === LocationShareType.Pin) { + const navigationControl = new maplibregl.NavigationControl({ + showCompass: false, showZoom: true, + }); + this.map.addControl(navigationControl, 'bottom-right'); + this.map.on('click', this.onClick); } - } catch (e) { logger.error("Failed to render map", e); this.setState({ error: e }); @@ -145,7 +149,7 @@ class LocationPicker extends React.Component { }) .setLngLat(new maplibregl.LngLat(0, 0)) .addTo(this.map); - } + }; private updateStyleUrl = (clientWellKnown: IClientWellKnown) => { const style = tileServerFromWellKnown(clientWellKnown)?.["map_style_url"]; @@ -164,8 +168,7 @@ class LocationPicker extends React.Component { ); }; - private onClick = (event: MapMouseEvent, ...rest) => { - console.log('hhh onClick', event, rest); + private onClick = (event: MapMouseEvent) => { if (!this.marker) { this.addMarkerToMap(); } @@ -175,15 +178,8 @@ class LocationPicker extends React.Component { timestamp: Date.now(), latitude: event.lngLat.lat, longitude: event.lngLat.lng, - } - }) - // this.setState({ position }); - // this.marker?.setLngLat( - // new maplibregl.LngLat( - // position.coords.longitude, - // position.coords.latitude, - // ), - // ); + }, + }); }; private onGeolocateError = (e: GeolocationPositionError) => { @@ -216,9 +212,9 @@ class LocationPicker extends React.Component { return (
- {this.props.shareType === LocationShareType.Pin &&
+ { this.props.shareType === LocationShareType.Pin &&
- {this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin")} + { this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin") }
} @@ -257,13 +253,13 @@ class LocationPicker extends React.Component { const genericPositionFromGeolocation = (geoPosition: GeolocationPosition): IPosition => { const { - latitude, longitude, altitude, accuracy + latitude, longitude, altitude, accuracy, } = geoPosition.coords; return { timestamp: geoPosition.timestamp, - latitude, longitude, altitude, accuracy - } -} + latitude, longitude, altitude, accuracy, + }; +}; export function getGeoUri(position: IPosition): string { const lat = position.latitude; diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index e28c8cea78e..7b8c6fa5e7a 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -20,80 +20,57 @@ import { getGeoUri } from "../../../../src/components/views/location/LocationPic describe("LocationPicker", () => { describe("getGeoUri", () => { it("Renders a URI with only lat and lon", () => { - const pos: GeolocationPosition = { - coords: { - latitude: 43.2, - longitude: 12.4, - altitude: undefined, - accuracy: undefined, - altitudeAccuracy: undefined, - heading: undefined, - speed: undefined, - }, + const pos = { + latitude: 43.2, + longitude: 12.4, + altitude: undefined, + accuracy: undefined, + timestamp: 12334, }; expect(getGeoUri(pos)).toEqual("geo:43.2,12.4"); }); it("Nulls in location are not shown in URI", () => { - const pos: GeolocationPosition = { - coords: { - latitude: 43.2, - longitude: 12.4, - altitude: null, - accuracy: null, - altitudeAccuracy: null, - heading: null, - speed: null, - }, + const pos = { + latitude: 43.2, + longitude: 12.4, + altitude: null, + accuracy: null, + timestamp: 12334, }; expect(getGeoUri(pos)).toEqual("geo:43.2,12.4"); }); it("Renders a URI with 3 coords", () => { - const pos: GeolocationPosition = { - coords: { - latitude: 43.2, - longitude: 12.4, - altitude: 332.54, - accuracy: undefined, - altitudeAccuracy: undefined, - heading: undefined, - speed: undefined, - }, + const pos = { + latitude: 43.2, + longitude: 12.4, + altitude: 332.54, + accuracy: undefined, timestamp: 12334, }; expect(getGeoUri(pos)).toEqual("geo:43.2,12.4,332.54"); }); it("Renders a URI with accuracy", () => { - const pos: GeolocationPosition = { - coords: { - latitude: 43.2, - longitude: 12.4, - altitude: undefined, - accuracy: 21, - altitudeAccuracy: undefined, - heading: undefined, - speed: undefined, - }, + const pos = { + latitude: 43.2, + longitude: 12.4, + altitude: undefined, + accuracy: 21, timestamp: 12334, }; expect(getGeoUri(pos)).toEqual("geo:43.2,12.4;u=21"); }); it("Renders a URI with accuracy and altitude", () => { - const pos: GeolocationPosition = { - coords: { - latitude: 43.2, - longitude: 12.4, - altitude: 12.3, - accuracy: 21, - altitudeAccuracy: undefined, - heading: undefined, - speed: undefined, - }, + const pos = { + latitude: 43.2, + longitude: 12.4, + altitude: 12.3, + accuracy: 21, timestamp: 12334, }; expect(getGeoUri(pos)).toEqual("geo:43.2,12.4,12.3;u=21"); From a22117e8af2adc54d5a4f0b6fcac2e13d68a0a7b Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 18:28:06 +0100 Subject: [PATCH 11/23] allow pin drop without location permissions Signed-off-by: Kerry Archibald --- .../views/location/LocationPicker.tsx | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index b9ec5a66a56..bf6944b5e62 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -183,17 +183,19 @@ class LocationPicker extends React.Component { }; private onGeolocateError = (e: GeolocationPositionError) => { - this.props.onFinished(); logger.error("Could not fetch location", e); - Modal.createTrackedDialog( - 'Could not fetch location', - '', - ErrorDialog, - { - title: _t("Could not fetch location"), - description: positionFailureMessage(e.code), - }, - ); + if (this.props.shareType === LocationShareType.Own) { + this.props.onFinished(); + Modal.createTrackedDialog( + 'Could not fetch location', + '', + ErrorDialog, + { + title: _t("Could not fetch location"), + description: positionFailureMessage(e.code), + }, + ); + } }; private onOk = () => { From 18c7aa631673d2202943e5890f8237d132e5e56f Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 18:40:44 +0100 Subject: [PATCH 12/23] remove geolocate control when pin dropping without geo perms Signed-off-by: Kerry Archibald --- src/components/views/location/LocationPicker.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index bf6944b5e62..da477f42aaa 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -116,7 +116,6 @@ class LocationPicker extends React.Component { this.geolocate.on('error', this.onGeolocateError); if (this.props.shareType === LocationShareType.Own) { - this.addMarkerToMap(); this.geolocate.on('geolocate', this.onGeolocate); } @@ -125,7 +124,6 @@ class LocationPicker extends React.Component { showCompass: false, showZoom: true, }); this.map.addControl(navigationControl, 'bottom-right'); - this.map.on('click', this.onClick); } } catch (e) { @@ -159,6 +157,9 @@ class LocationPicker extends React.Component { }; private onGeolocate = (position: GeolocationPosition) => { + if (!this.marker) { + this.addMarkerToMap(); + } this.setState({ position: genericPositionFromGeolocation(position) }); this.marker?.setLngLat( new maplibregl.LngLat( @@ -184,6 +185,8 @@ class LocationPicker extends React.Component { private onGeolocateError = (e: GeolocationPositionError) => { logger.error("Could not fetch location", e); + // close the dialog and show an error when trying to share own location + // pin drop location without permissions is ok if (this.props.shareType === LocationShareType.Own) { this.props.onFinished(); Modal.createTrackedDialog( @@ -196,6 +199,10 @@ class LocationPicker extends React.Component { }, ); } + + if (this.geolocate) { + this.map?.removeControl(this.geolocate); + } }; private onOk = () => { From b7c1a4cdda03f9705dce8f4a4f4f227e3fe1ee62 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 19:53:48 +0100 Subject: [PATCH 13/23] test locationpicker Signed-off-by: Kerry Archibald --- __mocks__/maplibre-gl.js | 20 +- .../views/location/LocationPicker.tsx | 3 +- src/utils/WellKnownUtils.ts | 1 + .../views/location/LocationPicker-test.tsx | 236 +++++++++++++++++- 4 files changed, 249 insertions(+), 11 deletions(-) diff --git a/__mocks__/maplibre-gl.js b/__mocks__/maplibre-gl.js index 8b347f2edb4..8686089825d 100644 --- a/__mocks__/maplibre-gl.js +++ b/__mocks__/maplibre-gl.js @@ -1,21 +1,23 @@ const EventEmitter = require("events"); -const { LngLat } = require('maplibre-gl'); +const { LngLat, NavigationControl } = require('maplibre-gl'); class MockMap extends EventEmitter { addControl = jest.fn(); removeControl = jest.fn(); } +const MockMapInstance = new MockMap(); class MockGeolocateControl extends EventEmitter { - -} -class MockMarker extends EventEmitter { - setLngLat = jest.fn().mockReturnValue(this); - addTo = jest.fn(); + trigger = jest.fn(); } +const MockGeolocateInstance = new MockGeolocateControl(); +const MockMarker = {} +MockMarker.setLngLat = jest.fn().mockReturnValue(MockMarker); +MockMarker.addTo = jest.fn().mockReturnValue(MockMarker); module.exports = { - Map: MockMap, - GeolocateControl: MockGeolocateControl, - Marker: MockMarker, + Map: jest.fn().mockReturnValue(MockMapInstance), + GeolocateControl: jest.fn().mockReturnValue(MockGeolocateInstance), + Marker: jest.fn().mockReturnValue(MockMarker), LngLat, + NavigationControl }; diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index da477f42aaa..5c7e1646f7d 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -98,6 +98,7 @@ class LocationPicker extends React.Component { }, trackUserLocation: true, }); + this.map.addControl(this.geolocate); this.map.on('error', (e) => { @@ -214,7 +215,7 @@ class LocationPicker extends React.Component { render() { const error = this.state.error ? -
+
{ _t("Failed to load map") }
: null; diff --git a/src/utils/WellKnownUtils.ts b/src/utils/WellKnownUtils.ts index cb3e92a7bde..100589f61d6 100644 --- a/src/utils/WellKnownUtils.ts +++ b/src/utils/WellKnownUtils.ts @@ -64,6 +64,7 @@ export function getTileServerWellKnown(): ITileServerWellKnown | undefined { export function tileServerFromWellKnown( clientWellKnown?: IClientWellKnown | undefined, ): ITileServerWellKnown { + console.log(clientWellKnown); return ( clientWellKnown?.[TILE_SERVER_WK_KEY.name] ?? clientWellKnown?.[TILE_SERVER_WK_KEY.altName] diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index 7b8c6fa5e7a..2c6dabc9fe2 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -13,9 +13,25 @@ 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 maplibregl from "maplibre-gl"; +import { mount } from "enzyme"; +import { act } from 'react-dom/test-utils'; +import { RoomMember } from "matrix-js-sdk/src/models/room-member"; +import { MatrixClient } from 'matrix-js-sdk/src/client'; +import { mocked } from 'jest-mock'; +import { logger } from 'matrix-js-sdk/src/logger'; import "../../../skinned-sdk"; // Must be first for skinning to work -import { getGeoUri } from "../../../../src/components/views/location/LocationPicker"; +import LocationPicker, { getGeoUri } from "../../../../src/components/views/location/LocationPicker"; +import { LocationShareType } from "../../../../src/components/views/location/shareLocation"; +import MatrixClientContext from '../../../../src/contexts/MatrixClientContext'; +import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; +import { findByTestId } from '../../../test-utils'; + +jest.mock('../../../../src/components/views/messages/MLocationBody', () => ({ + findMapStyleUrl: jest.fn().mockReturnValue('tileserver.com'), +})); describe("LocationPicker", () => { describe("getGeoUri", () => { @@ -76,4 +92,222 @@ describe("LocationPicker", () => { expect(getGeoUri(pos)).toEqual("geo:43.2,12.4,12.3;u=21"); }); }); + + describe('', () => { + const roomId = '!room:server.org'; + const userId = '@user:server.org'; + const sender = new RoomMember(roomId, userId); + const defaultProps = { + sender, + shareType: LocationShareType.Own, + onChoose: jest.fn(), + onFinished: jest.fn(), + }; + const mockClient = { + on: jest.fn(), + off: jest.fn(), + isGuest: jest.fn(), + getClientWellKnown: jest.fn(), + }; + const getComponent = (props = {}) => mount(, { + wrappingComponent: MatrixClientContext.Provider, + wrappingComponentProps: { value: mockClient }, + }); + + const mockMap = new maplibregl.Map(); + const mockGeolocate = new maplibregl.GeolocateControl(); + const mockMarker = new maplibregl.Marker(); + + const mockGeolocationPosition = { + coords: { + latitude: 43.2, + longitude: 12.4, + altitude: 12.3, + accuracy: 21, + }, + timestamp: 123, + }; + const mockClickEvent = { + lngLat: { + lat: 43.2, + lng: 12.4, + }, + }; + + beforeEach(() => { + jest.spyOn(logger, 'error').mockRestore(); + jest.spyOn(MatrixClientPeg, 'get').mockReturnValue(mockClient as unknown as MatrixClient); + jest.clearAllMocks(); + mocked(mockMap).addControl.mockReset(); + }); + + it('displays error when map emits an error', () => { + // suppress expected error log + jest.spyOn(logger, 'error').mockImplementation(() => { }); + const wrapper = getComponent(); + + act(() => { + // @ts-ignore + mocked(mockMap).emit('error', { error: 'Something went wrong' }); + wrapper.setProps({}); + }); + + expect(findByTestId(wrapper, 'location-picker-error').length).toBeTruthy(); + }); + + it('displays error when map setup throws', () => { + // suppress expected error log + jest.spyOn(logger, 'error').mockImplementation(() => { }); + + // throw an error + mocked(mockMap).addControl.mockImplementation(() => { throw new Error('oups'); }); + + const wrapper = getComponent(); + wrapper.setProps({}); + + expect(findByTestId(wrapper, 'location-picker-error').length).toBeTruthy(); + }); + + it('initiates map with geolocation', () => { + getComponent(); + + expect(mockMap.addControl).toHaveBeenCalledWith(mockGeolocate); + act(() => { + // @ts-ignore + mocked(mockMap).emit('load'); + }); + + expect(mockGeolocate.trigger).toHaveBeenCalled(); + }); + + describe('for Own location share type', () => { + it('closes and displays error when geolocation errors', () => { + // suppress expected error log + jest.spyOn(logger, 'error').mockImplementation(() => { }); + const onFinished = jest.fn(); + getComponent({ onFinished }); + + expect(mockMap.addControl).toHaveBeenCalledWith(mockGeolocate); + act(() => { + // @ts-ignore + mockMap.emit('load'); + // @ts-ignore + mockGeolocate.emit('error', {}); + }); + + // dialog is closed on error + expect(onFinished).toHaveBeenCalled(); + }); + + it('sets position on geolocate event', () => { + const wrapper = getComponent(); + act(() => { + // @ts-ignore + mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition); + wrapper.setProps({}); + }); + + // marker added + expect(maplibregl.Marker).toHaveBeenCalled(); + expect(mockMarker.setLngLat).toHaveBeenCalledWith(new maplibregl.LngLat( + 12.4, 43.2, + )); + // submit button is enabled when position is truthy + expect(findByTestId(wrapper, 'dialog-primary-button').at(0).props().disabled).toBeFalsy(); + }); + + it('submits location', () => { + const onChoose = jest.fn(); + const wrapper = getComponent({ onChoose }); + act(() => { + // @ts-ignore + mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition); + // make sure button is enabled + wrapper.setProps({}); + }); + + act(() => { + findByTestId(wrapper, 'dialog-primary-button').simulate('click'); + }); + + // content of this call is tested in LocationShareMenu-test + expect(onChoose).toHaveBeenCalled(); + }); + }); + + describe('for Pin drop location share type', () => { + const shareType = LocationShareType.Pin; + it('initiates map with geolocation', () => { + getComponent({ shareType }); + + expect(mockMap.addControl).toHaveBeenCalledWith(mockGeolocate); + act(() => { + // @ts-ignore + mocked(mockMap).emit('load'); + }); + + expect(mockGeolocate.trigger).toHaveBeenCalled(); + }); + + it('removes geolocation control on geolocation error', () => { + // suppress expected error log + jest.spyOn(logger, 'error').mockImplementation(() => { }); + const onFinished = jest.fn(); + getComponent({ onFinished, shareType }); + act(() => { + // @ts-ignore + mockMap.emit('load'); + // @ts-ignore + mockGeolocate.emit('error', {}); + }); + + expect(mockMap.removeControl).toHaveBeenCalledWith(mockGeolocate); + // dialog is not closed + expect(onFinished).not.toHaveBeenCalled(); + }); + + it('does not set position on geolocate event', () => { + getComponent({ shareType }); + act(() => { + // @ts-ignore + mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition); + }); + + // marker added + expect(maplibregl.Marker).not.toHaveBeenCalled(); + }); + + it('sets position on click event', () => { + const wrapper = getComponent({ shareType }); + act(() => { + // @ts-ignore + mocked(mockMap).emit('click', mockClickEvent); + wrapper.setProps({}); + }); + + // marker added + expect(maplibregl.Marker).toHaveBeenCalled(); + expect(mockMarker.setLngLat).toHaveBeenCalledWith(new maplibregl.LngLat( + 12.4, 43.2, + )); + }); + + it('submits location', () => { + const onChoose = jest.fn(); + const wrapper = getComponent({ onChoose, shareType }); + act(() => { + // @ts-ignore + mocked(mockMap).emit('click', mockClickEvent); + wrapper.setProps({}); + }); + + act(() => { + findByTestId(wrapper, 'dialog-primary-button').simulate('click'); + }); + + // content of this call is tested in LocationShareMenu-test + expect(onChoose).toHaveBeenCalled(); + }); + }); + }); }); From eed8c4e15e3d072afaf636ae7837efdb2c0f322c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 20:05:53 +0100 Subject: [PATCH 14/23] test marker type, tidy Signed-off-by: Kerry Archibald --- src/components/views/location/LocationPicker.tsx | 3 +-- src/utils/WellKnownUtils.ts | 1 - test/components/views/location/LocationPicker-test.tsx | 4 ++++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 5c7e1646f7d..e4a551cb295 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -145,8 +145,7 @@ class LocationPicker extends React.Component { element: document.getElementById(this.getMarkerId()), anchor: 'bottom', offset: [0, -1], - }) - .setLngLat(new maplibregl.LngLat(0, 0)) + }).setLngLat(new maplibregl.LngLat(0, 0)) .addTo(this.map); }; diff --git a/src/utils/WellKnownUtils.ts b/src/utils/WellKnownUtils.ts index 100589f61d6..cb3e92a7bde 100644 --- a/src/utils/WellKnownUtils.ts +++ b/src/utils/WellKnownUtils.ts @@ -64,7 +64,6 @@ export function getTileServerWellKnown(): ITileServerWellKnown | undefined { export function tileServerFromWellKnown( clientWellKnown?: IClientWellKnown | undefined, ): ITileServerWellKnown { - console.log(clientWellKnown); return ( clientWellKnown?.[TILE_SERVER_WK_KEY.name] ?? clientWellKnown?.[TILE_SERVER_WK_KEY.altName] diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index 2c6dabc9fe2..67b428fa5ee 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -214,6 +214,7 @@ describe("LocationPicker", () => { )); // submit button is enabled when position is truthy expect(findByTestId(wrapper, 'dialog-primary-button').at(0).props().disabled).toBeFalsy(); + expect(wrapper.find('MemberAvatar').length).toBeTruthy(); }); it('submits location', () => { @@ -290,6 +291,9 @@ describe("LocationPicker", () => { expect(mockMarker.setLngLat).toHaveBeenCalledWith(new maplibregl.LngLat( 12.4, 43.2, )); + + // marker is set, icon not avatar + expect(wrapper.find('.mx_MLocationBody_markerIcon').length).toBeTruthy(); }); it('submits location', () => { From a9f1ec140b153f30c3d888b6cae16558ec44b338 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 4 Mar 2022 15:32:16 +0100 Subject: [PATCH 15/23] move findMapStyleUrl Signed-off-by: Kerry Archibald --- .../views/location/LocationPicker.tsx | 2 +- .../views/location/LocationShareErrors.ts | 5 ++ .../views/location/findMapStyleUrl.ts | 25 ++++++++++ .../views/messages/MLocationBody.tsx | 48 +++++++------------ .../views/location/LocationShareMenu-test.tsx | 2 +- 5 files changed, 49 insertions(+), 33 deletions(-) create mode 100644 src/components/views/location/LocationShareErrors.ts create mode 100644 src/components/views/location/findMapStyleUrl.ts diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 150b8355e82..23953682b23 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -27,8 +27,8 @@ import MemberAvatar from '../avatars/MemberAvatar'; import MatrixClientContext from '../../../contexts/MatrixClientContext'; import Modal from '../../../Modal'; import ErrorDialog from '../dialogs/ErrorDialog'; -import { findMapStyleUrl } from '../messages/MLocationBody'; import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; +import { findMapStyleUrl } from './findMapStyleUrl'; export interface ILocationPickerProps { sender: RoomMember; diff --git a/src/components/views/location/LocationShareErrors.ts b/src/components/views/location/LocationShareErrors.ts new file mode 100644 index 00000000000..059050eb854 --- /dev/null +++ b/src/components/views/location/LocationShareErrors.ts @@ -0,0 +1,5 @@ +export enum LocationShareError { + MapStyleUrlNotConfigured = 'MapStyleUrlNotConfigured', + MapStyleUrlNotReachable = 'MapStyleUrlNotReachable', + Default = 'Default' +} diff --git a/src/components/views/location/findMapStyleUrl.ts b/src/components/views/location/findMapStyleUrl.ts new file mode 100644 index 00000000000..168a84c890d --- /dev/null +++ b/src/components/views/location/findMapStyleUrl.ts @@ -0,0 +1,25 @@ +import { logger } from "matrix-js-sdk/src/logger"; + +import SdkConfig from "../../../SdkConfig"; +import { getTileServerWellKnown } from "../../../utils/WellKnownUtils"; +import { LocationShareError } from "./LocationShareErrors"; + +/** + * Look up what map tile server style URL was provided in the homeserver's + * .well-known location, or, failing that, in our local config, or, failing + * that, defaults to the same tile server listed by matrix.org. + */ +export function findMapStyleUrl(): string { + const mapStyleUrl = ( + getTileServerWellKnown()?.map_style_url ?? + SdkConfig.get().map_style_url + ); + + if (!mapStyleUrl) { + logger.error("'map_style_url' missing from homeserver .well-known area, and " + + "missing from from config.json."); + throw new Error(LocationShareError.MapStyleUrlNotConfigured); + } + + return mapStyleUrl; +} diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index 0d3ce4eebdb..e4def0464f2 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -26,7 +26,6 @@ import { } from 'matrix-js-sdk/src/@types/location'; import { ClientEvent, IClientWellKnown } from 'matrix-js-sdk/src/client'; -import SdkConfig from '../../../SdkConfig'; import { replaceableComponent } from "../../../utils/replaceableComponent"; import { IBodyProps } from "./IBodyProps"; import { _t } from '../../../languageHandler'; @@ -36,8 +35,9 @@ import LocationViewDialog from '../location/LocationViewDialog'; import TooltipTarget from '../elements/TooltipTarget'; import { Alignment } from '../elements/Tooltip'; import AccessibleButton from '../elements/AccessibleButton'; -import { getTileServerWellKnown, tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; +import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; import MatrixClientContext from '../../../contexts/MatrixClientContext'; +import { findMapStyleUrl } from '../location/findMapStyleUrl'; interface IState { error: Error; @@ -117,14 +117,16 @@ export default class MLocationBody extends React.Component { }; render(): React.ReactElement { - return ; + return this.state.error ? + : + ; } } @@ -146,6 +148,11 @@ interface ILocationBodyContentProps { onZoomOut?: () => void; } +export const LocationBodyFallbackContent: React.FC<{ event: MatrixEvent, error: Error }> = ({ error, event }) => { + console.log('hhh', error, event); + return <>'Failed to load map; +}; + export function LocationBodyContent(props: ILocationBodyContentProps): React.ReactElement { const mapDiv =
; } -/** - * Look up what map tile server style URL was provided in the homeserver's - * .well-known location, or, failing that, in our local config, or, failing - * that, defaults to the same tile server listed by matrix.org. - */ -export function findMapStyleUrl(): string { - const mapStyleUrl = ( - getTileServerWellKnown()?.map_style_url ?? - SdkConfig.get().map_style_url - ); - - if (!mapStyleUrl) { - throw new Error( - "'map_style_url' missing from homeserver .well-known area, and " + - "missing from from config.json.", - ); - } - - return mapStyleUrl; -} - export function createMap( coords: GeolocationCoordinates, interactive: boolean, diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 8ffd80bf294..4c95ea3587e 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -30,7 +30,7 @@ import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; import { LocationShareType } from '../../../../src/components/views/location/ShareType'; import { findByTestId } from '../../../test-utils'; -jest.mock('../../../../src/components/views/messages/MLocationBody', () => ({ +jest.mock('../../../../src/components/views/location/findMapStyleUrl', () => ({ findMapStyleUrl: jest.fn().mockReturnValue('test'), })); From 87a53e8a0933e88ed84b639e26d0e09500f2fb4a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 4 Mar 2022 16:30:24 +0100 Subject: [PATCH 16/23] fallback to event content when cant render map Signed-off-by: Kerry Archibald --- .../views/location/LocationShareErrors.ts | 13 ++++++++++ .../views/messages/MLocationBody.tsx | 17 +++++++++++-- src/i18n/strings/en_EN.json | 4 ++++ .../views/messages/MLocationBody-test.tsx | 24 ++++++++++++++++++- 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/components/views/location/LocationShareErrors.ts b/src/components/views/location/LocationShareErrors.ts index 059050eb854..9edf2a0686c 100644 --- a/src/components/views/location/LocationShareErrors.ts +++ b/src/components/views/location/LocationShareErrors.ts @@ -1,5 +1,18 @@ +import { _t } from "../../../languageHandler"; + export enum LocationShareError { MapStyleUrlNotConfigured = 'MapStyleUrlNotConfigured', MapStyleUrlNotReachable = 'MapStyleUrlNotReachable', Default = 'Default' } + +export const getLocationShareErrorMessage = (errorType?: LocationShareError): string => { + switch (errorType) { + case LocationShareError.MapStyleUrlNotConfigured: + return _t('Unable to load map: This homeserver is not configured to display maps.'); + case LocationShareError.MapStyleUrlNotReachable: + default: + return _t(`Unable to load map: This homeserver is not configured correctly to display maps, ` + + `or the configured map server may be unreachable.`); + } +}; diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index e4def0464f2..78db75fdee4 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -38,6 +38,7 @@ import AccessibleButton from '../elements/AccessibleButton'; import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; import MatrixClientContext from '../../../contexts/MatrixClientContext'; import { findMapStyleUrl } from '../location/findMapStyleUrl'; +import { getLocationShareErrorMessage, LocationShareError } from '../location/LocationShareErrors'; interface IState { error: Error; @@ -149,8 +150,20 @@ interface ILocationBodyContentProps { } export const LocationBodyFallbackContent: React.FC<{ event: MatrixEvent, error: Error }> = ({ error, event }) => { - console.log('hhh', error, event); - return <>'Failed to load map; + const errorType = error?.message as LocationShareError; + const message = getLocationShareErrorMessage(errorType); + + const locationFallback = isSelfLocation(event.getContent()) ? + (_t('Shared their location: ') + event.getContent()?.body) : + (_t('Shared a location: ') + event.getContent()?.body); + + return
+ + { message } + +
+ { locationFallback } +
; }; export function LocationBodyContent(props: ILocationBodyContentProps): diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a5860d382ee..6d1d6ee0517 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2133,6 +2133,10 @@ "%(name)s wants to verify": "%(name)s wants to verify", "You sent a verification request": "You sent a verification request", "Expand map": "Expand map", + "Unable to load map: This homeserver is not configured to display maps.": "Unable to load map: This homeserver is not configured to display maps.", + "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.": "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.", + "Shared their location: ": "Shared their location: ", + "Shared a location: ": "Shared a location: ", "Failed to load map": "Failed to load map", "Zoom in": "Zoom in", "Zoom out": "Zoom out", diff --git a/test/components/views/messages/MLocationBody-test.tsx b/test/components/views/messages/MLocationBody-test.tsx index d5b9665f3a8..257210119f7 100644 --- a/test/components/views/messages/MLocationBody-test.tsx +++ b/test/components/views/messages/MLocationBody-test.tsx @@ -26,11 +26,13 @@ import { TEXT_NODE_TYPE } from "matrix-js-sdk/src/@types/extensible_events"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import sdk from "../../../skinned-sdk"; -import { +import MLocationBody, { createMapSiteLink, isSelfLocation, parseGeoUri, } from "../../../../src/components/views/messages/MLocationBody"; +import { mount } from "enzyme"; +import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; sdk.getComponent("views.messages.MLocationBody"); @@ -240,6 +242,26 @@ describe("MLocationBody", () => { expect(isSelfLocation(content)).toBe(false); }); }); + + describe('', () => { + describe('with error', () => { + const mockClient = {}; + const defaultEvent = modernLocationEvent("geo:51.5076,-0.1276") + const defaultProps = { + mxEvent: defaultEvent, + highlights: [], + highlightLink: '', + onHeightChanged: jest.fn(), + onMessageAllowed: jest.fn(), + permalinkCreator: {}, + mediaEventHelper: {} + } + const getComponent = (props = {}) => mount(, { + wrappingComponent: MatrixClientContext.Provider, + wrappingComponentProps: { value: mockClient } + }) + }); + }); }); function oldLocationEvent(geoUri: string): MatrixEvent { From b5226ba828a4ab075170a6dfc59422b2b286a594 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 4 Mar 2022 17:26:14 +0100 Subject: [PATCH 17/23] update mocks in location picker, show same messages as timeline Signed-off-by: Kerry Archibald --- .../views/location/LocationPicker.tsx | 20 +++-- .../views/messages/MLocationBody.tsx | 11 +-- .../views/location/LocationPicker-test.tsx | 30 +++++++- .../views/messages/MLocationBody-test.tsx | 75 +++++++++++++++---- .../__snapshots__/MLocationBody-test.tsx.snap | 29 +++++++ 5 files changed, 130 insertions(+), 35 deletions(-) create mode 100644 test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index db55ab8a94d..d9b10e40a44 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -31,6 +31,7 @@ import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; import { findMapStyleUrl } from './findMapStyleUrl'; import { LocationShareType } from './shareLocation'; import { Icon as LocationIcon } from '../../../../res/img/element-icons/location.svg'; +import { getLocationShareErrorMessage, LocationShareError } from './LocationShareErrors'; export interface ILocationPickerProps { sender: RoomMember; @@ -48,7 +49,7 @@ interface IPosition { } interface IState { position?: IPosition; - error: Error; + error?: LocationShareError; } /* @@ -107,7 +108,7 @@ class LocationPicker extends React.Component { + "has a valid URL and API key", e.error, ); - this.setState({ error: e.error }); + this.setState({ error: LocationShareError.MapStyleUrlNotReachable }); }); this.map.on('load', () => { @@ -129,7 +130,10 @@ class LocationPicker extends React.Component { } } catch (e) { logger.error("Failed to render map", e); - this.setState({ error: e }); + const errorType = e?.message === LocationShareError.MapStyleUrlNotConfigured ? + LocationShareError.MapStyleUrlNotConfigured : + LocationShareError.Default; + this.setState({ error: errorType }); } } @@ -215,19 +219,19 @@ class LocationPicker extends React.Component { render() { const error = this.state.error ?
- {_t("Failed to load map")} + { getLocationShareErrorMessage(this.state.error) }
: null; return (
- {this.props.shareType === LocationShareType.Pin &&
+ { this.props.shareType === LocationShareType.Pin &&
- {this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin")} + { this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin") }
} - {error} + { error }
{
- {this.props.shareType === LocationShareType.Own ? + { this.props.shareType === LocationShareType.Own ? - + { message }
@@ -186,13 +186,6 @@ export function LocationBodyContent(props: ILocationBodyContentProps): ); return
- { - props.error - ?
- { _t("Failed to load map") } -
- : null - } { props.tooltip ? ({ +jest.mock('../../../../src/components/views/location/findMapStyleUrl', () => ({ findMapStyleUrl: jest.fn().mockReturnValue('tileserver.com'), })); @@ -139,6 +141,7 @@ describe("LocationPicker", () => { jest.spyOn(MatrixClientPeg, 'get').mockReturnValue(mockClient as unknown as MatrixClient); jest.clearAllMocks(); mocked(mockMap).addControl.mockReset(); + mocked(findMapStyleUrl).mockReturnValue('tileserver.com'); }); it('displays error when map emits an error', () => { @@ -152,7 +155,25 @@ describe("LocationPicker", () => { wrapper.setProps({}); }); - expect(findByTestId(wrapper, 'location-picker-error').length).toBeTruthy(); + expect(findByTestId(wrapper, 'location-picker-error').text()).toEqual( + "Unable to load map: This homeserver is not configured correctly to display maps, " + + "or the configured map server may be unreachable.", + ); + }); + + it('displays error when map display is not configured properly', () => { + // suppress expected error log + jest.spyOn(logger, 'error').mockImplementation(() => { }); + mocked(findMapStyleUrl).mockImplementation(() => { + throw new Error(LocationShareError.MapStyleUrlNotConfigured); + }); + + const wrapper = getComponent(); + wrapper.setProps({}); + + expect(findByTestId(wrapper, 'location-picker-error').text()).toEqual( + "Unable to load map: This homeserver is not configured to display maps.", + ); }); it('displays error when map setup throws', () => { @@ -165,7 +186,10 @@ describe("LocationPicker", () => { const wrapper = getComponent(); wrapper.setProps({}); - expect(findByTestId(wrapper, 'location-picker-error').length).toBeTruthy(); + expect(findByTestId(wrapper, 'location-picker-error').text()).toEqual( + "Unable to load map: This homeserver is not configured correctly to display maps, " + + "or the configured map server may be unreachable.", + ); }); it('initiates map with geolocation', () => { diff --git a/test/components/views/messages/MLocationBody-test.tsx b/test/components/views/messages/MLocationBody-test.tsx index 257210119f7..c47f842e33c 100644 --- a/test/components/views/messages/MLocationBody-test.tsx +++ b/test/components/views/messages/MLocationBody-test.tsx @@ -14,6 +14,9 @@ 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 { makeLocationContent } from "matrix-js-sdk/src/content-helpers"; import { ASSET_NODE_TYPE, @@ -24,6 +27,8 @@ import { } from "matrix-js-sdk/src/@types/location"; import { TEXT_NODE_TYPE } from "matrix-js-sdk/src/@types/extensible_events"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; +import maplibregl from 'maplibre-gl'; +import { logger } from 'matrix-js-sdk/src/logger'; import sdk from "../../../skinned-sdk"; import MLocationBody, { @@ -31,9 +36,21 @@ import MLocationBody, { isSelfLocation, parseGeoUri, } from "../../../../src/components/views/messages/MLocationBody"; -import { mount } from "enzyme"; import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; +import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks"; +import { MediaEventHelper } from "../../../../src/utils/MediaEventHelper"; +import { getTileServerWellKnown } from "../../../../src/utils/WellKnownUtils"; +import SdkConfig from "../../../../src/SdkConfig"; + +jest.mock("../../../../src/utils/WellKnownUtils", () => ({ + getTileServerWellKnown: jest.fn(), +})); +let EVENT_ID = 0; +function nextId(): string { + EVENT_ID++; + return EVENT_ID.toString(); +} sdk.getComponent("views.messages.MLocationBody"); describe("MLocationBody", () => { @@ -245,21 +262,54 @@ describe("MLocationBody", () => { describe('', () => { describe('with error', () => { - const mockClient = {}; - const defaultEvent = modernLocationEvent("geo:51.5076,-0.1276") + const mockClient = { + on: jest.fn(), + off: jest.fn(), + }; + const defaultEvent = modernLocationEvent("geo:51.5076,-0.1276", LocationAssetType.Pin); const defaultProps = { mxEvent: defaultEvent, highlights: [], highlightLink: '', onHeightChanged: jest.fn(), onMessageAllowed: jest.fn(), - permalinkCreator: {}, - mediaEventHelper: {} - } + permalinkCreator: {} as RoomPermalinkCreator, + mediaEventHelper: {} as MediaEventHelper, + }; const getComponent = (props = {}) => mount(, { wrappingComponent: MatrixClientContext.Provider, - wrappingComponentProps: { value: mockClient } - }) + wrappingComponentProps: { value: mockClient }, + }); + let sdkConfigSpy; + + beforeEach(() => { + // eat expected errors to keep console clean + jest.spyOn(logger, 'error').mockImplementation(() => { }); + mocked(getTileServerWellKnown).mockReturnValue({}); + sdkConfigSpy = jest.spyOn(SdkConfig, 'get').mockReturnValue({}); + }); + + afterAll(() => { + sdkConfigSpy.mockRestore(); + jest.spyOn(logger, 'error').mockRestore(); + }); + + it('displays correct fallback content without error style when map_style_url is not configured', () => { + const component = getComponent(); + expect(component.find(".mx_EventTile_body")).toMatchSnapshot(); + }); + + it('displays correct fallback content when map_style_url is misconfigured', () => { + const mockMap = new maplibregl.Map(); + mocked(getTileServerWellKnown).mockReturnValue({ map_style_url: 'bad-tile-server.com' }); + const component = getComponent(); + + // simulate error initialising map in maplibregl + // @ts-ignore + mockMap.emit('error', { status: 404 }); + component.setProps({}); + expect(component.find(".mx_EventTile_body")).toMatchSnapshot(); + }); }); }); }); @@ -278,7 +328,7 @@ function oldLocationEvent(geoUri: string): MatrixEvent { ); } -function modernLocationEvent(geoUri: string): MatrixEvent { +function modernLocationEvent(geoUri: string, assetType?: LocationAssetType): MatrixEvent { return new MatrixEvent( { "event_id": nextId(), @@ -288,6 +338,7 @@ function modernLocationEvent(geoUri: string): MatrixEvent { geoUri, 252523, "Human-readable label", + assetType, ), }, ); @@ -307,9 +358,3 @@ function nonLocationEvent(): MatrixEvent { }, ); } - -let EVENT_ID = 0; -function nextId(): string { - EVENT_ID++; - return EVENT_ID.toString(); -} diff --git a/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap b/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap new file mode 100644 index 00000000000..7db39cdfcb6 --- /dev/null +++ b/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap @@ -0,0 +1,29 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`MLocationBody with error displays correct fallback content when map_style_url is misconfigured 1`] = ` +
+ + Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable. + +
+ Shared a location: Found at geo:51.5076,-0.1276 at 2021-12-21T12:22+0000 +
+`; + +exports[`MLocationBody with error displays correct fallback content without error style when map_style_url is not configured 1`] = ` +
+ + Unable to load map: This homeserver is not configured to display maps. + +
+ Shared a location: Found at geo:51.5076,-0.1276 at 2021-12-21T12:22+0000 +
+`; From 537398f9fee2a9ae9d2d8039306dbfe2494c2f27 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 4 Mar 2022 17:54:09 +0100 Subject: [PATCH 18/23] style error message in location share menu Signed-off-by: Kerry Archibald --- res/css/views/location/_LocationPicker.scss | 24 +++++++++++++++---- .../views/location/LocationPicker.tsx | 19 ++++++++++----- .../views/location/LocationPicker-test.tsx | 6 ++--- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index ecf2bf26b80..466a9d03cd1 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -20,6 +20,16 @@ limitations under the License. height: 100%; position: relative; + // when there are errors loading the map + // the canvas is still inserted + // and can overlap error message/close buttons + // hide it + &.mx_LocationPicker_hasError { + .maplibregl-canvas-container, .maplibregl-control-container { + display: none; + } + } + #mx_LocationPicker_map { height: 100%; border-radius: 8px; @@ -99,11 +109,6 @@ limitations under the License. } } } - - .mx_LocationPicker_error { - color: red; - margin: auto; - } } .mx_MLocationBody_markerIcon { @@ -130,3 +135,12 @@ limitations under the License. font-size: $font-12px; } } + +.mx_LocationPicker_error { + padding: 60px; + text-align: center; + + p { + margin: $spacing-24 0; + } +} diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index d9b10e40a44..8a4de27fcc1 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -32,7 +32,8 @@ import { findMapStyleUrl } from './findMapStyleUrl'; import { LocationShareType } from './shareLocation'; import { Icon as LocationIcon } from '../../../../res/img/element-icons/location.svg'; import { getLocationShareErrorMessage, LocationShareError } from './LocationShareErrors'; - +import { Icon as WarningBadge } from '../../../../res/img/element-icons/warning-badge.svg'; +import AccessibleButton from '../elements/AccessibleButton'; export interface ILocationPickerProps { sender: RoomMember; shareType: LocationShareType; @@ -217,10 +218,17 @@ class LocationPicker extends React.Component { }; render() { - const error = this.state.error ? -
- { getLocationShareErrorMessage(this.state.error) } -
: null; + if (this.state.error) { + return
+
+ +

+ { getLocationShareErrorMessage(this.state.error) } +

+ { _t("Cancel") } +
+
; + } return (
@@ -231,7 +239,6 @@ class LocationPicker extends React.Component {
} - { error }
{ wrapper.setProps({}); }); - expect(findByTestId(wrapper, 'location-picker-error').text()).toEqual( + expect(findByTestId(wrapper, 'location-picker-error').find('p').text()).toEqual( "Unable to load map: This homeserver is not configured correctly to display maps, " + "or the configured map server may be unreachable.", ); @@ -171,7 +171,7 @@ describe("LocationPicker", () => { const wrapper = getComponent(); wrapper.setProps({}); - expect(findByTestId(wrapper, 'location-picker-error').text()).toEqual( + expect(findByTestId(wrapper, 'location-picker-error').find('p').text()).toEqual( "Unable to load map: This homeserver is not configured to display maps.", ); }); @@ -186,7 +186,7 @@ describe("LocationPicker", () => { const wrapper = getComponent(); wrapper.setProps({}); - expect(findByTestId(wrapper, 'location-picker-error').text()).toEqual( + expect(findByTestId(wrapper, 'location-picker-error').find('p').text()).toEqual( "Unable to load map: This homeserver is not configured correctly to display maps, " + "or the configured map server may be unreachable.", ); From a8b25adb0ee5b7833545fd8111d6e1764cfd9477 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 4 Mar 2022 18:00:50 +0100 Subject: [PATCH 19/23] i18n Signed-off-by: Kerry Archibald --- src/i18n/strings/en_EN.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 109fea331bb..24e4dc88e86 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2133,11 +2133,8 @@ "%(name)s wants to verify": "%(name)s wants to verify", "You sent a verification request": "You sent a verification request", "Expand map": "Expand map", - "Unable to load map: This homeserver is not configured to display maps.": "Unable to load map: This homeserver is not configured to display maps.", - "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.": "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.", "Shared their location: ": "Shared their location: ", "Shared a location: ": "Shared a location: ", - "Failed to load map": "Failed to load map", "Zoom in": "Zoom in", "Zoom out": "Zoom out", "Can't edit poll": "Can't edit poll", @@ -2185,6 +2182,8 @@ "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.", + "Unable to load map: This homeserver is not configured to display maps.": "Unable to load map: This homeserver is not configured to display maps.", + "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.": "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.", "We couldn't send your location": "We couldn't send your location", "%(brand)s could not send your location. Please try again later.": "%(brand)s could not send your location. Please try again later.", "My current location": "My current location", From b27d473c5f0bb89ef345eb998143d3af04589384 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 7 Mar 2022 09:46:17 +0100 Subject: [PATCH 20/23] forgotten copyright Signed-off-by: Kerry Archibald --- .../views/location/LocationShareErrors.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/components/views/location/LocationShareErrors.ts b/src/components/views/location/LocationShareErrors.ts index 9edf2a0686c..cd1c2537504 100644 --- a/src/components/views/location/LocationShareErrors.ts +++ b/src/components/views/location/LocationShareErrors.ts @@ -1,3 +1,19 @@ +/* +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 { _t } from "../../../languageHandler"; export enum LocationShareError { From a749c726c6e5650d6f94cbf7a4457c9a898cc778 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 9 Mar 2022 10:00:33 +0100 Subject: [PATCH 21/23] add copyright Signed-off-by: Kerry Archibald --- src/components/views/location/findMapStyleUrl.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/components/views/location/findMapStyleUrl.ts b/src/components/views/location/findMapStyleUrl.ts index 168a84c890d..bf3823da7dc 100644 --- a/src/components/views/location/findMapStyleUrl.ts +++ b/src/components/views/location/findMapStyleUrl.ts @@ -1,3 +1,19 @@ +/* +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 { logger } from "matrix-js-sdk/src/logger"; import SdkConfig from "../../../SdkConfig"; From a8b653b8f3e5b8eeef5d5eb4017e732da957e153 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 9 Mar 2022 18:51:45 +0100 Subject: [PATCH 22/23] update style Signed-off-by: Kerry Archibald --- res/css/_components.scss | 1 + .../components/views/location/_MapError.scss | 36 +++++++ res/css/views/location/_LocationPicker.scss | 34 ------- .../views/location/LocationPicker.tsx | 22 ++--- .../views/location/LocationShareErrors.ts | 4 +- src/components/views/location/MapError.tsx | 39 ++++++++ .../views/messages/MLocationBody.tsx | 2 +- src/i18n/strings/en_EN.json | 5 +- .../views/location/LocationPicker-test.tsx | 7 +- .../views/location/MapError-test.tsx | 43 +++++++++ .../__snapshots__/MapError-test.tsx.snap | 95 +++++++++++++++++++ 11 files changed, 233 insertions(+), 55 deletions(-) create mode 100644 res/css/components/views/location/_MapError.scss create mode 100644 src/components/views/location/MapError.tsx create mode 100644 test/components/views/location/MapError-test.tsx create mode 100644 test/components/views/location/__snapshots__/MapError-test.tsx.snap diff --git a/res/css/_components.scss b/res/css/_components.scss index 65ce8742b87..9c7f6b878e3 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -5,6 +5,7 @@ @import "./_font-weights.scss"; @import "./_spacing.scss"; @import "./components/views/location/_LocationShareMenu.scss"; +@import "./components/views/location/_MapError.scss"; @import "./components/views/location/_ShareDialogButtons.scss"; @import "./components/views/location/_ShareType.scss"; @import "./components/views/spaces/_QuickThemeSwitcher.scss"; diff --git a/res/css/components/views/location/_MapError.scss b/res/css/components/views/location/_MapError.scss new file mode 100644 index 00000000000..4f042d7d20e --- /dev/null +++ b/res/css/components/views/location/_MapError.scss @@ -0,0 +1,36 @@ +/* +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. +*/ + +.mx_MapError { + padding: 100px $spacing-32 0; + text-align: center; + + p { + margin: $spacing-16 0 $spacing-32; + } +} + +.mx_MapError_heading { + padding-top: $spacing-24; +} + +.mx_MapError_icon { + height: 48px; + + path { + fill: $secondary-content; + } +} diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index f9ba7fd2e94..53b3655ed8b 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -131,40 +131,6 @@ limitations under the License. } } -.mx_LocationPicker_error { - padding: 60px; - text-align: center; - - p { - margin: $spacing-24 0; - } -} - -.mx_MLocationBody_markerIcon { - color: white; - height: 20px; -} - -.mx_LocationPicker_pinText { - position: absolute; - top: $spacing-16; - width: 100%; - box-sizing: border-box; - text-align: center; - height: 0; - pointer-events: none; - - span { - box-shadow: 0px 4px 15px rgba(0, 0, 0, 0.15); - border-radius: 8px; - padding: $spacing-8; - background-color: $background; - color: $primary-content; - - font-size: $font-12px; - } -} - .mx_LocationPicker_submitButton { width: 100%; height: 48px; diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index ab44a9cfa4d..26eda1edba5 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -30,9 +30,9 @@ import { tileServerFromWellKnown } from '../../../utils/WellKnownUtils'; import { findMapStyleUrl } from './findMapStyleUrl'; import { LocationShareType } from './shareLocation'; import { Icon as LocationIcon } from '../../../../res/img/element-icons/location.svg'; -import { getLocationShareErrorMessage, LocationShareError } from './LocationShareErrors'; -import { Icon as WarningBadge } from '../../../../res/img/element-icons/warning-badge.svg'; +import { LocationShareError } from './LocationShareErrors'; import AccessibleButton from '../elements/AccessibleButton'; +import { MapError } from './MapError'; export interface ILocationPickerProps { sender: RoomMember; shareType: LocationShareType; @@ -219,22 +219,18 @@ class LocationPicker extends React.Component { render() { if (this.state.error) { return
-
- -

- {getLocationShareErrorMessage(this.state.error)} -

- {_t("Cancel")} -
+
; } return (
- {this.props.shareType === LocationShareType.Pin &&
+ { this.props.shareType === LocationShareType.Pin &&
- {this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin")} + { this.state.position ? _t("Click to move the pin") : _t("Click to drop a pin") }
} @@ -249,13 +245,13 @@ class LocationPicker extends React.Component { className='mx_LocationPicker_submitButton' disabled={!this.state.position} onClick={this.onOk}> - {_t('Share location')} + { _t('Share location') }
- {this.props.shareType === LocationShareType.Own ? + { this.props.shareType === LocationShareType.Own ? { switch (errorType) { case LocationShareError.MapStyleUrlNotConfigured: - return _t('Unable to load map: This homeserver is not configured to display maps.'); + return _t('This homeserver is not configured to display maps.'); case LocationShareError.MapStyleUrlNotReachable: default: - return _t(`Unable to load map: This homeserver is not configured correctly to display maps, ` + return _t(`This homeserver is not configured correctly to display maps, ` + `or the configured map server may be unreachable.`); } }; diff --git a/src/components/views/location/MapError.tsx b/src/components/views/location/MapError.tsx new file mode 100644 index 00000000000..7ab5d681ab1 --- /dev/null +++ b/src/components/views/location/MapError.tsx @@ -0,0 +1,39 @@ +/* +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 { Icon as WarningBadge } from '../../../../res/img/element-icons/warning-badge.svg'; +import { _t } from '../../../languageHandler'; +import AccessibleButton from '../elements/AccessibleButton'; +import Heading from '../typography/Heading'; +import { getLocationShareErrorMessage, LocationShareError } from './LocationShareErrors'; + +interface Props { + onFinished: () => void; + error: LocationShareError; +} + +export const MapError: React.FC = ({ + onFinished, error, +}) => (
+ + { _t("Unable to load map") } +

+ { getLocationShareErrorMessage(error) } +

+ { _t("OK") } +
); diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index 98a66b81bdd..fdd92dcc6d4 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -151,7 +151,7 @@ interface ILocationBodyContentProps { export const LocationBodyFallbackContent: React.FC<{ event: MatrixEvent, error: Error }> = ({ error, event }) => { const errorType = error?.message as LocationShareError; - const message = getLocationShareErrorMessage(errorType); + const message = `${_t('Unable to load map')}: ${getLocationShareErrorMessage(errorType)}`; const locationFallback = isSelfLocation(event.getContent()) ? (_t('Shared their location: ') + event.getContent()?.body) : diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 0ac03e2e8b6..b33fe1d28ce 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2134,6 +2134,7 @@ "%(name)s wants to verify": "%(name)s wants to verify", "You sent a verification request": "You sent a verification request", "Expand map": "Expand map", + "Unable to load map": "Unable to load map", "Shared their location: ": "Shared their location: ", "Shared a location: ": "Shared a location: ", "Zoom in": "Zoom in", @@ -2183,8 +2184,8 @@ "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.", - "Unable to load map: This homeserver is not configured to display maps.": "Unable to load map: This homeserver is not configured to display maps.", - "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.": "Unable to load map: This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.", + "This homeserver is not configured to display maps.": "This homeserver is not configured to display maps.", + "This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.": "This homeserver is not configured correctly to display maps, or the configured map server may be unreachable.", "We couldn't send your location": "We couldn't send your location", "%(brand)s could not send your location. Please try again later.": "%(brand)s could not send your location. Please try again later.", "My current location": "My current location", diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index f1da485eca7..324ae549aab 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -13,6 +13,7 @@ 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 maplibregl from "maplibre-gl"; import { mount } from "enzyme"; @@ -156,7 +157,7 @@ describe("LocationPicker", () => { }); expect(findByTestId(wrapper, 'location-picker-error').find('p').text()).toEqual( - "Unable to load map: This homeserver is not configured correctly to display maps, " + "This homeserver is not configured correctly to display maps, " + "or the configured map server may be unreachable.", ); }); @@ -172,7 +173,7 @@ describe("LocationPicker", () => { wrapper.setProps({}); expect(findByTestId(wrapper, 'location-picker-error').find('p').text()).toEqual( - "Unable to load map: This homeserver is not configured to display maps.", + "This homeserver is not configured to display maps.", ); }); @@ -187,7 +188,7 @@ describe("LocationPicker", () => { wrapper.setProps({}); expect(findByTestId(wrapper, 'location-picker-error').find('p').text()).toEqual( - "Unable to load map: This homeserver is not configured correctly to display maps, " + "This homeserver is not configured correctly to display maps, " + "or the configured map server may be unreachable.", ); }); diff --git a/test/components/views/location/MapError-test.tsx b/test/components/views/location/MapError-test.tsx new file mode 100644 index 00000000000..e482d043d0a --- /dev/null +++ b/test/components/views/location/MapError-test.tsx @@ -0,0 +1,43 @@ +/* +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 '../../../skinned-sdk'; +import { MapError } from '../../../../src/components/views/location/MapError'; +import { LocationShareError } from '../../../../src/components/views/location/LocationShareErrors'; + +describe('', () => { + const defaultProps = { + onFinished: jest.fn(), + error: LocationShareError.MapStyleUrlNotConfigured, + }; + const getComponent = (props = {}) => + mount(); + + it('renders correctly for MapStyleUrlNotConfigured', () => { + const component = getComponent(); + expect(component).toMatchSnapshot(); + }); + + it('renders correctly for MapStyleUrlNotReachable', () => { + const component = getComponent({ + error: LocationShareError.MapStyleUrlNotReachable, + }); + expect(component).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/location/__snapshots__/MapError-test.tsx.snap b/test/components/views/location/__snapshots__/MapError-test.tsx.snap new file mode 100644 index 00000000000..726e9454938 --- /dev/null +++ b/test/components/views/location/__snapshots__/MapError-test.tsx.snap @@ -0,0 +1,95 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders correctly for MapStyleUrlNotConfigured 1`] = ` + +
+
+ +

+ Unable to load map +

+ +

+ This homeserver is not configured to display maps. +

+ + + +
+ +`; + +exports[` renders correctly for MapStyleUrlNotReachable 1`] = ` + +
+
+ +

+ Unable to load map +

+ +

+ This homeserver is not configured correctly to display maps, or the configured map server may be unreachable. +

+ + + +
+ +`; From 77a2c82a8a8b75cb5443edd5e66c4af3618c07db Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 9 Mar 2022 18:53:16 +0100 Subject: [PATCH 23/23] icon bigger Signed-off-by: Kerry Archibald --- res/css/components/views/location/_MapError.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/css/components/views/location/_MapError.scss b/res/css/components/views/location/_MapError.scss index 4f042d7d20e..83d6316ec44 100644 --- a/res/css/components/views/location/_MapError.scss +++ b/res/css/components/views/location/_MapError.scss @@ -28,7 +28,7 @@ limitations under the License. } .mx_MapError_icon { - height: 48px; + height: 58px; path { fill: $secondary-content;