Fix room list often showing the wrong icons for calls (#32881)

* Give rooms with calls a proper accessible description

Besides improving accessibility, this makes it possible to check for the presence of a call indicator in the room list in Playwright tests.

* Make room list react to calls in a room, even when not connected to them

To use the results of CallStore.getRoom reactively, you need to listen for Call events, not ConnectedCalls events.

* Don't assume that every call starts off as a video call

If a Call object is created by way of someone starting a voice call, then of course the call's initial type needs to be 'voice'.

* Make room list items react to changes in call type

The type of a call may change over time; therefore room list items explicitly need to react to the changes.

* Update a call's type before notifying listeners of the change

If we notify listeners of a change in a call's type before actually making that change, the listeners will be working with glitched state. This would cause the room list to show the wrong call type in certain situations.

* Ignore the Vitest attachments directory

Author: robintown

.gitignore

@@ -27,6 +27,7 @@ storybook-static
 /packages/shared-components/node_modules
 /packages/shared-components/dist
 /packages/shared-components/src/i18nKeys.d.ts
+/packages/shared-components/.vitest-attachments
 
 # TSC incremental compilation information
 *.tsbuildinfo

apps/web/playwright/e2e/voip/element-call.spec.ts

@@ -189,25 +189,31 @@ test.describe("Element Call", () => {
             expect(hash.get("skipLobby")).toEqual("true");
         });
 
-        test("should be able to join a call in progress", async ({ page, user, bot, room, app }) => {
-            await app.viewRoomById(room.roomId);
-            // Allow bob to create a call
-            await expect(page.getByText("Bob and one other were invited and joined")).toBeVisible();
-            await app.client.setPowerLevel(room.roomId, bot.credentials.userId, 50);
-            // Fake a start of a call
-            await sendRTCState(bot, room.roomId);
-            const button = page.getByTestId("join-call-button");
-            await expect(button).toBeInViewport({ timeout: 5000 });
-            // And test joining
-            await button.click();
-            const frameUrlStr = await page.locator("iframe").getAttribute("src");
-            await expect(frameUrlStr).toBeDefined();
-            const url = new URL(frameUrlStr);
-            const hash = new URLSearchParams(url.hash.slice(1));
-            assertCommonCallParameters(url.searchParams, hash, user, room);
+        ["voice", "video"].forEach((callType) => {
+            test(`should be able to join a ${callType} call in progress`, async ({ page, user, bot, room, app }) => {
+                await app.viewRoomById(room.roomId);
+                // Allow bob to create a call
+                await expect(page.getByText("Bob and one other were invited and joined")).toBeVisible();
+                await app.client.setPowerLevel(room.roomId, bot.credentials.userId, 50);
+                // Fake a start of a call
+                await sendRTCState(bot, room.roomId, undefined, callType === "voice" ? "audio" : "video");
+                const button = page.getByTestId("join-call-button");
+                await expect(button).toBeInViewport({ timeout: 5000 });
+                // Room list should show that a call is ongoing
+                await expect(
+                    page.getByRole("option", { name: `Open room TestRoom with a ${callType} call.` }),
+                ).toBeVisible();
+                // And test joining
+                await button.click();
+                const frameUrlStr = await page.locator("iframe").getAttribute("src");
+                await expect(frameUrlStr).toBeDefined();
+                const url = new URL(frameUrlStr);
+                const hash = new URLSearchParams(url.hash.slice(1));
+                assertCommonCallParameters(url.searchParams, hash, user, room);
 
-            expect(hash.get("intent")).toEqual("join_existing");
-            expect(hash.get("skipLobby")).toEqual(null);
+                expect(hash.get("intent")).toEqual("join_existing");
+                expect(hash.get("skipLobby")).toEqual(null);
+            });
         });
 
         [true, false].forEach((skipLobbyToggle) => {

apps/web/src/models/Call.ts

@@ -108,16 +108,15 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
     protected readonly widgetUid: string;
     protected readonly room: Room;
 
-    private _callType: CallType = CallType.Video;
+    private _callType: CallType;
     public get callType(): CallType {
         return this._callType;
     }
 
     protected set callType(callType: CallType) {
-        if (this._callType !== callType) {
-            this.emit(CallEvent.CallTypeChanged, callType);
-        }
+        const prevCallType = this._callType;
         this._callType = callType;
+        if (callType !== prevCallType) this.emit(CallEvent.CallTypeChanged, callType);
     }
 
     /**
@@ -184,11 +183,13 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
          */
         public readonly widget: IApp,
         protected readonly client: MatrixClient,
+        initialCallType: CallType,
     ) {
         super();
         this.widgetUid = WidgetUtils.getWidgetUid(this.widget);
         this.room = this.client.getRoom(this.roomId)!;
         WidgetMessagingStore.instance.on(WidgetMessagingStoreEvent.StopMessaging, this.onStopMessaging);
+        this._callType = initialCallType;
     }
 
     /**
@@ -347,7 +348,7 @@ export class JitsiCall extends Call {
     private participantsExpirationTimer: number | null = null;
 
     private constructor(widget: IApp, client: MatrixClient) {
-        super(widget, client);
+        super(widget, client, CallType.Video);
 
         this.room.on(RoomStateEvent.Update, this.onRoomState);
         this.on(CallEvent.ConnectionState, this.onConnectionState);
@@ -899,7 +900,7 @@ export class ElementCall extends Call {
         widget: IApp,
         client: MatrixClient,
     ) {
-        super(widget, client);
+        super(widget, client, session.getConsensusCallIntent() === "audio" ? CallType.Voice : CallType.Video);
 
         this.session.on(MatrixRTCSessionEvent.MembershipsChanged, this.onMembershipChanged);
         this.client.matrixRTC.on(MatrixRTCSessionManagerEvents.SessionEnded, this.checkDestroy);

apps/web/src/viewmodels/room-list/RoomListItemViewModel.ts

@@ -87,7 +87,7 @@ export class RoomListItemViewModel
         });
 
         // Subscribe to call state changes
-        this.disposables.trackListener(CallStore.instance, CallStoreEvent.ConnectedCalls, this.onCallStateChanged);
+        this.disposables.trackListener(CallStore.instance, CallStoreEvent.Call, this.onCallStateChanged);
         // If there is an active call for this room, listen to participant changes
         this.listenToCallParticipants();
 
@@ -102,6 +102,7 @@ export class RoomListItemViewModel
     public dispose(): void {
         super.dispose();
         this.currentCall?.off(CallEvent.Participants, this.onCallParticipantsChanged);
+        this.currentCall?.off(CallEvent.CallTypeChanged, this.onCallTypeChanged);
     }
 
     private onNotificationChanged = (): void => {
@@ -128,16 +129,25 @@ export class RoomListItemViewModel
         this.updateItem();
     };
 
+    /**
+     * Handler for call type changes. Only updates the item if the call type is actually present in the snapshot.
+     */
+    private onCallTypeChanged = (): void => {
+        if (this.snapshot.current.notification.callType !== undefined) this.updateItem();
+    };
+
     /**
      * Listen to participant changes for the current call in this room (if any) to trigger updates when participants join/leave the call.
      */
     private listenToCallParticipants(): void {
         const call = CallStore.instance.getCall(this.props.room.roomId);
 
-        // Remove listener from previous call (if any) and add to new call to track participant changes
+        // Remove listeners from previous call (if any) and add to new call to track changes
         if (call !== this.currentCall) {
             this.currentCall?.off(CallEvent.Participants, this.onCallParticipantsChanged);
+            this.currentCall?.off(CallEvent.CallTypeChanged, this.onCallTypeChanged);
             call?.on(CallEvent.Participants, this.onCallParticipantsChanged);
+            call?.on(CallEvent.CallTypeChanged, this.onCallTypeChanged);
         }
         this.currentCall = call;
     }

apps/web/test/test-utils/call.ts

@@ -18,6 +18,7 @@ import {
     RoomStateEvent,
     type IContent,
 } from "matrix-js-sdk/src/matrix";
+import { CallType } from "matrix-js-sdk/src/webrtc/call";
 import { mocked, type Mocked } from "jest-mock";
 import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc";
 
@@ -52,6 +53,7 @@ export class MockedCall extends Call {
                 waitForIframeLoad: false,
             },
             room.client,
+            CallType.Video,
         );
         this.groupCall = { creationTs: this.event.getTs() } as unknown as GroupCall;
     }

apps/web/test/test-utils/test-utils.ts

@@ -381,6 +381,7 @@ export function createStubMatrixRTC(): MatrixRTCSessionManager {
         const session = new EventEmitter() as MatrixRTCSession;
         session.memberships = [];
         session.getOldestMembership = () => undefined;
+        session.getConsensusCallIntent = () => "video";
         return session;
     });
     return {

apps/web/test/unit-tests/models/Call-test.ts

@@ -26,6 +26,7 @@ import {
     MatrixRTCSession,
     MatrixRTCSessionEvent,
 } from "matrix-js-sdk/src/matrixrtc";
+import { CallType } from "matrix-js-sdk/src/webrtc/call";
 
 import type { Mocked } from "jest-mock";
 import type { ClientWidgetApi } from "matrix-widget-api";
@@ -987,6 +988,35 @@ describe("ElementCall", () => {
             call.off(CallEvent.Participants, onParticipants);
         });
 
+        it("emits events when call type changes", async () => {
+            const onCallTypeChanged = jest.fn();
+            call.on(CallEvent.CallTypeChanged, onCallTypeChanged);
+            // Should default to video when unknown
+            expect(call.callType).toBe(CallType.Video);
+
+            // Change call type to voice
+            roomSession.memberships = [
+                { sender: alice.userId, deviceId: "alices_device", callIntent: "audio" } as Mocked<CallMembership>,
+            ];
+            roomSession.getConsensusCallIntent.mockReturnValue("audio");
+            roomSession.emit(MatrixRTCSessionEvent.MembershipsChanged, [], []);
+
+            expect(call.callType).toBe(CallType.Voice);
+            expect(onCallTypeChanged.mock.calls).toEqual([[CallType.Voice]]);
+
+            // Change call type back to video
+            roomSession.memberships = [
+                { sender: alice.userId, deviceId: "alices_device", callIntent: "video" } as Mocked<CallMembership>,
+            ];
+            roomSession.getConsensusCallIntent.mockReturnValue("video");
+            roomSession.emit(MatrixRTCSessionEvent.MembershipsChanged, [], []);
+
+            expect(call.callType).toBe(CallType.Video);
+            expect(onCallTypeChanged.mock.calls).toEqual([[CallType.Voice], [CallType.Video]]);
+
+            call.off(CallEvent.CallTypeChanged, onCallTypeChanged);
+        });
+
         it("ends the call immediately if the session ended", async () => {
             await connect(call, widgetApi);
             const onDestroy = jest.fn();

apps/web/test/viewmodels/room-list/RoomListItemViewModel-test.tsx

@@ -5,6 +5,7 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
+import EventEmitter from "events";
 import {
     type MatrixClient,
     type MatrixEvent,
@@ -26,7 +27,7 @@ import { DefaultTagID } from "../../../src/stores/room-list-v3/skip-list/tag";
 import dispatcher from "../../../src/dispatcher/dispatcher";
 import { Action } from "../../../src/dispatcher/actions";
 import { CallStore } from "../../../src/stores/CallStore";
-import type { Call } from "../../../src/models/Call";
+import { CallEvent, type Call } from "../../../src/models/Call";
 import { RoomListItemViewModel } from "../../../src/viewmodels/room-list/RoomListItemViewModel";
 
 jest.mock("../../../src/viewmodels/room-list/utils", () => ({
@@ -436,6 +437,28 @@ describe("RoomListItemViewModel", () => {
             // The new call must have a listener registered
             expect(secondCall.on).toHaveBeenCalledWith("participants", expect.any(Function));
         });
+
+        it("should listen to call type changes", async () => {
+            // Start with a voice call
+            let callType = CallType.Voice;
+            const mockCall = new (class extends EventEmitter {
+                get callType() {
+                    return callType;
+                }
+                participants = new Map([[matrixClient.getUserId()!, {}]]);
+            })() as unknown as Call;
+            jest.spyOn(CallStore.instance, "getCall").mockReturnValue(mockCall);
+
+            viewModel = new RoomListItemViewModel({ room, client: matrixClient });
+            await flushPromises();
+
+            expect(viewModel.getSnapshot().notification.callType).toBe("voice");
+
+            // Now turn it into a video call
+            callType = CallType.Video;
+            mockCall.emit(CallEvent.CallTypeChanged, callType);
+            expect(viewModel.getSnapshot().notification.callType).toBe("video");
+        });
     });
 
     describe("Room name updates", () => {