Reload map on reconnect (#8848)

PSD-282

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<IBodyProps, IState> {
     private image = createRef<HTMLImageElement>();
     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<IBodyProps, IState> {
         };
     }
 
-    // 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<IBodyProps, IState> {
         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<IBodyProps, IState> {
 
     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<IBodyProps, IState> {
 
     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) {

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<IBodyProps, IState> {
     public static contextType = MatrixClientContext;
     public context!: React.ContextType<typeof MatrixClientContext>;
     private mapId: string;
+    private reconnectedListener: ClientEventHandlerMap[ClientEvent.Sync];
 
     constructor(props: IBodyProps) {
         super(props);
@@ -51,6 +54,8 @@ export default class MLocationBody extends React.Component<IBodyProps, IState> {
         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<IBodyProps, IState> {
         );
     };
 
-    private onError = (error) => {
+    private clearError = () => {
-        this.setState({ error });
+        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<HTMLDivElement> {
         return this.state.error ?
             <LocationBodyFallbackContent error={this.state.error} event={this.props.mxEvent} /> :

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();
+        }
+    };
+};

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();
+                const component = getMapErrorComponent();
-                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 });
                 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', () => {

test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap

@@ -120,6 +120,10 @@ exports[`MLocationBody <MLocationBody> without error renders map correctly 1`] =
                       "error": Array [
                         [Function],
                         [Function],
+                        [Function],
+                        [Function],
+                        [Function],
+                        [Function],
                       ],
                     },
                     "_eventsCount": 1,
@@ -130,12 +134,20 @@ exports[`MLocationBody <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 <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],

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();
+        });
+    });
+});