Make EC widget theme reactive - Update widget url when the theme changes (#12295)

* update widget url when the theme changes

Signed-off-by: Timo K <toger5@hotmail.de>

* quick "make it EC specific" workaround proposal.

Signed-off-by: Timo K <toger5@hotmail.de>

* use `matches`

Signed-off-by: Timo K <toger5@hotmail.de>

* test coverage

Signed-off-by: Timo K <toger5@hotmail.de>

* more test coverage

Signed-off-by: Timo K <toger5@hotmail.de>

* fix jest

Signed-off-by: Timo K <toger5@hotmail.de>

* add tests for theme changes

Signed-off-by: Timo K <toger5@hotmail.de>

* update snapshots

Signed-off-by: Timo K <toger5@hotmail.de>

* test for theme update with non ec widget

Signed-off-by: Timo K <toger5@hotmail.de>

* add dark custom theme widget url

Signed-off-by: Timo K <toger5@hotmail.de>

* trigger conditions for theme cleanup

Signed-off-by: Timo K <toger5@hotmail.de>

* update tests using testId

Signed-off-by: Timo K <toger5@hotmail.de>

* use typed event emitter for theme watcher

Signed-off-by: Timo K <toger5@hotmail.de>

* simplify condition

Signed-off-by: Timo K <toger5@hotmail.de>

---------

Signed-off-by: Timo K <toger5@hotmail.de>

src/components/views/elements/AppTile.tsx

@@ -57,6 +57,7 @@ import { WidgetMessagingStore } from "../../../stores/widgets/WidgetMessagingSto
 import { SdkContextClass } from "../../../contexts/SDKContext";
 import { ModuleRunner } from "../../../modules/ModuleRunner";
 import { parseUrl } from "../../../utils/UrlUtils";
+import ThemeWatcher, { ThemeWatcherEvents } from "../../../settings/watchers/ThemeWatcher";
 
 interface IProps {
     app: IWidget | IApp;
@@ -115,6 +116,7 @@ interface IState {
     menuDisplayed: boolean;
     requiresClient: boolean;
     hasContextMenuOptions: boolean;
+    widgetUrl?: string;
 }
 
 export default class AppTile extends React.Component<IProps, IState> {
@@ -140,7 +142,7 @@ export default class AppTile extends React.Component<IProps, IState> {
     private sgWidget: StopGapWidget | null;
     private dispatcherRef?: string;
     private unmounted = false;
-
+    private themeWatcher = new ThemeWatcher();
     public constructor(props: IProps, context: ContextType<typeof MatrixClientContext>) {
         super(props);
         this.context = context; // XXX: workaround for lack of `declare` support on `public context!:` definition
@@ -267,6 +269,7 @@ export default class AppTile extends React.Component<IProps, IState> {
                 !newProps.userWidget,
                 newProps.onDeleteClick,
             ),
+            widgetUrl: this.sgWidget?.embedUrl,
         };
     }
 
@@ -352,6 +355,8 @@ export default class AppTile extends React.Component<IProps, IState> {
     }
 
     private setupSgListeners(): void {
+        this.themeWatcher.on(ThemeWatcherEvents.ThemeChange, this.onThemeChanged);
+        this.themeWatcher.start();
         this.sgWidget?.on("ready", this.onWidgetReady);
         this.sgWidget?.on("error:preparing", this.updateRequiresClient);
         // emits when the capabilities have been set up or changed
@@ -359,7 +364,9 @@ export default class AppTile extends React.Component<IProps, IState> {
     }
 
     private stopSgListeners(): void {
+        this.themeWatcher.stop();
         if (!this.sgWidget) return;
+        this.themeWatcher.off(ThemeWatcherEvents.ThemeChange, this.onThemeChanged);
         this.sgWidget?.off("ready", this.onWidgetReady);
         this.sgWidget.off("error:preparing", this.updateRequiresClient);
         this.sgWidget.off("capabilitiesNotified", this.updateRequiresClient);
@@ -382,6 +389,7 @@ export default class AppTile extends React.Component<IProps, IState> {
     private startWidget(): void {
         this.sgWidget?.prepare().then(() => {
             if (this.unmounted) return;
+            if (!this.state.initialising) return;
             this.setState({ initialising: false });
         });
     }
@@ -456,6 +464,17 @@ export default class AppTile extends React.Component<IProps, IState> {
         });
     };
 
+    private onThemeChanged = (): void => {
+        // Regenerate widget url when the theme changes
+        // this updates the url from e.g. `theme=light` to `theme=dark`
+        // We only do this with EC widgets where the theme prop is in the hash. If the widget puts the
+        // theme template variable outside the url hash this would cause a (IFrame) page reload on every theme change.
+        if (WidgetType.CALL.matches(this.props.app.type)) this.setState({ widgetUrl: this.sgWidget?.embedUrl });
+
+        // TODO: This is a stop gap solution to responsively update the theme of the widget.
+        // A new action should be introduced and the widget driver should be called here, so it informs the widget. (or connect to this by itself)
+    };
+
     private onAction = (payload: ActionPayload): void => {
         switch (payload.action) {
             case "m.sticker":
@@ -548,9 +567,9 @@ export default class AppTile extends React.Component<IProps, IState> {
             this.resetWidget(this.props);
             this.startMessaging();
 
-            if (this.iframe && this.sgWidget) {
+            if (this.iframe && this.state.widgetUrl) {
                 // Reload iframe
-                this.iframe.src = this.sgWidget.embedUrl;
+                this.iframe.src = this.state.widgetUrl;
             }
         });
     }
@@ -619,7 +638,7 @@ export default class AppTile extends React.Component<IProps, IState> {
             "mx_AppTileBody--mini": this.props.miniMode,
             "mx_AppTileBody--loading": this.state.loading,
             // We don't want mx_AppTileBody (rounded corners) for call widgets
-            "mx_AppTileBody--call": this.props.app.type === WidgetType.CALL.preferred,
+            "mx_AppTileBody--call": WidgetType.CALL.matches(this.props.app.type),
         });
         const appTileBodyStyles: CSSProperties = {};
         if (this.props.pointerEvents) {
@@ -648,7 +667,7 @@ export default class AppTile extends React.Component<IProps, IState> {
                     <AppPermission
                         roomId={this.props.room.roomId}
                         creatorUserId={this.props.creatorUserId}
-                        url={this.sgWidget.embedUrl}
+                        url={this.state.widgetUrl}
                         isRoomEncrypted={isEncrypted}
                         onPermissionGranted={this.grantWidgetPermission}
                     />
@@ -676,7 +695,7 @@ export default class AppTile extends React.Component<IProps, IState> {
                                 title={widgetTitle}
                                 allow={iframeFeatures}
                                 ref={this.iframeRefChange}
-                                src={this.sgWidget.embedUrl}
+                                src={this.state.widgetUrl}
                                 allowFullScreen={true}
                                 sandbox={sandboxFlags}
                             />
@@ -699,7 +718,12 @@ export default class AppTile extends React.Component<IProps, IState> {
                     const zIndexAboveOtherPersistentElements = 101;
 
                     appTileBody = (
-                        <div className="mx_AppTile_persistedWrapper">
+                        <div
+                            className="mx_AppTile_persistedWrapper"
+                            // We store the widget url to make it possible to test the value of the widgetUrl. since the iframe itself wont be here. (PersistedElement are in a different dom tree)
+                            data-test-widget-url={this.state.widgetUrl}
+                            data-testid="widget-app-tile"
+                        >
                             <PersistedElement
                                 zIndex={this.props.miniMode ? zIndexAboveOtherPersistentElements : 9}
                                 persistKey={this.persistKey}

src/settings/watchers/ThemeWatcher.ts

@@ -16,6 +16,7 @@ limitations under the License.
 */
 
 import { logger } from "matrix-js-sdk/src/logger";
+import { TypedEventEmitter } from "matrix-js-sdk/src/matrix";
 
 import SettingsStore from "../SettingsStore";
 import dis from "../../dispatcher/dispatcher";
@@ -25,7 +26,15 @@ import { findHighContrastTheme, setTheme } from "../../theme";
 import { ActionPayload } from "../../dispatcher/payloads";
 import { SettingLevel } from "../SettingLevel";
 
-export default class ThemeWatcher {
+export enum ThemeWatcherEvents {
+    ThemeChange = "theme_change",
+}
+
+type EventHandlerMap = {
+    [ThemeWatcherEvents.ThemeChange]: (theme: string) => void;
+};
+
+export default class ThemeWatcher extends TypedEventEmitter<ThemeWatcherEvents, EventHandlerMap> {
     private themeWatchRef: string | null;
     private systemThemeWatchRef: string | null;
     private dispatcherRef: string | null;
@@ -37,6 +46,7 @@ export default class ThemeWatcher {
     private currentTheme: string;
 
     public constructor() {
+        super();
         this.themeWatchRef = null;
         this.systemThemeWatchRef = null;
         this.dispatcherRef = null;
@@ -86,6 +96,7 @@ export default class ThemeWatcher {
         this.currentTheme = forceTheme === undefined ? this.getEffectiveTheme() : forceTheme;
         if (oldTheme !== this.currentTheme) {
             setTheme(this.currentTheme);
+            this.emit(ThemeWatcherEvents.ThemeChange, this.currentTheme);
         }
     }
 

src/stores/widgets/StopGapWidget.ts

@@ -44,7 +44,6 @@ import { MatrixClientPeg } from "../../MatrixClientPeg";
 import { OwnProfileStore } from "../OwnProfileStore";
 import WidgetUtils from "../../utils/WidgetUtils";
 import { IntegrationManagers } from "../../integrations/IntegrationManagers";
-import SettingsStore from "../../settings/SettingsStore";
 import { WidgetType } from "../../widgets/WidgetType";
 import ActiveWidgetStore from "../ActiveWidgetStore";
 import { objectShallowClone } from "../../utils/objects";
@@ -162,6 +161,7 @@ export class StopGapWidget extends EventEmitter {
     private readonly virtual: boolean;
     private readUpToMap: { [roomId: string]: string } = {}; // room ID to event ID
     private stickyPromise?: () => Promise<void>; // This promise will be called and needs to resolve before the widget will actually become sticky.
+    private themeWatcher = new ThemeWatcher();
 
     public constructor(private appTileProps: IAppTileProps) {
         super();
@@ -212,13 +212,19 @@ export class StopGapWidget extends EventEmitter {
 
     private runUrlTemplate(opts = { asPopout: false }): string {
         const fromCustomisation = WidgetVariableCustomisations?.provideVariables?.() ?? {};
+        let theme = this.themeWatcher.getEffectiveTheme();
+        if (theme.startsWith("custom-")) {
+            // simplify custom theme to only light/dark
+            const customTheme = getCustomTheme(theme.slice(7));
+            theme = customTheme.is_dark ? "dark" : "light";
+        }
         const defaults: ITemplateParams = {
             widgetRoomId: this.roomId,
             currentUserId: this.client.getUserId()!,
             userDisplayName: OwnProfileStore.instance.displayName ?? undefined,
             userHttpAvatarUrl: OwnProfileStore.instance.getHttpAvatarUrl() ?? undefined,
             clientId: ELEMENT_CLIENT_ID,
-            clientTheme: SettingsStore.getValue("theme"),
+            clientTheme: theme,
             clientLanguage: getUserLanguage(),
             deviceId: this.client.getDeviceId() ?? undefined,
             baseUrl: this.client.baseUrl,

src/theme.ts

@@ -121,7 +121,7 @@ function clearCustomTheme(): void {
     // remove all css variables, we assume these are there because of the custom theme
     const inlineStyleProps = Object.values(document.body.style);
     for (const prop of inlineStyleProps) {
-        if (prop.startsWith("--")) {
+        if (typeof prop === "string" && prop.startsWith("--")) {
             document.body.style.removeProperty(prop);
         }
     }

test/components/views/elements/AppTile-test.tsx

@@ -19,7 +19,7 @@ import { jest } from "@jest/globals";
 import { Room, MatrixClient } from "matrix-js-sdk/src/matrix";
 import { ClientWidgetApi, IWidget, MatrixWidgetType } from "matrix-widget-api";
 import { Optional } from "matrix-events-sdk";
-import { act, render, RenderResult } from "@testing-library/react";
+import { act, render, RenderResult, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { SpiedFunction } from "jest-mock";
 import {
@@ -50,6 +50,8 @@ import { ElementWidget } from "../../../../src/stores/widgets/StopGapWidget";
 import { WidgetMessagingStore } from "../../../../src/stores/widgets/WidgetMessagingStore";
 import { ModuleRunner } from "../../../../src/modules/ModuleRunner";
 import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
+import { SettingLevel } from "../../../../src/settings/SettingLevel";
+import { WidgetType } from "../../../../src/widgets/WidgetType";
 
 jest.mock("../../../../src/stores/OwnProfileStore", () => ({
     OwnProfileStore: {
@@ -68,6 +70,7 @@ describe("AppTile", () => {
     const resizeNotifier = new ResizeNotifier();
     let app1: IApp;
     let app2: IApp;
+    let appElementCall: IApp;
 
     const waitForRps = (roomId: string) =>
         new Promise<void>((resolve) => {
@@ -120,6 +123,17 @@ describe("AppTile", () => {
             creatorUserId: cli.getSafeUserId(),
             avatar_url: undefined,
         };
+        appElementCall = {
+            id: "1",
+            eventId: "2",
+            roomId: "r2",
+            type: WidgetType.CALL.preferred,
+            url: "https://example.com#theme=$org.matrix.msc2873.client_theme",
+            name: "Element Call",
+            creatorUserId: cli.getSafeUserId(),
+            avatar_url: undefined,
+        };
+
         jest.spyOn(WidgetStore.instance, "getApps").mockImplementation((roomId: string): Array<IApp> => {
             if (roomId === "r1") return [app1];
             if (roomId === "r2") return [app2];
@@ -439,7 +453,6 @@ describe("AppTile", () => {
                 expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Top);
             });
         });
-
         describe("with an existing widgetApi with requiresClient = false", () => {
             beforeEach(() => {
                 const api = {
@@ -466,6 +479,68 @@ describe("AppTile", () => {
         });
     });
 
+    describe("with an element call widget", () => {
+        beforeEach(() => {
+            document.body.style.setProperty("--custom-color", "red");
+            document.body.style.setProperty("normal-color", "blue");
+        });
+        it("should update the widget url on theme change", async () => {
+            const renderResult = render(
+                <MatrixClientContext.Provider value={cli}>
+                    <a href="http://themeb" data-mx-theme="light">
+                        A
+                    </a>
+                    <a href="http://themeA" data-mx-theme="dark">
+                        B
+                    </a>
+                    <AppTile key={appElementCall.id} app={appElementCall} room={r1} />
+                </MatrixClientContext.Provider>,
+            );
+            await waitFor(() => {
+                expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
+                    "https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
+                );
+            });
+            await SettingsStore.setValue("theme", null, SettingLevel.DEVICE, "dark");
+            await waitFor(() => {
+                expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
+                    "https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=dark",
+                );
+            });
+            await SettingsStore.setValue("theme", null, SettingLevel.DEVICE, "light");
+            await waitFor(() => {
+                expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
+                    "https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
+                );
+            });
+        });
+        it("should not update the widget url for non Element Call widgets on theme change", async () => {
+            const appNonElementCall = { ...appElementCall, type: MatrixWidgetType.Custom };
+            const renderResult = render(
+                <MatrixClientContext.Provider value={cli}>
+                    <a href="http://themeb" data-mx-theme="light">
+                        A
+                    </a>
+                    <a href="http://themeA" data-mx-theme="dark">
+                        B
+                    </a>
+                    <AppTile key={appNonElementCall.id} app={appNonElementCall} room={r1} />
+                </MatrixClientContext.Provider>,
+            );
+            await waitFor(() => {
+                expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
+                    "https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
+                );
+            });
+            await SettingsStore.setValue("theme", null, SettingLevel.DEVICE, "dark");
+            await waitFor(() => {
+                expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
+                    "https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
+                );
+            });
+        });
+    });
+
     describe("for a persistent app", () => {
         let renderResult: RenderResult;
 

test/components/views/elements/__snapshots__/AppTile-test.tsx.snap

@@ -85,6 +85,8 @@ exports[`AppTile for a persistent app should render 1`] = `
   >
     <div
       class="mx_AppTile_persistedWrapper"
+      data-test-widget-url="https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F"
+      data-testid="widget-app-tile"
     >
       <div />
     </div>
@@ -172,6 +174,8 @@ exports[`AppTile for a pinned widget should render 1`] = `
     </div>
     <div
       class="mx_AppTile_persistedWrapper"
+      data-test-widget-url="https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F"
+      data-testid="widget-app-tile"
     >
       <div />
     </div>

test/stores/widgets/StopGapWidget-test.ts

@@ -28,6 +28,7 @@ import { VoiceBroadcastInfoEventType, VoiceBroadcastRecording } from "../../../s
 import { SdkContextClass } from "../../../src/contexts/SDKContext";
 import ActiveWidgetStore from "../../../src/stores/ActiveWidgetStore";
 import SettingsStore from "../../../src/settings/SettingsStore";
+import * as Theme from "../../../src/theme";
 
 jest.mock("matrix-widget-api/lib/ClientWidgetApi");
 
@@ -63,18 +64,46 @@ describe("StopGapWidget", () => {
         widget.stopMessaging();
     });
 
-    it("should replace parameters in widget url template", () => {
+    describe("url template", () => {
-        const originGetValue = SettingsStore.getValue;
+        it("should replace parameters in widget url template", () => {
-        const spy = jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
+            const originalGetValue = SettingsStore.getValue;
-            if (setting === "theme") return "my-theme-for-testing";
+            const spy = jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
-            return originGetValue(setting);
+                if (setting === "theme") return "my-theme-for-testing";
+                return originalGetValue(setting);
+            });
+            expect(widget.embedUrl).toBe(
+                "https://example.org/?user-id=%40userId%3Amatrix.org&device-id=ABCDEFGHI&base-url=https%3A%2F%2Fmatrix-client.matrix.org&theme=my-theme-for-testing&widgetId=test&parentUrl=http%3A%2F%2Flocalhost%2F",
+            );
+            spy.mockRestore();
+        });
+        it("should replace custom theme with light/dark", () => {
+            const originalGetValue = SettingsStore.getValue;
+            const spy = jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
+                if (setting === "theme") return "custom-my-theme";
+                return originalGetValue(setting);
+            });
+            jest.spyOn(Theme, "getCustomTheme").mockReturnValue({ is_dark: false } as unknown as Theme.CustomTheme);
+            expect(widget.embedUrl).toBe(
+                "https://example.org/?user-id=%40userId%3Amatrix.org&device-id=ABCDEFGHI&base-url=https%3A%2F%2Fmatrix-client.matrix.org&theme=light&widgetId=test&parentUrl=http%3A%2F%2Flocalhost%2F",
+            );
+            jest.spyOn(Theme, "getCustomTheme").mockReturnValue({ is_dark: true } as unknown as Theme.CustomTheme);
+            expect(widget.embedUrl).toBe(
+                "https://example.org/?user-id=%40userId%3Amatrix.org&device-id=ABCDEFGHI&base-url=https%3A%2F%2Fmatrix-client.matrix.org&theme=dark&widgetId=test&parentUrl=http%3A%2F%2Flocalhost%2F",
+            );
+            spy.mockRestore();
+        });
+        it("should replace parameters in widget popoutUrl template", () => {
+            const originalGetValue = SettingsStore.getValue;
+            const spy = jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => {
+                if (setting === "theme") return "my-theme-for-testing";
+                return originalGetValue(setting);
+            });
+            expect(widget.popoutUrl).toBe(
+                "https://example.org/?user-id=%40userId%3Amatrix.org&device-id=ABCDEFGHI&base-url=https%3A%2F%2Fmatrix-client.matrix.org&theme=my-theme-for-testing",
+            );
+            spy.mockRestore();
         });
-        expect(widget.embedUrl).toBe(
-            "https://example.org/?user-id=%40userId%3Amatrix.org&device-id=ABCDEFGHI&base-url=https%3A%2F%2Fmatrix-client.matrix.org&theme=my-theme-for-testing&widgetId=test&parentUrl=http%3A%2F%2Flocalhost%2F",
-        );
-        spy.mockClear();
     });
-
     it("feeds incoming to-device messages to the widget", async () => {
         const event = mkEvent({
             event: true,