From 14653d13780b29f40bce77fbfdc56d0aba4d7083 Mon Sep 17 00:00:00 2001 From: Kerry Date: Mon, 21 Mar 2022 12:42:58 +0100 Subject: [PATCH] Live location share - set time limit (#8082) * add mocking helpers for platform peg Signed-off-by: Kerry Archibald * basic working live duration dropdown Signed-off-by: Kerry Archibald * add duration format utility Signed-off-by: Kerry Archibald * add duration dropdown to live location picker Signed-off-by: Kerry Archibald * adjust style to allow overflow and variable height chin Signed-off-by: Kerry Archibald * tidy comments Signed-off-by: Kerry Archibald * arrow fn change Signed-off-by: Kerry Archibald * lint Signed-off-by: Kerry Archibald --- res/css/_components.scss | 1 + .../views/location/_LiveDurationDropdown.scss | 19 +++++ res/css/views/location/_LocationPicker.scss | 18 ++--- src/DateUtils.ts | 22 ++++++ .../views/location/LiveDurationDropdown.tsx | 72 ++++++++++++++++++ .../views/location/LocationPicker.tsx | 64 +++++++++++----- src/i18n/strings/en_EN.json | 5 ++ .../location/LiveDurationDropdown-test.tsx | 75 +++++++++++++++++++ .../views/location/LocationPicker-test.tsx | 53 +++++++++++-- .../views/location/LocationShareMenu-test.tsx | 3 +- test/test-utils/index.ts | 1 + test/test-utils/platform.ts | 46 ++++++++++++ test/utils/DateUtils-test.ts | 24 +++++- 13 files changed, 366 insertions(+), 37 deletions(-) create mode 100644 res/css/components/views/location/_LiveDurationDropdown.scss create mode 100644 src/components/views/location/LiveDurationDropdown.tsx create mode 100644 test/components/views/location/LiveDurationDropdown-test.tsx create mode 100644 test/test-utils/platform.ts diff --git a/res/css/_components.scss b/res/css/_components.scss index ad4193f7e2..f982417917 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/beacon/_LeftPanelLiveShareWarning.scss"; +@import "./components/views/location/_LiveDurationDropdown.scss"; @import "./components/views/location/_LocationShareMenu.scss"; @import "./components/views/location/_MapError.scss"; @import "./components/views/location/_ShareDialogButtons.scss"; diff --git a/res/css/components/views/location/_LiveDurationDropdown.scss b/res/css/components/views/location/_LiveDurationDropdown.scss new file mode 100644 index 0000000000..9b0e39a07c --- /dev/null +++ b/res/css/components/views/location/_LiveDurationDropdown.scss @@ -0,0 +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. +*/ + +.mx_LiveDurationDropdown { + margin-bottom: $spacing-16; +} diff --git a/res/css/views/location/_LocationPicker.scss b/res/css/views/location/_LocationPicker.scss index 0b2555abf6..91c3e02bf8 100644 --- a/res/css/views/location/_LocationPicker.scss +++ b/res/css/views/location/_LocationPicker.scss @@ -19,7 +19,8 @@ limitations under the License. height: 100%; position: relative; - overflow: hidden; + display: flex; + flex-direction: column; // when there are errors loading the map // the canvas is still inserted @@ -32,8 +33,9 @@ limitations under the License. } #mx_LocationPicker_map { - height: 100%; - border-radius: 8px; + border-top-left-radius: inherit; + border-top-right-radius: inherit; + flex: 1; .maplibregl-ctrl.maplibregl-ctrl-group, .maplibregl-ctrl.maplibregl-ctrl-attrib { @@ -46,10 +48,6 @@ limitations under the License. margin-top: 50px; } - .maplibregl-ctrl-bottom-right { - bottom: 80px; - } - .maplibregl-user-location-accuracy-circle { display: none; } @@ -93,8 +91,7 @@ limitations under the License. } .mx_LocationPicker_footer { - position: absolute; - bottom: 0px; + flex: 0; width: 100%; box-sizing: border-box; padding: $spacing-16; @@ -102,6 +99,9 @@ limitations under the License. flex-direction: column; justify-content: stretch; + border-bottom-left-radius: inherit; + border-bottom-right-radius: inherit; + background-color: $header-panel-bg-color; } } diff --git a/src/DateUtils.ts b/src/DateUtils.ts index c9f33b2eee..20227354f0 100644 --- a/src/DateUtils.ts +++ b/src/DateUtils.ts @@ -201,3 +201,25 @@ export function formatRelativeTime(date: Date, showTwelveHour = false): string { return relativeDate; } } + +/** + * Formats duration in ms to human readable string + * Returns value in biggest possible unit (day, hour, min, second) + * Rounds values up until unit threshold + * ie. 23:13:57 -> 23h, 24:13:57 -> 1d, 44:56:56 -> 2d + */ +const MINUTE_MS = 60000; +const HOUR_MS = MINUTE_MS * 60; +const DAY_MS = HOUR_MS * 24; +export function formatDuration(durationMs: number): string { + if (durationMs >= DAY_MS) { + return _t('%(value)sd', { value: Math.round(durationMs / DAY_MS) }); + } + if (durationMs >= HOUR_MS) { + return _t('%(value)sh', { value: Math.round(durationMs / HOUR_MS) }); + } + if (durationMs >= MINUTE_MS) { + return _t('%(value)sm', { value: Math.round(durationMs / MINUTE_MS) }); + } + return _t('%(value)ss', { value: Math.round(durationMs / 1000) }); +} diff --git a/src/components/views/location/LiveDurationDropdown.tsx b/src/components/views/location/LiveDurationDropdown.tsx new file mode 100644 index 0000000000..3c2df61514 --- /dev/null +++ b/src/components/views/location/LiveDurationDropdown.tsx @@ -0,0 +1,72 @@ +/* +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 { formatDuration } from '../../../DateUtils'; +import { _t } from '../../../languageHandler'; +import Dropdown from '../elements/Dropdown'; + +const DURATION_MS = { + fifteenMins: 900000, + oneHour: 3600000, + eightHours: 28800000, +}; + +export const DEFAULT_DURATION_MS = DURATION_MS.fifteenMins; + +interface Props { + timeout: number; + onChange: (timeout: number) => void; +} + +const getLabel = (durationMs: number) => { + return _t('Share for %(duration)s', { duration: formatDuration(durationMs) }); +}; + +const LiveDurationDropdown: React.FC = ({ timeout, onChange }) => { + const options = Object.values(DURATION_MS).map((duration) => + ({ key: duration.toString(), duration, label: getLabel(duration) }), + ); + + // timeout is not one of our default values + // eg it was set by another client + if (!Object.values(DURATION_MS).includes(timeout)) { + options.push({ + key: timeout.toString(), duration: timeout, label: getLabel(timeout), + }); + } + + const onOptionChange = (key: string) => { + // stringified value back to number + onChange(+key); + }; + + return + { options.map(({ key, label }) => +
{ label }
, + ) } + ; +}; + +export default LiveDurationDropdown; diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 4003063795..28b472fcdb 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -35,6 +35,7 @@ import { LocationShareError } from './LocationShareErrors'; import AccessibleButton from '../elements/AccessibleButton'; import { MapError } from './MapError'; import { getUserNameColorClass } from '../../../utils/FormattingUtils'; +import LiveDurationDropdown, { DEFAULT_DURATION_MS } from './LiveDurationDropdown'; export interface ILocationPickerProps { sender: RoomMember; shareType: LocationShareType; @@ -50,6 +51,7 @@ interface IPosition { timestamp: number; } interface IState { + timeout: number; position?: IPosition; error?: LocationShareError; } @@ -70,6 +72,7 @@ class LocationPicker extends React.Component { this.state = { position: undefined, + timeout: DEFAULT_DURATION_MS, error: undefined, }; } @@ -206,10 +209,17 @@ class LocationPicker extends React.Component { } }; - private onOk = () => { - const position = this.state.position; + private onTimeoutChange = (timeout: number): void => { + this.setState({ timeout }); + }; - this.props.onChoose(position ? { uri: getGeoUri(position), timestamp: position.timestamp } : {}); + private onOk = () => { + const { timeout, position } = this.state; + + this.props.onChoose( + position ? { uri: getGeoUri(position), timestamp: position.timestamp, timeout } : { + timeout, + }); this.props.onFinished(); }; @@ -235,7 +245,12 @@ class LocationPicker extends React.Component { }
- + { this.props.shareType === LocationShareType.Live && + + } { `mx_MLocationBody_marker-${this.props.shareType}`, userColorClass, )} - id={this.getMarkerId()}> -
- { isSharingOwnLocation(this.props.shareType) ? - - : - } -
-
+ id={this.getMarkerId()} + > + { /* + maplibregl hijacks the div above to style the marker + it must be in the dom when the map is initialised + and keep a consistent class + we want to hide the marker until it is set in the case of pin drop + so hide the internal visible elements + */ } + + { !!this.marker && <> +
+ { isSharingOwnLocation(this.props.shareType) ? + + : + } +
+
+ }
); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 1c849ca7e9..ba3f4ce88d 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -106,6 +106,10 @@ "%(weekDayName)s, %(monthName)s %(day)s %(fullYear)s": "%(weekDayName)s, %(monthName)s %(day)s %(fullYear)s", "%(weekDayName)s, %(monthName)s %(day)s %(fullYear)s %(time)s": "%(weekDayName)s, %(monthName)s %(day)s %(fullYear)s %(time)s", "%(date)s at %(time)s": "%(date)s at %(time)s", + "%(value)sd": "%(value)sd", + "%(value)sh": "%(value)sh", + "%(value)sm": "%(value)sm", + "%(value)ss": "%(value)ss", "Who would you like to add to this community?": "Who would you like to add to this community?", "Warning: any person you add to a community will be publicly visible to anyone who knows the community ID": "Warning: any person you add to a community will be publicly visible to anyone who knows the community ID", "Invite new community members": "Invite new community members", @@ -2172,6 +2176,7 @@ "Submit logs": "Submit logs", "Can't load this message": "Can't load this message", "toggle event": "toggle event", + "Share for %(duration)s": "Share for %(duration)s", "Location": "Location", "Could not fetch location": "Could not fetch location", "Click to move the pin": "Click to move the pin", diff --git a/test/components/views/location/LiveDurationDropdown-test.tsx b/test/components/views/location/LiveDurationDropdown-test.tsx new file mode 100644 index 0000000000..864639db06 --- /dev/null +++ b/test/components/views/location/LiveDurationDropdown-test.tsx @@ -0,0 +1,75 @@ +/* +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 { act } from 'react-dom/test-utils'; + +import '../../../skinned-sdk'; +import LiveDurationDropdown, { DEFAULT_DURATION_MS } + from '../../../../src/components/views/location/LiveDurationDropdown'; +import { findById, mockPlatformPeg } from '../../../test-utils'; + +mockPlatformPeg({ overrideBrowserShortcuts: jest.fn().mockReturnValue(false) }); + +describe('', () => { + const defaultProps = { + timeout: DEFAULT_DURATION_MS, + onChange: jest.fn(), + }; + const getComponent = (props = {}) => + mount(); + + const getOption = (wrapper, timeout) => findById(wrapper, `live-duration__${timeout}`).at(0); + const getSelectedOption = (wrapper) => findById(wrapper, 'live-duration_value'); + const openDropdown = (wrapper) => act(() => { + wrapper.find('[role="button"]').at(0).simulate('click'); + wrapper.setProps({}); + }); + + it('renders timeout as selected option', () => { + const wrapper = getComponent(); + expect(getSelectedOption(wrapper).text()).toEqual('Share for 15m'); + }); + + it('renders non-default timeout as selected option', () => { + const timeout = 1234567; + const wrapper = getComponent({ timeout }); + expect(getSelectedOption(wrapper).text()).toEqual(`Share for 21m`); + }); + + it('renders a dropdown option for a non-default timeout value', () => { + const timeout = 1234567; + const wrapper = getComponent({ timeout }); + openDropdown(wrapper); + expect(getOption(wrapper, timeout).text()).toEqual(`Share for 21m`); + }); + + it('updates value on option selection', () => { + const onChange = jest.fn(); + const wrapper = getComponent({ onChange }); + + const ONE_HOUR = 3600000; + + openDropdown(wrapper); + + act(() => { + getOption(wrapper, ONE_HOUR).simulate('click'); + }); + + expect(onChange).toHaveBeenCalledWith(ONE_HOUR); + }); +}); diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index ca056d87a6..914821a53d 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -28,7 +28,7 @@ import LocationPicker, { getGeoUri } from "../../../../src/components/views/loca import { LocationShareType } from "../../../../src/components/views/location/shareLocation"; import MatrixClientContext from '../../../../src/contexts/MatrixClientContext'; import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; -import { findByTestId } from '../../../test-utils'; +import { findById, findByTestId, mockPlatformPeg } from '../../../test-utils'; import { findMapStyleUrl } from '../../../../src/components/views/location/findMapStyleUrl'; import { LocationShareError } from '../../../../src/components/views/location/LocationShareErrors'; @@ -36,6 +36,9 @@ jest.mock('../../../../src/components/views/location/findMapStyleUrl', () => ({ findMapStyleUrl: jest.fn().mockReturnValue('tileserver.com'), })); +// dropdown uses this +mockPlatformPeg({ overrideBrowserShortcuts: jest.fn().mockReturnValue(false) }); + describe("LocationPicker", () => { describe("getGeoUri", () => { it("Renders a URI with only lat and lon", () => { @@ -108,6 +111,7 @@ describe("LocationPicker", () => { }; const mockClient = { on: jest.fn(), + removeListener: jest.fn(), off: jest.fn(), isGuest: jest.fn(), getClientWellKnown: jest.fn(), @@ -206,7 +210,7 @@ describe("LocationPicker", () => { }); const testUserLocationShareTypes = (shareType: LocationShareType.Own | LocationShareType.Live) => { - describe(`for ${shareType} location share type`, () => { + describe('user location behaviours', () => { it('closes and displays error when geolocation errors', () => { // suppress expected error log jest.spyOn(logger, 'error').mockImplementation(() => { }); @@ -263,8 +267,42 @@ describe("LocationPicker", () => { }); }; - testUserLocationShareTypes(LocationShareType.Own); - testUserLocationShareTypes(LocationShareType.Live); + describe('for Own location share type', () => { + testUserLocationShareTypes(LocationShareType.Own); + }); + + describe('for Live location share type', () => { + const shareType = LocationShareType.Live; + testUserLocationShareTypes(shareType); + + const getOption = (wrapper, timeout) => findById(wrapper, `live-duration__${timeout}`).at(0); + const getDropdown = wrapper => findByTestId(wrapper, 'live-duration-dropdown'); + const getSelectedOption = (wrapper) => findById(wrapper, 'live-duration_value'); + + const openDropdown = (wrapper) => act(() => { + const dropdown = getDropdown(wrapper); + dropdown.find('[role="button"]').at(0).simulate('click'); + wrapper.setProps({}); + }); + + it('renders live duration dropdown with default option', () => { + const wrapper = getComponent({ shareType }); + expect(getSelectedOption(getDropdown(wrapper)).text()).toEqual('Share for 15m'); + }); + + it('updates selected duration', () => { + const wrapper = getComponent({ shareType }); + + openDropdown(wrapper); + const dropdown = getDropdown(wrapper); + act(() => { + getOption(dropdown, 3600000).simulate('click'); + }); + + // value updated + expect(getSelectedOption(getDropdown(wrapper)).text()).toEqual('Share for 1h'); + }); + }); describe('for Pin drop location share type', () => { const shareType = LocationShareType.Pin; @@ -298,14 +336,15 @@ describe("LocationPicker", () => { }); it('does not set position on geolocate event', () => { - getComponent({ shareType }); + mocked(maplibregl.Marker).mockClear(); + const wrapper = getComponent({ shareType }); act(() => { // @ts-ignore mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition); }); - // marker added - expect(maplibregl.Marker).not.toHaveBeenCalled(); + // marker not added + expect(wrapper.find('.mx_MLocationBody_markerBorder').length).toBeFalsy(); }); it('sets position on click event', () => { diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 1afb28f183..1fa19201a1 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -33,6 +33,7 @@ import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; import { LocationShareType } from '../../../../src/components/views/location/shareLocation'; import { findByTagAndTestId, flushPromises } from '../../../test-utils'; import Modal from '../../../../src/Modal'; +import { DEFAULT_DURATION_MS } from '../../../../src/components/views/location/LiveDurationDropdown'; jest.mock('../../../../src/components/views/location/findMapStyleUrl', () => ({ findMapStyleUrl: jest.fn().mockReturnValue('test'), @@ -316,7 +317,7 @@ describe('', () => { expect(eventContent).toEqual(expect.objectContaining({ [M_BEACON_INFO.name]: { // default timeout - timeout: 300000, + timeout: DEFAULT_DURATION_MS, description: `Ernie's live location`, live: true, }, diff --git a/test/test-utils/index.ts b/test/test-utils/index.ts index 7f6b69f1d7..c6e4cbd182 100644 --- a/test/test-utils/index.ts +++ b/test/test-utils/index.ts @@ -1,5 +1,6 @@ export * from './beacon'; export * from './client'; +export * from './platform'; export * from './test-utils'; export * from './wrappers'; export * from './utilities'; diff --git a/test/test-utils/platform.ts b/test/test-utils/platform.ts new file mode 100644 index 0000000000..7b82a67a56 --- /dev/null +++ b/test/test-utils/platform.ts @@ -0,0 +1,46 @@ +/* +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 { MethodKeysOf, mocked, MockedObject } from "jest-mock"; + +import BasePlatform from "../../src/BasePlatform"; +import PlatformPeg from "../../src/PlatformPeg"; + +// doesn't implement abstract +// @ts-ignore +class MockPlatform extends BasePlatform { + constructor(platformMocks: Partial, unknown>>) { + super(); + Object.assign(this, platformMocks); + } +} +/** + * Mock Platform Peg + * Creates a mock BasePlatform class + * spys on PlatformPeg.get and returns mock platform + * @returns MockPlatform instance + */ +export const mockPlatformPeg = ( + platformMocks: Partial, unknown>> = {}, +): MockedObject => { + const mockPlatform = new MockPlatform(platformMocks); + jest.spyOn(PlatformPeg, 'get').mockReturnValue(mockPlatform); + return mocked(mockPlatform); +}; + +export const unmockPlatformPeg = () => { + jest.spyOn(PlatformPeg, 'get').mockRestore(); +}; diff --git a/test/utils/DateUtils-test.ts b/test/utils/DateUtils-test.ts index 78b52d3417..7804fd6e9c 100644 --- a/test/utils/DateUtils-test.ts +++ b/test/utils/DateUtils-test.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { formatSeconds, formatRelativeTime } from "../../src/DateUtils"; +import { formatSeconds, formatRelativeTime, formatDuration } from "../../src/DateUtils"; describe("formatSeconds", () => { it("correctly formats time with hours", () => { @@ -70,3 +70,25 @@ describe("formatRelativeTime", () => { expect(formatRelativeTime(date, true)).toBe("Oct 31, 2020"); }); }); + +describe('formatDuration()', () => { + type TestCase = [string, string, number]; + + const MINUTE_MS = 60000; + const HOUR_MS = MINUTE_MS * 60; + + it.each([ + ['rounds up to nearest day when more than 24h - 40 hours', '2d', 40 * HOUR_MS], + ['rounds down to nearest day when more than 24h - 26 hours', '1d', 26 * HOUR_MS], + ['24 hours', '1d', 24 * HOUR_MS], + ['rounds to nearest hour when less than 24h - 23h', '23h', 23 * HOUR_MS], + ['rounds to nearest hour when less than 24h - 6h and 10min', '6h', 6 * HOUR_MS + 10 * MINUTE_MS], + ['rounds to nearest hours when less than 24h', '2h', 2 * HOUR_MS + 124234], + ['rounds to nearest minute when less than 1h - 59 minutes', '59m', 59 * MINUTE_MS], + ['rounds to nearest minute when less than 1h - 1 minute', '1m', MINUTE_MS], + ['rounds to nearest second when less than 1min - 59 seconds', '59s', 59000], + ['rounds to 0 seconds when less than a second - 123ms', '0s', 123], + ])('%s formats to %s', (_description, expectedResult, input) => { + expect(formatDuration(input)).toEqual(expectedResult); + }); +});