diff --git a/src/components/views/messages/MImageBody.tsx b/src/components/views/messages/MImageBody.tsx index a22137b5a2e..45f18f9642c 100644 --- a/src/components/views/messages/MImageBody.tsx +++ b/src/components/views/messages/MImageBody.tsx @@ -17,11 +17,10 @@ limitations under the License. import React, { ComponentProps, createRef } from 'react'; import { Blurhash } from "react-blurhash"; -import { SyncState } from 'matrix-js-sdk/src/sync'; import classNames from 'classnames'; import { CSSTransition, SwitchTransition } from 'react-transition-group'; import { logger } from "matrix-js-sdk/src/logger"; -import { ClientEvent } from "matrix-js-sdk/src/client"; +import { ClientEvent, ClientEventHandlerMap } from "matrix-js-sdk/src/client"; import MFileBody from './MFileBody'; import Modal from '../../../Modal'; @@ -38,6 +37,7 @@ import { MatrixClientPeg } from '../../../MatrixClientPeg'; import RoomContext, { TimelineRenderingType } from "../../../contexts/RoomContext"; import { blobIsAnimated, mayBeAnimated } from '../../../utils/Image'; import { presentableTextForFile } from "../../../utils/FileUtils"; +import { createReconnectedListener } from '../../../utils/connection'; enum Placeholder { NoImage, @@ -68,10 +68,13 @@ export default class MImageBody extends React.Component { private image = createRef(); private timeout?: number; private sizeWatcher: string; + private reconnectedListener: ClientEventHandlerMap[ClientEvent.Sync]; constructor(props: IBodyProps) { super(props); + this.reconnectedListener = createReconnectedListener(this.clearError); + this.state = { imgError: false, imgLoaded: false, @@ -81,20 +84,6 @@ export default class MImageBody extends React.Component { }; } - // FIXME: factor this out and apply it to MVideoBody and MAudioBody too! - private onClientSync = (syncState: SyncState, prevState: SyncState): void => { - if (this.unmounted) return; - // Consider the client reconnected if there is no error with syncing. - // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP. - const reconnected = syncState !== SyncState.Error && prevState !== syncState; - if (reconnected && this.state.imgError) { - // Load the image again - this.setState({ - imgError: false, - }); - } - }; - protected showImage(): void { localStorage.setItem("mx_ShowImage_" + this.props.mxEvent.getId(), "true"); this.setState({ showImage: true }); @@ -159,11 +148,17 @@ export default class MImageBody extends React.Component { imgElement.src = this.state.thumbUrl ?? this.state.contentUrl; }; + private clearError = () => { + MatrixClientPeg.get().off(ClientEvent.Sync, this.reconnectedListener); + this.setState({ imgError: false }); + }; + private onImageError = (): void => { this.clearBlurhashTimeout(); this.setState({ imgError: true, }); + MatrixClientPeg.get().on(ClientEvent.Sync, this.reconnectedListener); }; private onImageLoad = (): void => { @@ -317,7 +312,6 @@ export default class MImageBody extends React.Component { componentDidMount() { this.unmounted = false; - MatrixClientPeg.get().on(ClientEvent.Sync, this.onClientSync); const showImage = this.state.showImage || localStorage.getItem("mx_ShowImage_" + this.props.mxEvent.getId()) === "true"; @@ -347,7 +341,7 @@ export default class MImageBody extends React.Component { componentWillUnmount() { this.unmounted = true; - MatrixClientPeg.get().removeListener(ClientEvent.Sync, this.onClientSync); + MatrixClientPeg.get().off(ClientEvent.Sync, this.reconnectedListener); this.clearBlurhashTimeout(); SettingsStore.unwatchSetting(this.sizeWatcher); if (this.state.isAnimated && this.state.thumbUrl) { diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index d841133ed86..2aa25c29ff6 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -17,6 +17,7 @@ limitations under the License. import React from 'react'; import { MatrixEvent } from 'matrix-js-sdk/src/models/event'; import { randomString } from 'matrix-js-sdk/src/randomstring'; +import { ClientEvent, ClientEventHandlerMap } from 'matrix-js-sdk/src/matrix'; import { _t } from '../../../languageHandler'; import Modal from '../../../Modal'; @@ -33,6 +34,7 @@ import LocationViewDialog from '../location/LocationViewDialog'; import Map from '../location/Map'; import SmartMarker from '../location/SmartMarker'; import { IBodyProps } from "./IBodyProps"; +import { createReconnectedListener } from '../../../utils/connection'; interface IState { error: Error; @@ -42,6 +44,7 @@ export default class MLocationBody extends React.Component { public static contextType = MatrixClientContext; public context!: React.ContextType; private mapId: string; + private reconnectedListener: ClientEventHandlerMap[ClientEvent.Sync]; constructor(props: IBodyProps) { super(props); @@ -51,6 +54,8 @@ export default class MLocationBody extends React.Component { const idSuffix = `${props.mxEvent.getId()}_${randomString(8)}`; this.mapId = `mx_MLocationBody_${idSuffix}`; + this.reconnectedListener = createReconnectedListener(this.clearError); + this.state = { error: undefined, }; @@ -69,10 +74,20 @@ export default class MLocationBody extends React.Component { ); }; - private onError = (error) => { + private clearError = () => { + this.context.off(ClientEvent.Sync, this.reconnectedListener); + this.setState({ error: undefined }); + }; + + private onError = (error: Error) => { this.setState({ error }); + this.context.on(ClientEvent.Sync, this.reconnectedListener); }; + componentWillUnmount(): void { + this.context.off(ClientEvent.Sync, this.reconnectedListener); + } + render(): React.ReactElement { return this.state.error ? : diff --git a/src/utils/connection.ts b/src/utils/connection.ts new file mode 100644 index 00000000000..55eed17d478 --- /dev/null +++ b/src/utils/connection.ts @@ -0,0 +1,32 @@ +/* +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 { ClientEvent, ClientEventHandlerMap } from "matrix-js-sdk/src/matrix"; +import { SyncState } from "matrix-js-sdk/src/sync"; + +/** + * Creates a MatrixClient event listener function that can be used to get notified about reconnects. + * @param callback The callback to be called on reconnect + */ +export const createReconnectedListener = (callback: () => void): ClientEventHandlerMap[ClientEvent.Sync] => { + return (syncState: SyncState, prevState: SyncState) => { + if (syncState !== SyncState.Error && prevState !== syncState) { + // Consider the client reconnected if there is no error with syncing. + // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP. + callback(); + } + }; +}; diff --git a/test/components/views/messages/MLocationBody-test.tsx b/test/components/views/messages/MLocationBody-test.tsx index 5fe5a213808..81d5daaa8be 100644 --- a/test/components/views/messages/MLocationBody-test.tsx +++ b/test/components/views/messages/MLocationBody-test.tsx @@ -17,10 +17,11 @@ limitations under the License. import React from 'react'; import { mount } from "enzyme"; import { LocationAssetType } from "matrix-js-sdk/src/@types/location"; -import { RoomMember } from 'matrix-js-sdk/src/matrix'; +import { ClientEvent, RoomMember } from 'matrix-js-sdk/src/matrix'; import maplibregl from 'maplibre-gl'; import { logger } from 'matrix-js-sdk/src/logger'; import { act } from 'react-dom/test-utils'; +import { SyncState } from 'matrix-js-sdk/src/sync'; import MLocationBody from "../../../../src/components/views/messages/MLocationBody"; import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; @@ -56,6 +57,19 @@ describe("MLocationBody", () => { wrappingComponent: MatrixClientContext.Provider, wrappingComponentProps: { value: mockClient }, }); + const getMapErrorComponent = () => { + const mockMap = new maplibregl.Map(); + mockClient.getClientWellKnown.mockReturnValue({ + [TILE_SERVER_WK_KEY.name]: { map_style_url: 'bad-tile-server.com' }, + }); + const component = getComponent(); + + // simulate error initialising map in maplibregl + // @ts-ignore + mockMap.emit('error', { status: 404 }); + + return component; + }; beforeAll(() => { maplibregl.AttributionControl = jest.fn(); @@ -86,18 +100,17 @@ describe("MLocationBody", () => { }); it('displays correct fallback content when map_style_url is misconfigured', () => { - const mockMap = new maplibregl.Map(); - mockClient.getClientWellKnown.mockReturnValue({ - [TILE_SERVER_WK_KEY.name]: { map_style_url: 'bad-tile-server.com' }, - }); - const component = getComponent(); - - // simulate error initialising map in maplibregl - // @ts-ignore - mockMap.emit('error', { status: 404 }); + const component = getMapErrorComponent(); component.setProps({}); expect(component.find(".mx_EventTile_body")).toMatchSnapshot(); }); + + it('should clear the error on reconnect', () => { + const component = getMapErrorComponent(); + expect((component.state() as React.ComponentState).error).toBeDefined(); + mockClient.emit(ClientEvent.Sync, SyncState.Reconnecting, SyncState.Error); + expect((component.state() as React.ComponentState).error).toBeUndefined(); + }); }); describe('without error', () => { diff --git a/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap b/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap index 6123b3b7b46..9f21199585c 100644 --- a/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap +++ b/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap @@ -120,6 +120,10 @@ exports[`MLocationBody without error renders map correctly 1`] = "error": Array [ [Function], [Function], + [Function], + [Function], + [Function], + [Function], ], }, "_eventsCount": 1, @@ -130,12 +134,20 @@ exports[`MLocationBody without error renders map correctly 1`] = mockConstructor {}, "top-right", ], + Array [ + mockConstructor {}, + "top-right", + ], ], "results": Array [ Object { "type": "return", "value": undefined, }, + Object { + "type": "return", + "value": undefined, + }, ], }, "fitBounds": [MockFunction], @@ -148,12 +160,22 @@ exports[`MLocationBody without error renders map correctly 1`] = "lon": -0.1276, }, ], + Array [ + Object { + "lat": 51.5076, + "lon": -0.1276, + }, + ], ], "results": Array [ Object { "type": "return", "value": undefined, }, + Object { + "type": "return", + "value": undefined, + }, ], }, "setStyle": [MockFunction], diff --git a/test/utils/connection-test.ts b/test/utils/connection-test.ts new file mode 100644 index 00000000000..70b8a119417 --- /dev/null +++ b/test/utils/connection-test.ts @@ -0,0 +1,52 @@ +/* +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 { ClientEvent, ClientEventHandlerMap } from "matrix-js-sdk/src/matrix"; +import { SyncState } from "matrix-js-sdk/src/sync"; + +import { createReconnectedListener } from "../../src/utils/connection"; + +describe("createReconnectedListener", () => { + let reconnectedListener: ClientEventHandlerMap[ClientEvent.Sync]; + let onReconnect: jest.Mock; + + beforeEach(() => { + onReconnect = jest.fn(); + reconnectedListener = createReconnectedListener(onReconnect); + }); + + [ + [SyncState.Prepared, SyncState.Syncing], + [SyncState.Syncing, SyncState.Reconnecting], + [SyncState.Reconnecting, SyncState.Syncing], + ].forEach(([from, to]) => { + it(`should invoke the callback on a transition from ${from} to ${to}`, () => { + reconnectedListener(to, from); + expect(onReconnect).toBeCalled(); + }); + }); + + [ + [SyncState.Syncing, SyncState.Syncing], + [SyncState.Catchup, SyncState.Error], + [SyncState.Reconnecting, SyncState.Error], + ].forEach(([from, to]) => { + it(`should not invoke the callback on a transition from ${from} to ${to}`, () => { + reconnectedListener(to, from); + expect(onReconnect).not.toBeCalled(); + }); + }); +});