From 1175226bcb4045f8f302dd59c41ed29f90fd0ae1 Mon Sep 17 00:00:00 2001 From: Kerry Date: Thu, 31 Mar 2022 10:57:12 +0200 Subject: [PATCH] Live location sharing - display wire error in room (#8198) * expose wire errors in more useful way * add wire error state to room live share warning bar Signed-off-by: Kerry Archibald * stylelint Signed-off-by: Kerry Archibald * add types to getLabel helper Signed-off-by: Kerry Archibald --- .../views/beacon/_RoomLiveShareWarning.scss | 10 ++ .../views/beacon/RoomLiveShareWarning.tsx | 133 +++++++++++++----- src/i18n/strings/en_EN.json | 2 + src/stores/OwnBeaconStore.ts | 22 ++- .../beacon/RoomLiveShareWarning-test.tsx | 87 +++++++++++- .../RoomLiveShareWarning-test.tsx.snap | 85 ++++++++++- test/stores/OwnBeaconStore-test.ts | 15 +- 7 files changed, 296 insertions(+), 58 deletions(-) diff --git a/res/css/components/views/beacon/_RoomLiveShareWarning.scss b/res/css/components/views/beacon/_RoomLiveShareWarning.scss index c0d5ea47fe..7404f88aea 100644 --- a/res/css/components/views/beacon/_RoomLiveShareWarning.scss +++ b/res/css/components/views/beacon/_RoomLiveShareWarning.scss @@ -48,3 +48,13 @@ limitations under the License. .mx_RoomLiveShareWarning_spinner { margin-right: $spacing-16; } + +.mx_RoomLiveShareWarning_closeButton { + @mixin ButtonResetDefault; + margin-left: $spacing-16; +} + +.mx_RoomLiveShareWarning_closeButtonIcon { + height: $font-18px; + padding: $spacing-4; +} diff --git a/src/components/views/beacon/RoomLiveShareWarning.tsx b/src/components/views/beacon/RoomLiveShareWarning.tsx index 0f6b0ee809..d51c22f644 100644 --- a/src/components/views/beacon/RoomLiveShareWarning.tsx +++ b/src/components/views/beacon/RoomLiveShareWarning.tsx @@ -18,19 +18,16 @@ import React, { useCallback, useEffect, useState } from 'react'; import classNames from 'classnames'; import { Room, Beacon } from 'matrix-js-sdk/src/matrix'; +import { formatDuration } from '../../../DateUtils'; import { _t } from '../../../languageHandler'; import { useEventEmitterState } from '../../../hooks/useEventEmitter'; -import { OwnBeaconStore, OwnBeaconStoreEvent } from '../../../stores/OwnBeaconStore'; -import AccessibleButton from '../elements/AccessibleButton'; -import StyledLiveBeaconIcon from './StyledLiveBeaconIcon'; -import { formatDuration } from '../../../DateUtils'; -import { getBeaconMsUntilExpiry, sortBeaconsByLatestExpiry } from '../../../utils/beacon'; -import Spinner from '../elements/Spinner'; import { useInterval } from '../../../hooks/useTimeout'; - -interface Props { - roomId: Room['roomId']; -} +import { OwnBeaconStore, OwnBeaconStoreEvent } from '../../../stores/OwnBeaconStore'; +import { getBeaconMsUntilExpiry, sortBeaconsByLatestExpiry } from '../../../utils/beacon'; +import AccessibleButton from '../elements/AccessibleButton'; +import Spinner from '../elements/Spinner'; +import StyledLiveBeaconIcon from './StyledLiveBeaconIcon'; +import { Icon as CloseIcon } from '../../../../res/img/image-view/close.svg'; const MINUTE_MS = 60000; const HOUR_MS = MINUTE_MS * 60; @@ -72,24 +69,20 @@ const useMsRemaining = (beacon: Beacon): number => { type LiveBeaconsState = { beacon?: Beacon; onStopSharing?: () => void; + onResetWireError?: () => void; stoppingInProgress?: boolean; hasStopSharingError?: boolean; + hasWireError?: boolean; }; -const useLiveBeacons = (roomId: Room['roomId']): LiveBeaconsState => { +const useLiveBeacons = (liveBeaconIds: string[], roomId: string): LiveBeaconsState => { const [stoppingInProgress, setStoppingInProgress] = useState(false); const [error, setError] = useState(); - // do we have an active geolocation.watchPosition - const isMonitoringLiveLocation = useEventEmitterState( + const hasWireError = useEventEmitterState( OwnBeaconStore.instance, - OwnBeaconStoreEvent.MonitoringLivePosition, - () => OwnBeaconStore.instance.isMonitoringLiveLocation, - ); - - const liveBeaconIds = useEventEmitterState( - OwnBeaconStore.instance, - OwnBeaconStoreEvent.LivenessChange, - () => OwnBeaconStore.instance.getLiveBeaconIds(roomId), + OwnBeaconStoreEvent.WireError, + () => + OwnBeaconStore.instance.hasWireErrors(roomId), ); // reset stopping in progress on change in live ids @@ -98,10 +91,6 @@ const useLiveBeacons = (roomId: Room['roomId']): LiveBeaconsState => { setError(undefined); }, [liveBeaconIds]); - if (!isMonitoringLiveLocation || !liveBeaconIds?.length) { - return {}; - } - // select the beacon with latest expiry to display expiry time const beacon = liveBeaconIds.map(beaconId => OwnBeaconStore.instance.getBeaconById(beaconId)) .sort(sortBeaconsByLatestExpiry) @@ -120,7 +109,18 @@ const useLiveBeacons = (roomId: Room['roomId']): LiveBeaconsState => { } }; - return { onStopSharing, beacon, stoppingInProgress, hasStopSharingError: !!error }; + const onResetWireError = () => { + liveBeaconIds.map(beaconId => OwnBeaconStore.instance.resetWireError(beaconId)); + }; + + return { + onStopSharing, + onResetWireError, + beacon, + stoppingInProgress, + hasWireError, + hasStopSharingError: !!error, + }; }; const LiveTimeRemaining: React.FC<{ beacon: Beacon }> = ({ beacon }) => { @@ -135,44 +135,103 @@ const LiveTimeRemaining: React.FC<{ beacon: Beacon }> = ({ beacon }) => { >{ liveTimeRemaining }; }; -const RoomLiveShareWarning: React.FC = ({ roomId }) => { +const getLabel = (hasWireError: boolean, hasStopSharingError: boolean): string => { + if (hasWireError) { + return _t('An error occured whilst sharing your live location, please try again'); + } + if (hasStopSharingError) { + return _t('An error occurred while stopping your live location, please try again'); + } + return _t('You are sharing your live location'); +}; + +interface RoomLiveShareWarningInnerProps { + liveBeaconIds: string[]; + roomId: Room['roomId']; +} +const RoomLiveShareWarningInner: React.FC = ({ liveBeaconIds, roomId }) => { const { onStopSharing, + onResetWireError, beacon, stoppingInProgress, hasStopSharingError, - } = useLiveBeacons(roomId); + hasWireError, + } = useLiveBeacons(liveBeaconIds, roomId); if (!beacon) { return null; } + const hasError = hasStopSharingError || hasWireError; + + const onButtonClick = () => { + if (hasWireError) { + onResetWireError(); + } else { + onStopSharing(); + } + }; + return
- + + - { hasStopSharingError ? - _t('An error occurred while stopping your live location, please try again') : - _t('You are sharing your live location') - } + { getLabel(hasWireError, hasStopSharingError) } { stoppingInProgress && } - { !stoppingInProgress && !hasStopSharingError && } + { !stoppingInProgress && !hasError && } - { hasStopSharingError ? _t('Retry') : _t('Stop sharing') } + { hasError ? _t('Retry') : _t('Stop sharing') } + { hasWireError && + + }
; }; +interface Props { + roomId: Room['roomId']; +} +const RoomLiveShareWarning: React.FC = ({ roomId }) => { + // do we have an active geolocation.watchPosition + const isMonitoringLiveLocation = useEventEmitterState( + OwnBeaconStore.instance, + OwnBeaconStoreEvent.MonitoringLivePosition, + () => OwnBeaconStore.instance.isMonitoringLiveLocation, + ); + + const liveBeaconIds = useEventEmitterState( + OwnBeaconStore.instance, + OwnBeaconStoreEvent.LivenessChange, + () => OwnBeaconStore.instance.getLiveBeaconIds(roomId), + ); + + if (!isMonitoringLiveLocation || !liveBeaconIds.length) { + return null; + } + + // split into outer/inner to avoid watching various parts of live beacon state + // when there are none + return ; +}; + export default RoomLiveShareWarning; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index eee6e7075c..32d1500377 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2898,8 +2898,10 @@ "Join the beta": "Join the beta", "You are sharing your live location": "You are sharing your live location", "%(timeRemaining)s left": "%(timeRemaining)s left", + "An error occured whilst sharing your live location, please try again": "An error occured whilst sharing your live location, please try again", "An error occurred while stopping your live location, please try again": "An error occurred while stopping your live location, please try again", "Stop sharing": "Stop sharing", + "Stop sharing and close": "Stop sharing and close", "Avatar": "Avatar", "This room is public": "This room is public", "Away": "Away", diff --git a/src/stores/OwnBeaconStore.ts b/src/stores/OwnBeaconStore.ts index e4c0d46a6d..3bc6e78469 100644 --- a/src/stores/OwnBeaconStore.ts +++ b/src/stores/OwnBeaconStore.ts @@ -130,16 +130,24 @@ export class OwnBeaconStore extends AsyncStoreWithClient { return !!this.getLiveBeaconIds(roomId).length; } + /** + * Some live beacon has a wire error + * Optionally filter by room + */ + public hasWireErrors(roomId?: string): boolean { + return this.getLiveBeaconIds(roomId).some(this.beaconHasWireError); + } + /** * If a beacon has failed to publish position * past the allowed consecutive failure count (BAIL_AFTER_CONSECUTIVE_ERROR_COUNT) * Then consider it to have an error */ - public hasWireError(beaconId: string): boolean { + public beaconHasWireError = (beaconId: string): boolean => { return this.beaconWireErrorCounts.get(beaconId) >= BAIL_AFTER_CONSECUTIVE_ERROR_COUNT; - } + }; - public resetWireError(beaconId: string): void { + public resetWireError = (beaconId: string): void => { this.incrementBeaconWireErrorCount(beaconId, false); // always publish to all live beacons together @@ -147,7 +155,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient { // to keep lastPublishedTimestamp simple // and extra published locations don't hurt this.publishCurrentLocationToBeacons(); - } + }; public getLiveBeaconIds(roomId?: string): string[] { if (!roomId) { @@ -230,7 +238,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient { * Live beacon ids that do not have wire errors */ private get healthyLiveBeaconIds() { - return this.liveBeaconIds.filter(beaconId => !this.hasWireError(beaconId)); + return this.liveBeaconIds.filter(beaconId => !this.beaconHasWireError(beaconId)); } private initialiseBeaconState = () => { @@ -458,7 +466,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient { * - emit if beacon error count crossed threshold */ private incrementBeaconWireErrorCount = (beaconId: string, isError: boolean): void => { - const hadError = this.hasWireError(beaconId); + const hadError = this.beaconHasWireError(beaconId); if (isError) { // increment error count @@ -471,7 +479,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient { this.beaconWireErrorCounts.delete(beaconId); } - if (this.hasWireError(beaconId) !== hadError) { + if (this.beaconHasWireError(beaconId) !== hadError) { this.emit(OwnBeaconStoreEvent.WireError, beaconId); } }; diff --git a/test/components/views/beacon/RoomLiveShareWarning-test.tsx b/test/components/views/beacon/RoomLiveShareWarning-test.tsx index d549e9a51e..97cc953d52 100644 --- a/test/components/views/beacon/RoomLiveShareWarning-test.tsx +++ b/test/components/views/beacon/RoomLiveShareWarning-test.tsx @@ -101,6 +101,7 @@ describe('', () => { }); afterEach(async () => { + jest.spyOn(OwnBeaconStore.instance, 'hasWireErrors').mockRestore(); await resetAsyncStoreWithClient(OwnBeaconStore.instance); }); @@ -238,13 +239,13 @@ describe('', () => { const component = getComponent({ roomId: room2Id }); act(() => { - findByTestId(component, 'room-live-share-stop-sharing').at(0).simulate('click'); + findByTestId(component, 'room-live-share-primary-button').at(0).simulate('click'); component.setProps({}); }); expect(mockClient.unstable_setLiveBeacon).toHaveBeenCalledTimes(2); expect(component.find('Spinner').length).toBeTruthy(); - expect(findByTestId(component, 'room-live-share-stop-sharing').at(0).props().disabled).toBeTruthy(); + expect(findByTestId(component, 'room-live-share-primary-button').at(0).props().disabled).toBeTruthy(); }); it('displays error when stop sharing fails', async () => { @@ -256,7 +257,7 @@ describe('', () => { .mockResolvedValue(({ event_id: '1' })); await act(async () => { - findByTestId(component, 'room-live-share-stop-sharing').at(0).simulate('click'); + findByTestId(component, 'room-live-share-primary-button').at(0).simulate('click'); await flushPromisesWithFakeTimers(); }); component.setProps({}); @@ -264,7 +265,7 @@ describe('', () => { expect(component.html()).toMatchSnapshot(); act(() => { - findByTestId(component, 'room-live-share-stop-sharing').at(0).simulate('click'); + findByTestId(component, 'room-live-share-primary-button').at(0).simulate('click'); component.setProps({}); }); @@ -277,7 +278,7 @@ describe('', () => { // stop the beacon act(() => { - findByTestId(component, 'room-live-share-stop-sharing').at(0).simulate('click'); + findByTestId(component, 'room-live-share-primary-button').at(0).simulate('click'); }); // time travel until room1Beacon1 is expired act(() => { @@ -293,9 +294,83 @@ describe('', () => { }); // button not disabled and expiry time shown - expect(findByTestId(component, 'room-live-share-stop-sharing').at(0).props().disabled).toBeFalsy(); + expect(findByTestId(component, 'room-live-share-primary-button').at(0).props().disabled).toBeFalsy(); expect(findByTestId(component, 'room-live-share-expiry').text()).toEqual('1h left'); }); }); + + describe('with wire errors', () => { + it('displays wire error when mounted with wire errors', async () => { + const hasWireErrorsSpy = jest.spyOn(OwnBeaconStore.instance, 'hasWireErrors').mockReturnValue(true); + const component = getComponent({ roomId: room2Id }); + + expect(component).toMatchSnapshot(); + expect(hasWireErrorsSpy).toHaveBeenCalledWith(room2Id); + }); + + it('displays wire error when wireError event is emitted and beacons have errors', async () => { + const hasWireErrorsSpy = jest.spyOn(OwnBeaconStore.instance, 'hasWireErrors').mockReturnValue(false); + const component = getComponent({ roomId: room2Id }); + + // update mock and emit event + act(() => { + hasWireErrorsSpy.mockReturnValue(true); + OwnBeaconStore.instance.emit(OwnBeaconStoreEvent.WireError, room2Beacon1.getType()); + }); + component.setProps({}); + + // renders wire error ui + expect(component.find('.mx_RoomLiveShareWarning_label').text()).toEqual( + 'An error occured whilst sharing your live location, please try again', + ); + expect(findByTestId(component, 'room-live-share-wire-error-close-button').length).toBeTruthy(); + }); + + it('stops displaying wire error when errors are cleared', async () => { + const hasWireErrorsSpy = jest.spyOn(OwnBeaconStore.instance, 'hasWireErrors').mockReturnValue(true); + const component = getComponent({ roomId: room2Id }); + + // update mock and emit event + act(() => { + hasWireErrorsSpy.mockReturnValue(false); + OwnBeaconStore.instance.emit(OwnBeaconStoreEvent.WireError, room2Beacon1.getType()); + }); + component.setProps({}); + + // renders error-free ui + expect(component.find('.mx_RoomLiveShareWarning_label').text()).toEqual( + 'You are sharing your live location', + ); + expect(findByTestId(component, 'room-live-share-wire-error-close-button').length).toBeFalsy(); + }); + + it('clicking retry button resets wire errors', async () => { + jest.spyOn(OwnBeaconStore.instance, 'hasWireErrors').mockReturnValue(true); + const resetErrorSpy = jest.spyOn(OwnBeaconStore.instance, 'resetWireError'); + + const component = getComponent({ roomId: room2Id }); + + act(() => { + findByTestId(component, 'room-live-share-primary-button').at(0).simulate('click'); + }); + + expect(resetErrorSpy).toHaveBeenCalledWith(room2Beacon1.getType()); + expect(resetErrorSpy).toHaveBeenCalledWith(room2Beacon2.getType()); + }); + + it('clicking close button stops beacons', async () => { + jest.spyOn(OwnBeaconStore.instance, 'hasWireErrors').mockReturnValue(true); + const stopBeaconSpy = jest.spyOn(OwnBeaconStore.instance, 'stopBeacon'); + + const component = getComponent({ roomId: room2Id }); + + act(() => { + findByTestId(component, 'room-live-share-wire-error-close-button').at(0).simulate('click'); + }); + + expect(stopBeaconSpy).toHaveBeenCalledWith(room2Beacon1.getType()); + expect(stopBeaconSpy).toHaveBeenCalledWith(room2Beacon2.getType()); + }); + }); }); }); diff --git a/test/components/views/beacon/__snapshots__/RoomLiveShareWarning-test.tsx.snap b/test/components/views/beacon/__snapshots__/RoomLiveShareWarning-test.tsx.snap index 0f76512445..8ae076a2a1 100644 --- a/test/components/views/beacon/__snapshots__/RoomLiveShareWarning-test.tsx.snap +++ b/test/components/views/beacon/__snapshots__/RoomLiveShareWarning-test.tsx.snap @@ -1,7 +1,86 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` when user has live beacons and geolocation is available renders correctly with one live beacon in room 1`] = `"
You are sharing your live location1h left
"`; +exports[` when user has live beacons and geolocation is available renders correctly with one live beacon in room 1`] = `"
You are sharing your live location1h left
"`; -exports[` when user has live beacons and geolocation is available renders correctly with two live beacons in room 1`] = `"
You are sharing your live location12h left
"`; +exports[` when user has live beacons and geolocation is available renders correctly with two live beacons in room 1`] = `"
You are sharing your live location12h left
"`; -exports[` when user has live beacons and geolocation is available stopping beacons displays error when stop sharing fails 1`] = `"
An error occurred while stopping your live location, please try again
"`; +exports[` when user has live beacons and geolocation is available stopping beacons displays error when stop sharing fails 1`] = `"
An error occurred while stopping your live location, please try again
"`; + +exports[` when user has live beacons and geolocation is available with wire errors displays wire error when mounted with wire errors 1`] = ` + + +
+ +
+ + + An error occured whilst sharing your live location, please try again + + + + + + + +
+ + +`; diff --git a/test/stores/OwnBeaconStore-test.ts b/test/stores/OwnBeaconStore-test.ts index 3c924594f5..57e66d636b 100644 --- a/test/stores/OwnBeaconStore-test.ts +++ b/test/stores/OwnBeaconStore-test.ts @@ -863,7 +863,8 @@ describe('OwnBeaconStore', () => { // called for each position from watchPosition expect(mockClient.sendEvent).toHaveBeenCalledTimes(5); - expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(false); + expect(store.beaconHasWireError(alicesRoom1BeaconInfo.getType())).toBe(false); + expect(store.hasWireErrors()).toBe(false); }); it('continues publishing positions when a beacon fails intermittently', async () => { @@ -889,7 +890,8 @@ describe('OwnBeaconStore', () => { // called for each position from watchPosition expect(mockClient.sendEvent).toHaveBeenCalledTimes(5); - expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(false); + expect(store.beaconHasWireError(alicesRoom1BeaconInfo.getType())).toBe(false); + expect(store.hasWireErrors()).toBe(false); expect(emitSpy).not.toHaveBeenCalledWith( OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(), ); @@ -911,7 +913,8 @@ describe('OwnBeaconStore', () => { // only two allowed failures expect(mockClient.sendEvent).toHaveBeenCalledTimes(2); - expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(true); + expect(store.beaconHasWireError(alicesRoom1BeaconInfo.getType())).toBe(true); + expect(store.hasWireErrors()).toBe(true); expect(emitSpy).toHaveBeenCalledWith( OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(), ); @@ -933,7 +936,9 @@ describe('OwnBeaconStore', () => { // only two allowed failures expect(mockClient.sendEvent).toHaveBeenCalledTimes(2); - expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(true); + expect(store.beaconHasWireError(alicesRoom1BeaconInfo.getType())).toBe(true); + expect(store.hasWireErrors()).toBe(true); + expect(store.hasWireErrors(room1Id)).toBe(true); expect(emitSpy).toHaveBeenCalledWith( OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(), ); @@ -942,7 +947,7 @@ describe('OwnBeaconStore', () => { emitSpy.mockClear(); store.resetWireError(alicesRoom1BeaconInfo.getType()); - expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(false); + expect(store.beaconHasWireError(alicesRoom1BeaconInfo.getType())).toBe(false); // 2 more positions from watchPosition in this period await advanceAndFlushPromises(10000);