Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Make EC widget theme reactive - Update widget url when the theme chan…
Browse files Browse the repository at this point in the history 
…ges (#12295)

* update widget url when the theme changes

Signed-off-by: Timo K <[email protected]>

* quick "make it EC specific" workaround proposal.

Signed-off-by: Timo K <[email protected]>

* use `matches`

Signed-off-by: Timo K <[email protected]>

* test coverage

Signed-off-by: Timo K <[email protected]>

* more test coverage

Signed-off-by: Timo K <[email protected]>

* fix jest

Signed-off-by: Timo K <[email protected]>

* add tests for theme changes

Signed-off-by: Timo K <[email protected]>

* update snapshots

Signed-off-by: Timo K <[email protected]>

* test for theme update with non ec widget

Signed-off-by: Timo K <[email protected]>

* add dark custom theme widget url

Signed-off-by: Timo K <[email protected]>

* trigger conditions for theme cleanup

Signed-off-by: Timo K <[email protected]>

* update tests using testId

Signed-off-by: Timo K <[email protected]>

* use typed event emitter for theme watcher

Signed-off-by: Timo K <[email protected]>

* simplify condition

Signed-off-by: Timo K <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>
  • Loading branch information
@toger5 @andybalaam
toger5 authored and andybalaam committed Mar 18, 2024
1 parent 6231213 commit d402abc
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 23 deletions.
38 changes: 31 additions & 7 deletions src/components/views/elements/AppTile.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -115,6 +116,7 @@ interface IState {
menuDisplayed: boolean;
requiresClient: boolean;
hasContextMenuOptions: boolean;
widgetUrl?: string;
}

export default class AppTile extends React.Component<IProps, IState> {
Expand All @@ -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
Expand Down Expand Up @@ -270,6 +272,7 @@ export default class AppTile extends React.Component<IProps, IState> {
!newProps.userWidget,
newProps.onDeleteClick,
),
widgetUrl: this.sgWidget?.embedUrl,
};
}

Expand Down Expand Up @@ -355,14 +358,18 @@ 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
this.sgWidget?.on("capabilitiesNotified", this.updateRequiresClient);
}

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);
Expand All @@ -385,6 +392,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 });
});
}
Expand Down Expand Up @@ -459,6 +467,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":
Expand Down Expand Up @@ -551,9 +570,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;
}
});
}
Expand Down Expand Up @@ -622,7 +641,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) {
Expand Down Expand Up @@ -651,7 +670,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}
/>
Expand Down Expand Up @@ -679,7 +698,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}
/>
Expand All @@ -702,7 +721,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}
Expand Down
13 changes: 12 additions & 1 deletion src/settings/watchers/ThemeWatcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand All @@ -37,6 +46,7 @@ export default class ThemeWatcher {
private currentTheme: string;

public constructor() {
super();
this.themeWatchRef = null;
this.systemThemeWatchRef = null;
this.dispatcherRef = null;
Expand Down Expand Up @@ -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);
}
}

Expand Down
10 changes: 8 additions & 2 deletions src/stores/widgets/StopGapWidget.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,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";
Expand Down Expand Up @@ -169,6 +168,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();
Expand Down Expand Up @@ -219,13 +219,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,
Expand Down
2 changes: 1 addition & 1 deletion src/theme.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
79 changes: 77 additions & 2 deletions test/components/views/elements/AppTile-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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: {
Expand All @@ -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) => {
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -439,7 +453,6 @@ describe("AppTile", () => {
expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Top);
});
});

describe("with an existing widgetApi with requiresClient = false", () => {
beforeEach(() => {
const api = {
Expand All @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions test/components/views/elements/__snapshots__/AppTile-test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Loading

0 comments on commit d402abc

Please sign in to comment.