From 169676720d0509e47b3daa3ca559d639b6cf8a13 Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 27 Feb 2024 21:36:43 +0100 Subject: [PATCH 1/8] show loading spinner also if waitForIframeLoad = false Configure EC so it waits for the content loaded action !WARNING This breaks compatibility with the full mesh branch. I would like to discuss here if/when we can do that. Signed-off-by: Timo K --- src/components/views/elements/AppTile.tsx | 2 +- src/models/Call.ts | 2 +- test/test-utils/call.ts | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index 74317041bd2..e903cc8786d 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -250,7 +250,7 @@ export default class AppTile extends React.Component { return { initialising: true, // True while we are mangling the widget URL // True while the iframe content is loading - loading: this.props.waitForIframeLoad && !PersistedElement.isMounted(this.persistKey), + loading: !PersistedElement.isMounted(this.persistKey), // Assume that widget has permission to load if we are the user who // added it to the room, or if explicitly granted by the user hasPermissionToLoad: this.hasPermissionToLoad(newProps), diff --git a/src/models/Call.ts b/src/models/Call.ts index b6c1cda662e..369f712534f 100644 --- a/src/models/Call.ts +++ b/src/models/Call.ts @@ -758,7 +758,7 @@ export class ElementCall extends Call { name: "Element Call", type: WidgetType.CALL.preferred, url: url.toString(), - // waitForIframeLoad: false, + waitForIframeLoad: false, data: ElementCall.getWidgetData( client, roomId, diff --git a/test/test-utils/call.ts b/test/test-utils/call.ts index 2d049c258ef..e97c7a59e9b 100644 --- a/test/test-utils/call.ts +++ b/test/test-utils/call.ts @@ -38,9 +38,8 @@ export class MockedCall extends Call { url: "https://example.org", name: "Group call", creatorUserId: "@alice:example.org", - // waitForIframeLoad = false, makes the widget API wait for the 'contentLoaded' event instead. - // This is how the EC is designed, but for backwards compatibility (full mesh) we currently need to use waitForIframeLoad = true - // waitForIframeLoad: false + // waitForIframeLoad = false, makes the widget API wait for the 'contentLoaded' event. + waitForIframeLoad: false, }, room.client, ); From f7b0035669e85f8fe725d400bf60779bd07dc3b0 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 28 Feb 2024 16:34:15 +0100 Subject: [PATCH 2/8] stop show loading screen on widget ready (instead of preparing) Signed-off-by: Timo K --- src/components/views/elements/AppTile.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index e903cc8786d..b2f5bb9467a 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -352,7 +352,7 @@ export default class AppTile extends React.Component { } private setupSgListeners(): void { - this.sgWidget?.on("preparing", this.onWidgetPreparing); + 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); @@ -360,7 +360,7 @@ export default class AppTile extends React.Component { private stopSgListeners(): void { if (!this.sgWidget) return; - this.sgWidget.off("preparing", this.onWidgetPreparing); + this.sgWidget?.on("ready", this.onWidgetReady); this.sgWidget.off("error:preparing", this.updateRequiresClient); this.sgWidget.off("capabilitiesNotified", this.updateRequiresClient); } @@ -447,7 +447,7 @@ export default class AppTile extends React.Component { this.sgWidget?.stopMessaging({ forceDestroy: true }); } - private onWidgetPreparing = (): void => { + private onWidgetReady = (): void => { this.setState({ loading: false }); }; From df821ca584641253d095c3aff28668ad76f258cb Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 28 Feb 2024 21:24:44 +0100 Subject: [PATCH 3/8] wait until widget loading is over before comparing screenshots Signed-off-by: Timo K --- playwright/e2e/widgets/layout.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/widgets/layout.spec.ts b/playwright/e2e/widgets/layout.spec.ts index a5dd856a931..00d12f41b15 100644 --- a/playwright/e2e/widgets/layout.spec.ts +++ b/playwright/e2e/widgets/layout.spec.ts @@ -25,7 +25,7 @@ const WIDGET_HTML = ` Fake Widget - + Hello World @@ -38,7 +38,7 @@ test.describe("Widget Layout", () => { let roomId: string; let widgetUrl: string; - test.beforeEach(async ({ webserver, app, user }) => { + test.beforeEach(async ({ webserver, app, user, page }) => { widgetUrl = webserver.start(WIDGET_HTML); roomId = await app.client.createRoom({ name: ROOM_NAME }); @@ -76,6 +76,7 @@ test.describe("Widget Layout", () => { // open the room await app.viewRoomByName(ROOM_NAME); + await page.waitForSelector(".widget_body_class"); }); test("should be set properly", async ({ page }) => { From cbf98c22aac1da52276d2e398904842163539313 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 29 Feb 2024 14:09:49 +0100 Subject: [PATCH 4/8] fix waitForIFrame=true widgets Signed-off-by: Timo K --- src/components/views/elements/AppTile.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index b2f5bb9467a..685ea0a1d54 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -353,6 +353,7 @@ export default class AppTile extends React.Component { private setupSgListeners(): void { this.sgWidget?.on("ready", this.onWidgetReady); + this.sgWidget?.on("preparing", this.onWidgetPreparing); this.sgWidget?.on("error:preparing", this.updateRequiresClient); // emits when the capabilities have been set up or changed this.sgWidget?.on("capabilitiesNotified", this.updateRequiresClient); @@ -361,6 +362,7 @@ export default class AppTile extends React.Component { private stopSgListeners(): void { if (!this.sgWidget) return; this.sgWidget?.on("ready", this.onWidgetReady); + this.sgWidget?.on("off", this.onWidgetPreparing); this.sgWidget.off("error:preparing", this.updateRequiresClient); this.sgWidget.off("capabilitiesNotified", this.updateRequiresClient); } @@ -446,7 +448,15 @@ export default class AppTile extends React.Component { this.sgWidget?.stopMessaging({ forceDestroy: true }); } - + private onWidgetPreparing = (): void => { + if (this.props.waitForIframeLoad) { + // If the widget is configured to start immediately after the IFrame is loaded, (it does not have any custom initialization) + // we need to consider it ready now since it might not even use the widget api. + this.onWidgetReady(); + } + // If waitForIframeLoad = false. We do nothing here. The widget is responsible to send a content_loaded action + // and the messaging will emit "ready" (-> this will call onWidgetReady) + }; private onWidgetReady = (): void => { this.setState({ loading: false }); }; From 0691d5af8c3077df338f742690bf72cb1c580292 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 29 Feb 2024 14:35:22 +0100 Subject: [PATCH 5/8] test Signed-off-by: Timo K --- playwright/e2e/widgets/layout.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/playwright/e2e/widgets/layout.spec.ts b/playwright/e2e/widgets/layout.spec.ts index 00d12f41b15..6393dcb5a82 100644 --- a/playwright/e2e/widgets/layout.spec.ts +++ b/playwright/e2e/widgets/layout.spec.ts @@ -76,7 +76,8 @@ test.describe("Widget Layout", () => { // open the room await app.viewRoomByName(ROOM_NAME); - await page.waitForSelector(".widget_body_class"); + // TODO try the end-to-end tests without this + // await page.waitForSelector(".widget_body_class"); }); test("should be set properly", async ({ page }) => { From 780ce48ddb50fc5baaf89e4652cd685d36d679f8 Mon Sep 17 00:00:00 2001 From: Timo K Date: Fri, 1 Mar 2024 12:14:23 +0100 Subject: [PATCH 6/8] always start with loading true. + cleanup Signed-off-by: Timo K --- playwright/e2e/widgets/layout.spec.ts | 6 ++---- src/components/views/elements/AppTile.tsx | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/playwright/e2e/widgets/layout.spec.ts b/playwright/e2e/widgets/layout.spec.ts index 6393dcb5a82..a5dd856a931 100644 --- a/playwright/e2e/widgets/layout.spec.ts +++ b/playwright/e2e/widgets/layout.spec.ts @@ -25,7 +25,7 @@ const WIDGET_HTML = ` Fake Widget - + Hello World @@ -38,7 +38,7 @@ test.describe("Widget Layout", () => { let roomId: string; let widgetUrl: string; - test.beforeEach(async ({ webserver, app, user, page }) => { + test.beforeEach(async ({ webserver, app, user }) => { widgetUrl = webserver.start(WIDGET_HTML); roomId = await app.client.createRoom({ name: ROOM_NAME }); @@ -76,8 +76,6 @@ test.describe("Widget Layout", () => { // open the room await app.viewRoomByName(ROOM_NAME); - // TODO try the end-to-end tests without this - // await page.waitForSelector(".widget_body_class"); }); test("should be set properly", async ({ page }) => { diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index 685ea0a1d54..0ec1e06fd28 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -250,7 +250,7 @@ export default class AppTile extends React.Component { return { initialising: true, // True while we are mangling the widget URL // True while the iframe content is loading - loading: !PersistedElement.isMounted(this.persistKey), + loading: true, // Assume that widget has permission to load if we are the user who // added it to the room, or if explicitly granted by the user hasPermissionToLoad: this.hasPermissionToLoad(newProps), @@ -361,8 +361,8 @@ export default class AppTile extends React.Component { private stopSgListeners(): void { if (!this.sgWidget) return; - this.sgWidget?.on("ready", this.onWidgetReady); - this.sgWidget?.on("off", this.onWidgetPreparing); + this.sgWidget?.off("ready", this.onWidgetReady); + this.sgWidget?.off("preparing", this.onWidgetPreparing); this.sgWidget.off("error:preparing", this.updateRequiresClient); this.sgWidget.off("capabilitiesNotified", this.updateRequiresClient); } From 409535af4703afe81188801a051a928be91f126f Mon Sep 17 00:00:00 2001 From: Timo K Date: Fri, 1 Mar 2024 13:07:13 +0100 Subject: [PATCH 7/8] simplify loading Signed-off-by: Timo K --- src/components/views/elements/AppTile.tsx | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx index 0ec1e06fd28..34c773d3c94 100644 --- a/src/components/views/elements/AppTile.tsx +++ b/src/components/views/elements/AppTile.tsx @@ -249,8 +249,9 @@ export default class AppTile extends React.Component { private getNewState(newProps: IProps): IState { return { initialising: true, // True while we are mangling the widget URL - // True while the iframe content is loading - loading: true, + // Don't show loading at all if the widget is ready once the IFrame is loaded (waitForIframeLoad = true). + // We only need the loading screen if the widget sends a contentLoaded event (waitForIframeLoad = false). + loading: !this.props.waitForIframeLoad && !PersistedElement.isMounted(this.persistKey), // Assume that widget has permission to load if we are the user who // added it to the room, or if explicitly granted by the user hasPermissionToLoad: this.hasPermissionToLoad(newProps), @@ -312,7 +313,6 @@ export default class AppTile extends React.Component { if (this.props.room) { this.context.on(RoomEvent.MyMembership, this.onMyMembership); } - this.allowedWidgetsWatchRef = SettingsStore.watchSetting("allowedWidgets", null, this.onAllowedWidgetsChange); // Widget action listeners this.dispatcherRef = dis.register(this.onAction); @@ -353,7 +353,6 @@ export default class AppTile extends React.Component { private setupSgListeners(): void { this.sgWidget?.on("ready", this.onWidgetReady); - this.sgWidget?.on("preparing", this.onWidgetPreparing); this.sgWidget?.on("error:preparing", this.updateRequiresClient); // emits when the capabilities have been set up or changed this.sgWidget?.on("capabilitiesNotified", this.updateRequiresClient); @@ -362,7 +361,6 @@ export default class AppTile extends React.Component { private stopSgListeners(): void { if (!this.sgWidget) return; this.sgWidget?.off("ready", this.onWidgetReady); - this.sgWidget?.off("preparing", this.onWidgetPreparing); this.sgWidget.off("error:preparing", this.updateRequiresClient); this.sgWidget.off("capabilitiesNotified", this.updateRequiresClient); } @@ -448,15 +446,6 @@ export default class AppTile extends React.Component { this.sgWidget?.stopMessaging({ forceDestroy: true }); } - private onWidgetPreparing = (): void => { - if (this.props.waitForIframeLoad) { - // If the widget is configured to start immediately after the IFrame is loaded, (it does not have any custom initialization) - // we need to consider it ready now since it might not even use the widget api. - this.onWidgetReady(); - } - // If waitForIframeLoad = false. We do nothing here. The widget is responsible to send a content_loaded action - // and the messaging will emit "ready" (-> this will call onWidgetReady) - }; private onWidgetReady = (): void => { this.setState({ loading: false }); }; From 1c9df2d030c5c98c884015cf7d335ea5922f3477 Mon Sep 17 00:00:00 2001 From: Timo K Date: Fri, 1 Mar 2024 13:24:20 +0100 Subject: [PATCH 8/8] update snapshots (start not in loading state for waitForIframe = true widgets) Signed-off-by: Timo K --- .../views/elements/__snapshots__/AppTile-test.tsx.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap index 4771fc776e4..eee2c86a2ce 100644 --- a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap +++ b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap @@ -47,7 +47,7 @@ exports[`AppTile destroys non-persisted right panel widget on room change 1`] = id="1" >