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
This commit is contained in:
@@ -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) => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user