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

Commit 75a989e

Browse files
Half-ShotHalf-Shot
authored
Revert "Make EC widget theme reactive - Update widget url when the theme chan…" (#12382)
This reverts commit c42562e.
1 parent f23c992 commit 75a989e

File tree

7 files changed

+23
-172
lines changed

7 files changed

+23
-172
lines changed

‎src/components/views/elements/AppTile.tsx

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ import { WidgetMessagingStore } from "../../../stores/widgets/WidgetMessagingSto
5858
import { SdkContextClass } from "../../../contexts/SDKContext";
5959
import { ModuleRunner } from "../../../modules/ModuleRunner";
6060
import { parseUrl } from "../../../utils/UrlUtils";
61-
import ThemeWatcher, { ThemeWatcherEvents } from "../../../settings/watchers/ThemeWatcher";
6261

6362
interface IProps {
6463
app: IWidget | IApp;
@@ -117,7 +116,6 @@ interface IState {
117116
menuDisplayed: boolean;
118117
requiresClient: boolean;
119118
hasContextMenuOptions: boolean;
120-
widgetUrl?: string;
121119
}
122120

123121
export default class AppTile extends React.Component<IProps, IState> {
@@ -143,7 +141,7 @@ export default class AppTile extends React.Component<IProps, IState> {
143141
private sgWidget: StopGapWidget | null;
144142
private dispatcherRef?: string;
145143
private unmounted = false;
146-
private themeWatcher = new ThemeWatcher();
144+
147145
public constructor(props: IProps, context: ContextType<typeof MatrixClientContext>) {
148146
super(props);
149147
this.context = context; // XXX: workaround for lack of `declare` support on `public context!:` definition
@@ -273,7 +271,6 @@ export default class AppTile extends React.Component<IProps, IState> {
273271
!newProps.userWidget,
274272
newProps.onDeleteClick,
275273
),
276-
widgetUrl: this.sgWidget?.embedUrl,
277274
};
278275
}
279276

@@ -359,18 +356,14 @@ export default class AppTile extends React.Component<IProps, IState> {
359356
}
360357

361358
private setupSgListeners(): void {
362-
this.themeWatcher.on(ThemeWatcherEvents.ThemeChange, this.onThemeChanged);
363-
this.themeWatcher.start();
364359
this.sgWidget?.on("ready", this.onWidgetReady);
365360
this.sgWidget?.on("error:preparing", this.updateRequiresClient);
366361
// emits when the capabilities have been set up or changed
367362
this.sgWidget?.on("capabilitiesNotified", this.updateRequiresClient);
368363
}
369364

370365
private stopSgListeners(): void {
371-
this.themeWatcher.stop();
372366
if (!this.sgWidget) return;
373-
this.themeWatcher.off(ThemeWatcherEvents.ThemeChange, this.onThemeChanged);
374367
this.sgWidget?.off("ready", this.onWidgetReady);
375368
this.sgWidget.off("error:preparing", this.updateRequiresClient);
376369
this.sgWidget.off("capabilitiesNotified", this.updateRequiresClient);
@@ -393,7 +386,6 @@ export default class AppTile extends React.Component<IProps, IState> {
393386
private startWidget(): void {
394387
this.sgWidget?.prepare().then(() => {
395388
if (this.unmounted) return;
396-
if (!this.state.initialising) return;
397389
this.setState({ initialising: false });
398390
});
399391
}
@@ -468,17 +460,6 @@ export default class AppTile extends React.Component<IProps, IState> {
468460
});
469461
};
470462

471-
private onThemeChanged = (): void => {
472-
// Regenerate widget url when the theme changes
473-
// this updates the url from e.g. `theme=light` to `theme=dark`
474-
// We only do this with EC widgets where the theme prop is in the hash. If the widget puts the
475-
// theme template variable outside the url hash this would cause a (IFrame) page reload on every theme change.
476-
if (WidgetType.CALL.matches(this.props.app.type)) this.setState({ widgetUrl: this.sgWidget?.embedUrl });
477-
478-
// TODO: This is a stop gap solution to responsively update the theme of the widget.
479-
// 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)
480-
};
481-
482463
private onAction = (payload: ActionPayload): void => {
483464
switch (payload.action) {
484465
case "m.sticker":
@@ -571,9 +552,9 @@ export default class AppTile extends React.Component<IProps, IState> {
571552
this.resetWidget(this.props);
572553
this.startMessaging();
573554

574-
if (this.iframe && this.state.widgetUrl) {
555+
if (this.iframe && this.sgWidget) {
575556
// Reload iframe
576-
this.iframe.src = this.state.widgetUrl;
557+
this.iframe.src = this.sgWidget.embedUrl;
577558
}
578559
});
579560
}
@@ -642,7 +623,7 @@ export default class AppTile extends React.Component<IProps, IState> {
642623
"mx_AppTileBody--mini": this.props.miniMode,
643624
"mx_AppTileBody--loading": this.state.loading,
644625
// We don't want mx_AppTileBody (rounded corners) for call widgets
645-
"mx_AppTileBody--call": WidgetType.CALL.matches(this.props.app.type),
626+
"mx_AppTileBody--call": this.props.app.type === WidgetType.CALL.preferred,
646627
});
647628
const appTileBodyStyles: CSSProperties = {};
648629
if (this.props.pointerEvents) {
@@ -671,7 +652,7 @@ export default class AppTile extends React.Component<IProps, IState> {
671652
<AppPermission
672653
roomId={this.props.room.roomId}
673654
creatorUserId={this.props.creatorUserId}
674-
url={this.state.widgetUrl}
655+
url={this.sgWidget.embedUrl}
675656
isRoomEncrypted={isEncrypted}
676657
onPermissionGranted={this.grantWidgetPermission}
677658
/>
@@ -699,7 +680,7 @@ export default class AppTile extends React.Component<IProps, IState> {
699680
title={widgetTitle}
700681
allow={iframeFeatures}
701682
ref={this.iframeRefChange}
702-
src={this.state.widgetUrl}
683+
src={this.sgWidget.embedUrl}
703684
allowFullScreen={true}
704685
sandbox={sandboxFlags}
705686
/>
@@ -722,12 +703,7 @@ export default class AppTile extends React.Component<IProps, IState> {
722703
const zIndexAboveOtherPersistentElements = 101;
723704

724705
appTileBody = (
725-
<div
726-
className="mx_AppTile_persistedWrapper"
727-
// 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)
728-
data-test-widget-url={this.state.widgetUrl}
729-
data-testid="widget-app-tile"
730-
>
706+
<div className="mx_AppTile_persistedWrapper">
731707
<PersistedElement
732708
zIndex={this.props.miniMode ? zIndexAboveOtherPersistentElements : 9}
733709
persistKey={this.persistKey}

‎src/settings/watchers/ThemeWatcher.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ limitations under the License.
1616
*/
1717

1818
import { logger } from "matrix-js-sdk/src/logger";
19-
import { TypedEventEmitter } from "matrix-js-sdk/src/matrix";
2019

2120
import SettingsStore from "../SettingsStore";
2221
import dis from "../../dispatcher/dispatcher";
@@ -26,15 +25,7 @@ import { findHighContrastTheme, setTheme } from "../../theme";
2625
import { ActionPayload } from "../../dispatcher/payloads";
2726
import { SettingLevel } from "../SettingLevel";
2827

29-
export enum ThemeWatcherEvents {
30-
ThemeChange = "theme_change",
31-
}
32-
33-
type EventHandlerMap = {
34-
[ThemeWatcherEvents.ThemeChange]: (theme: string) => void;
35-
};
36-
37-
export default class ThemeWatcher extends TypedEventEmitter<ThemeWatcherEvents, EventHandlerMap> {
28+
export default class ThemeWatcher {
3829
private themeWatchRef: string | null;
3930
private systemThemeWatchRef: string | null;
4031
private dispatcherRef: string | null;
@@ -46,7 +37,6 @@ export default class ThemeWatcher extends TypedEventEmitter<ThemeWatcherEvents,
4637
private currentTheme: string;
4738

4839
public constructor() {
49-
super();
5040
this.themeWatchRef = null;
5141
this.systemThemeWatchRef = null;
5242
this.dispatcherRef = null;
@@ -96,7 +86,6 @@ export default class ThemeWatcher extends TypedEventEmitter<ThemeWatcherEvents,
9686
this.currentTheme = forceTheme === undefined ? this.getEffectiveTheme() : forceTheme;
9787
if (oldTheme !== this.currentTheme) {
9888
setTheme(this.currentTheme);
99-
this.emit(ThemeWatcherEvents.ThemeChange, this.currentTheme);
10089
}
10190
}
10291

‎src/stores/widgets/StopGapWidget.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { MatrixClientPeg } from "../../MatrixClientPeg";
4545
import { OwnProfileStore } from "../OwnProfileStore";
4646
import WidgetUtils from "../../utils/WidgetUtils";
4747
import { IntegrationManagers } from "../../integrations/IntegrationManagers";
48+
import SettingsStore from "../../settings/SettingsStore";
4849
import { WidgetType } from "../../widgets/WidgetType";
4950
import ActiveWidgetStore from "../ActiveWidgetStore";
5051
import { objectShallowClone } from "../../utils/objects";
@@ -162,7 +163,6 @@ export class StopGapWidget extends EventEmitter {
162163
private readonly virtual: boolean;
163164
private readUpToMap: { [roomId: string]: string } = {}; // room ID to event ID
164165
private stickyPromise?: () => Promise<void>; // This promise will be called and needs to resolve before the widget will actually become sticky.
165-
private themeWatcher = new ThemeWatcher();
166166

167167
public constructor(private appTileProps: IAppTileProps) {
168168
super();
@@ -213,19 +213,13 @@ export class StopGapWidget extends EventEmitter {
213213

214214
private runUrlTemplate(opts = { asPopout: false }): string {
215215
const fromCustomisation = WidgetVariableCustomisations?.provideVariables?.() ?? {};
216-
let theme = this.themeWatcher.getEffectiveTheme();
217-
if (theme.startsWith("custom-")) {
218-
// simplify custom theme to only light/dark
219-
const customTheme = getCustomTheme(theme.slice(7));
220-
theme = customTheme.is_dark ? "dark" : "light";
221-
}
222216
const defaults: ITemplateParams = {
223217
widgetRoomId: this.roomId,
224218
currentUserId: this.client.getUserId()!,
225219
userDisplayName: OwnProfileStore.instance.displayName ?? undefined,
226220
userHttpAvatarUrl: OwnProfileStore.instance.getHttpAvatarUrl() ?? undefined,
227221
clientId: ELEMENT_CLIENT_ID,
228-
clientTheme: theme,
222+
clientTheme: SettingsStore.getValue("theme"),
229223
clientLanguage: getUserLanguage(),
230224
deviceId: this.client.getDeviceId() ?? undefined,
231225
baseUrl: this.client.baseUrl,

‎src/theme.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ function clearCustomTheme(): void {
121121
// remove all css variables, we assume these are there because of the custom theme
122122
const inlineStyleProps = Object.values(document.body.style);
123123
for (const prop of inlineStyleProps) {
124-
if (typeof prop === "string" && prop.startsWith("--")) {
124+
if (prop.startsWith("--")) {
125125
document.body.style.removeProperty(prop);
126126
}
127127
}

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

Lines changed: 2 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { jest } from "@jest/globals";
1919
import { Room, MatrixClient } from "matrix-js-sdk/src/matrix";
2020
import { ClientWidgetApi, IWidget, MatrixWidgetType } from "matrix-widget-api";
2121
import { Optional } from "matrix-events-sdk";
22-
import { act, render, RenderResult, waitFor } from "@testing-library/react";
22+
import { act, render, RenderResult } from "@testing-library/react";
2323
import userEvent from "@testing-library/user-event";
2424
import { SpiedFunction } from "jest-mock";
2525
import {
@@ -50,8 +50,6 @@ import { ElementWidget } from "../../../../src/stores/widgets/StopGapWidget";
5050
import { WidgetMessagingStore } from "../../../../src/stores/widgets/WidgetMessagingStore";
5151
import { ModuleRunner } from "../../../../src/modules/ModuleRunner";
5252
import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
53-
import { SettingLevel } from "../../../../src/settings/SettingLevel";
54-
import { WidgetType } from "../../../../src/widgets/WidgetType";
5553

5654
jest.mock("../../../../src/stores/OwnProfileStore", () => ({
5755
OwnProfileStore: {
@@ -70,7 +68,6 @@ describe("AppTile", () => {
7068
const resizeNotifier = new ResizeNotifier();
7169
let app1: IApp;
7270
let app2: IApp;
73-
let appElementCall: IApp;
7471

7572
const waitForRps = (roomId: string) =>
7673
new Promise<void>((resolve) => {
@@ -123,17 +120,6 @@ describe("AppTile", () => {
123120
creatorUserId: cli.getSafeUserId(),
124121
avatar_url: undefined,
125122
};
126-
appElementCall = {
127-
id: "1",
128-
eventId: "2",
129-
roomId: "r2",
130-
type: WidgetType.CALL.preferred,
131-
url: "https://example.com#theme=$org.matrix.msc2873.client_theme",
132-
name: "Element Call",
133-
creatorUserId: cli.getSafeUserId(),
134-
avatar_url: undefined,
135-
};
136-
137123
jest.spyOn(WidgetStore.instance, "getApps").mockImplementation((roomId: string): Array<IApp> => {
138124
if (roomId === "r1") return [app1];
139125
if (roomId === "r2") return [app2];
@@ -453,6 +439,7 @@ describe("AppTile", () => {
453439
expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Top);
454440
});
455441
});
442+
456443
describe("with an existing widgetApi with requiresClient = false", () => {
457444
beforeEach(() => {
458445
const api = {
@@ -479,68 +466,6 @@ describe("AppTile", () => {
479466
});
480467
});
481468

482-
describe("with an element call widget", () => {
483-
beforeEach(() => {
484-
document.body.style.setProperty("--custom-color", "red");
485-
document.body.style.setProperty("normal-color", "blue");
486-
});
487-
it("should update the widget url on theme change", async () => {
488-
const renderResult = render(
489-
<MatrixClientContext.Provider value={cli}>
490-
<a href="http://themeb" data-mx-theme="light">
491-
A
492-
</a>
493-
<a href="http://themeA" data-mx-theme="dark">
494-
B
495-
</a>
496-
<AppTile key={appElementCall.id} app={appElementCall} room={r1} />
497-
</MatrixClientContext.Provider>,
498-
);
499-
await waitFor(() => {
500-
expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
501-
"https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
502-
);
503-
});
504-
await SettingsStore.setValue("theme", null, SettingLevel.DEVICE, "dark");
505-
await waitFor(() => {
506-
expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
507-
"https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=dark",
508-
);
509-
});
510-
await SettingsStore.setValue("theme", null, SettingLevel.DEVICE, "light");
511-
await waitFor(() => {
512-
expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
513-
"https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
514-
);
515-
});
516-
});
517-
it("should not update the widget url for non Element Call widgets on theme change", async () => {
518-
const appNonElementCall = { ...appElementCall, type: MatrixWidgetType.Custom };
519-
const renderResult = render(
520-
<MatrixClientContext.Provider value={cli}>
521-
<a href="http://themeb" data-mx-theme="light">
522-
A
523-
</a>
524-
<a href="http://themeA" data-mx-theme="dark">
525-
B
526-
</a>
527-
<AppTile key={appNonElementCall.id} app={appNonElementCall} room={r1} />
528-
</MatrixClientContext.Provider>,
529-
);
530-
await waitFor(() => {
531-
expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
532-
"https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
533-
);
534-
});
535-
await SettingsStore.setValue("theme", null, SettingLevel.DEVICE, "dark");
536-
await waitFor(() => {
537-
expect(renderResult.getByTestId("widget-app-tile").dataset.testWidgetUrl).toEqual(
538-
"https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F#theme=light",
539-
);
540-
});
541-
});
542-
});
543-
544469
describe("for a persistent app", () => {
545470
let renderResult: RenderResult;
546471

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ exports[`AppTile for a persistent app should render 1`] = `
8585
>
8686
<div
8787
class="mx_AppTile_persistedWrapper"
88-
data-test-widget-url="https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F"
89-
data-testid="widget-app-tile"
9088
>
9189
<div />
9290
</div>
@@ -174,8 +172,6 @@ exports[`AppTile for a pinned widget should render 1`] = `
174172
</div>
175173
<div
176174
class="mx_AppTile_persistedWrapper"
177-
data-test-widget-url="https://example.com/?widgetId=1&parentUrl=http%3A%2F%2Flocalhost%2F"
178-
data-testid="widget-app-tile"
179175
>
180176
<div />
181177
</div>

0 commit comments

Comments
 (0)