From d3c81cf4a5cab3ea18a3fae16aa9482fbefa40ba Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 3 Mar 2022 10:42:19 +0100 Subject: [PATCH 01/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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 fd71d85ef811333d531421bc07530feb011826e5 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 9 Mar 2022 14:53:30 +0100 Subject: [PATCH 15/16] tweak style Signed-off-by: Kerry Archibald --- res/css/views/location/_LocationPicker.scss | 26 +++++++++---------- .../views/location/LocationPicker.tsx | 20 ++++++++------ .../views/location/LocationPicker-test.tsx | 6 ++--- .../views/location/LocationShareMenu-test.tsx | 2 +- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index ecf2bf26b80..be68eaf5ebe 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -19,6 +19,7 @@ limitations under the License. height: 100%; position: relative; + overflow: hidden; #mx_LocationPicker_map { height: 100%; @@ -36,7 +37,7 @@ limitations under the License. } .maplibregl-ctrl-bottom-right { - bottom: 68px; + bottom: 80px; } .maplibregl-user-location-accuracy-circle { @@ -85,19 +86,13 @@ limitations under the License. position: absolute; bottom: 0px; width: 100%; + box-sizing: border-box; + padding: $spacing-16; + display: flex; + flex-direction: column; + justify-content: stretch; - .mx_Dialog_buttons { - text-align: center; - - /* Note the `button` prefix and `not()` clauses are needed to make - these selectors more specific than those in _common.scss. */ - - button.mx_Dialog_primary:not(.mx_Dialog_nonDialogButton):not(.mx_AccessibleButton) { - margin: 0px 0px 16px 0px; - min-width: 328px; - min-height: 48px; - } - } + background-color: $header-panel-bg-color; } .mx_LocationPicker_error { @@ -130,3 +125,8 @@ limitations under the License. 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 e4a551cb295..01b6b62d3c2 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -20,7 +20,6 @@ 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'; -import DialogButtons from "../elements/DialogButtons"; import { _t } from '../../../languageHandler'; import { replaceableComponent } from "../../../utils/replaceableComponent"; import MemberAvatar from '../avatars/MemberAvatar'; @@ -31,6 +30,7 @@ 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'; +import AccessibleButton from '../elements/AccessibleButton'; export interface ILocationPickerProps { sender: RoomMember; @@ -230,13 +230,17 @@ class LocationPicker extends React.Component { { error }
- + + + {_t('Share location')} +
diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index 67b428fa5ee..46ca04aeffc 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -213,7 +213,7 @@ describe("LocationPicker", () => { 12.4, 43.2, )); // submit button is enabled when position is truthy - expect(findByTestId(wrapper, 'dialog-primary-button').at(0).props().disabled).toBeFalsy(); + expect(findByTestId(wrapper, 'location-picker-submit-button').at(0).props().disabled).toBeFalsy(); expect(wrapper.find('MemberAvatar').length).toBeTruthy(); }); @@ -228,7 +228,7 @@ describe("LocationPicker", () => { }); act(() => { - findByTestId(wrapper, 'dialog-primary-button').simulate('click'); + findByTestId(wrapper, 'location-picker-submit-button').at(0).simulate('click'); }); // content of this call is tested in LocationShareMenu-test @@ -306,7 +306,7 @@ describe("LocationPicker", () => { }); act(() => { - findByTestId(wrapper, 'dialog-primary-button').simulate('click'); + findByTestId(wrapper, 'location-picker-submit-button').at(0).simulate('click'); }); // content of this call is tested in LocationShareMenu-test diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 7542a40cf63..c7a840e33a9 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -103,7 +103,7 @@ 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 getSubmitButton = component => findByTestId(component, 'location-picker-submit-button'); const setLocation = (component) => { // set the location const locationPickerInstance = component.find('LocationPicker').instance(); From 0eb6050b6580dbd7ebfb73c0fe93c88d242a76c3 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 9 Mar 2022 15:04:16 +0100 Subject: [PATCH 16/16] lint Signed-off-by: Kerry Archibald --- src/components/views/location/LocationPicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 01b6b62d3c2..c906c9d1482 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -239,7 +239,7 @@ class LocationPicker extends React.Component { className='mx_LocationPicker_submitButton' disabled={!this.state.position} onClick={this.onOk}> - {_t('Share location')} + { _t('Share location') }