Simple refactor for skipLobby (#30848)

* Simple refactor for skipLobby (and remove returnToLobby)

* Tidyup

* Remove unused tests

* Add video room support.

* Add a test for video rooms

* tidy

* Document
This commit is contained in:
Will Hunt
2025-09-25 13:46:37 +01:00
committed by GitHub
parent 65eb4ce1d3
commit 75083c2e80
11 changed files with 184 additions and 164 deletions

View File

@@ -25,8 +25,6 @@ function assertCommonCallParameters(
expect(hash.get("deviceId")).toEqual(user.deviceId);
expect(hash.get("roomId")).toEqual(room.roomId);
expect(hash.get("preload")).toEqual("false");
expect(hash.get("returnToLobby")).toEqual("false");
}
async function sendRTCState(bot: Bot, roomId: string, notification?: "ring" | "notification") {
@@ -125,7 +123,7 @@ test.describe("Element Call", () => {
const hash = new URLSearchParams(url.hash.slice(1));
assertCommonCallParameters(url.searchParams, hash, user, room);
expect(hash.get("intent")).toEqual("start_call");
expect(hash.get("skipLobby")).toEqual("false");
expect(hash.get("skipLobby")).toEqual(null);
});
test("should be able to skip lobby by holding down shift", async ({ page, user, bot, room, app }) => {
@@ -165,7 +163,7 @@ test.describe("Element Call", () => {
assertCommonCallParameters(url.searchParams, hash, user, room);
expect(hash.get("intent")).toEqual("join_existing");
expect(hash.get("skipLobby")).toEqual("false");
expect(hash.get("skipLobby")).toEqual(null);
});
[true, false].forEach((skipLobbyToggle) => {
@@ -232,7 +230,7 @@ test.describe("Element Call", () => {
const hash = new URLSearchParams(url.hash.slice(1));
assertCommonCallParameters(url.searchParams, hash, user, room);
expect(hash.get("intent")).toEqual("start_call_dm");
expect(hash.get("skipLobby")).toEqual("false");
expect(hash.get("skipLobby")).toEqual(null);
});
test("should be able to skip lobby by holding down shift", async ({ page, user, room, app }) => {
@@ -271,7 +269,7 @@ test.describe("Element Call", () => {
assertCommonCallParameters(url.searchParams, hash, user, room);
expect(hash.get("intent")).toEqual("join_existing_dm");
expect(hash.get("skipLobby")).toEqual("false");
expect(hash.get("skipLobby")).toEqual(null);
});
[true, false].forEach((skipLobbyToggle) => {
@@ -309,4 +307,33 @@ test.describe("Element Call", () => {
);
});
});
test.describe("Video Rooms", () => {
test.use({
config: {
features: {
feature_video_rooms: true,
feature_element_call_video_rooms: true,
},
},
});
test("should be able to create and join a video room", async ({ page, user }) => {
await page.getByRole("navigation", { name: "Room list" }).getByRole("button", { name: "Add" }).click();
await page.getByRole("menuitem", { name: "New video room" }).click();
await page.getByRole("textbox", { name: "Name" }).fill("Test room");
await page.getByRole("button", { name: "Create video room" }).click();
await expect(page).toHaveURL(new RegExp(`/#/room/`));
const roomId = new URL(page.url()).hash.slice("#/room/".length);
const frameUrlStr = await page.locator("iframe").getAttribute("src");
await expect(frameUrlStr).toBeDefined();
// Ensure we set the correct parameters for ECall.
const url = new URL(frameUrlStr);
const hash = new URLSearchParams(url.hash.slice(1));
assertCommonCallParameters(url.searchParams, hash, user, { roomId });
expect(hash.get("intent")).toEqual("join_existing");
expect(hash.get("skipLobby")).toEqual("false");
expect(hash.get("returnToLobby")).toEqual("true");
});
});
});

View File

@@ -2609,7 +2609,6 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
<CallView
room={this.state.room}
resizing={this.state.resizing}
skipLobby={this.context.roomViewStore.skipCallLobby() ?? false}
role="main"
onClose={this.onCallClose}
/>

View File

@@ -35,7 +35,7 @@ const RoomCallBannerInner: React.FC<RoomCallBannerProps> = ({ roomId, call }) =>
action: Action.ViewRoom,
room_id: roomId,
view_call: true,
skipLobby: "shiftKey" in ev ? ev.shiftKey : false,
skipLobby: ("shiftKey" in ev && ev.shiftKey) || undefined,
metricsTrigger: undefined,
});
},

View File

@@ -21,12 +21,11 @@ interface JoinCallViewProps {
room: Room;
resizing: boolean;
call: Call;
skipLobby?: boolean;
role?: AriaRole;
onClose: () => void;
}
const JoinCallView: FC<JoinCallViewProps> = ({ room, resizing, call, skipLobby, role, onClose }) => {
const JoinCallView: FC<JoinCallViewProps> = ({ room, resizing, call, role, onClose }) => {
const cli = useContext(MatrixClientContext);
useTypedEventEmitter(call, CallEvent.Close, onClose);
@@ -35,12 +34,6 @@ const JoinCallView: FC<JoinCallViewProps> = ({ room, resizing, call, skipLobby,
call.clean();
}, [call]);
useEffect(() => {
// Always update the widget data so that we don't ignore "skipLobby" accidentally.
call.widget.data ??= {};
call.widget.data.skipLobby = skipLobby;
}, [call.widget, skipLobby]);
const disconnectAllOtherCalls: () => Promise<void> = useCallback(async () => {
// The stickyPromise has to resolve before the widget actually becomes sticky.
// We only let the widget become sticky after disconnecting all other active calls.
@@ -69,7 +62,6 @@ const JoinCallView: FC<JoinCallViewProps> = ({ room, resizing, call, skipLobby,
interface CallViewProps {
room: Room;
resizing: boolean;
skipLobby?: boolean;
role?: AriaRole;
/**
* Callback for when the user closes the call.
@@ -77,19 +69,8 @@ interface CallViewProps {
onClose: () => void;
}
export const CallView: FC<CallViewProps> = ({ room, resizing, skipLobby, role, onClose }) => {
export const CallView: FC<CallViewProps> = ({ room, resizing, role, onClose }) => {
const call = useCall(room.roomId);
return (
call && (
<JoinCallView
room={room}
resizing={resizing}
call={call}
skipLobby={skipLobby}
role={role}
onClose={onClose}
/>
)
);
return call && <JoinCallView room={room} resizing={resizing} call={call} role={role} onClose={onClose} />;
};

View File

@@ -229,7 +229,7 @@ export const useRoomCall = (
if (widget && promptPinWidget) {
WidgetLayoutStore.instance.moveToContainer(room, widget, Container.Top);
} else {
placeCall(room, CallType.Voice, callPlatformType, evt?.shiftKey ?? false);
placeCall(room, CallType.Voice, callPlatformType, evt?.shiftKey || undefined);
}
},
[promptPinWidget, room, widget],
@@ -240,7 +240,9 @@ export const useRoomCall = (
if (widget && promptPinWidget) {
WidgetLayoutStore.instance.moveToContainer(room, widget, Container.Top);
} else {
placeCall(room, CallType.Video, callPlatformType, evt?.shiftKey ?? false);
// If we have pressed shift then always skip the lobby, otherwise `undefined` will defer
// to the defaults of the call implementation.
placeCall(room, CallType.Video, callPlatformType, evt?.shiftKey || undefined);
}
},
[widget, promptPinWidget, room],

View File

@@ -199,7 +199,7 @@ export abstract class Call extends TypedEventEmitter<CallEvent, CallEventHandler
* The widget associated with the call must be active for this to succeed.
* Only call this if the call state is: ConnectionState.Disconnected.
*/
public async start(): Promise<void> {
public async start(_params?: WidgetGenerationParameters): Promise<void> {
const messagingStore = WidgetMessagingStore.instance;
this.messaging = messagingStore.getMessagingForUid(this.widgetUid) ?? null;
if (!this.messaging) {
@@ -547,6 +547,17 @@ export enum ElementCallIntent {
JoinExistingDM = "join_existing_dm",
}
/**
* Parameters to be passed during widget creation.
* These parameters are hints only, and may not be accepted by the implementation.
*/
export interface WidgetGenerationParameters {
/**
* Skip showing the lobby screen of a call.
*/
skipLobby?: boolean;
}
/**
* A group call using MSC3401 and Element Call as a backend.
* (somewhat cheekily named)
@@ -565,32 +576,32 @@ export class ElementCall extends Call {
this.checkDestroy();
}
private static generateWidgetUrl(client: MatrixClient, roomId: string): URL {
const baseUrl = window.location.href;
let url = new URL("./widgets/element-call/index.html#", baseUrl); // this strips hash fragment from baseUrl
const elementCallUrl = SettingsStore.getValue("Developer.elementCallUrl");
if (elementCallUrl) url = new URL(elementCallUrl);
// Splice together the Element Call URL for this call
const params = new URLSearchParams({
confineToRoom: "true", // Only show the call interface for the configured room
// Template variables are used, so that this can be configured using the widget data.
skipLobby: "$skipLobby", // Skip the lobby in case we show a lobby component of our own.
returnToLobby: "$returnToLobby", // Returns to the lobby (instead of blank screen) when the call ends. (For video rooms)
perParticipantE2EE: "$perParticipantE2EE",
header: "none", // Hide the header since our room header is enough
userId: client.getUserId()!,
deviceId: client.getDeviceId()!,
roomId: roomId,
baseUrl: client.baseUrl,
lang: getCurrentLanguage().replace("_", "-"),
fontScale: (FontWatcher.getRootFontSize() / FontWatcher.getBrowserDefaultFontSize()).toString(),
theme: "$org.matrix.msc2873.client_theme",
});
public widgetGenerationParameters: WidgetGenerationParameters = {};
/**
* Calculate the correct intent (and associated parameters) for an Element Call room. Paarameters
* will be applied to the `params` instance.
*
* @param params Existing URL parameters
* @param client The current client.
* @param roomId The room ID for the call.
*/
private static appendRoomParams(params: URLSearchParams, client: MatrixClient, roomId: string): void {
const room = client.getRoom(roomId);
if (room !== null && !isVideoRoom(room)) {
if (!room) {
// If the room isn't known, or the room is a video room then skip setting an intent.
return;
} else if (isVideoRoom(room)) {
// Video rooms already exist, so just treat as if we're joining a group call.
params.append("intent", ElementCallIntent.JoinExisting);
// Video rooms should always return to lobby.
params.append("returnToLobby", "true");
// Never skip the lobby, we always want to give the caller a chance to explicitly join.
params.append("skipLobby", "false");
// Never preload, as per below warning.
params.append("preload", "false");
return;
}
const isDM = !!DMRoomMap.shared().getUserIdForRoomId(room.roomId);
const oldestCallMember = client.matrixRTC.getRoomSession(room).getOldestMembership();
const hasCallStarted = !!oldestCallMember && oldestCallMember.sender !== client.getSafeUserId();
@@ -598,7 +609,6 @@ export class ElementCall extends Call {
// preload by default so we override here. This can be removed when that package
// is released and upgraded.
if (isDM) {
params.append("sendNotificationType", "ring");
if (hasCallStarted) {
params.append("intent", ElementCallIntent.JoinExistingDM);
params.append("preload", "false");
@@ -607,7 +617,6 @@ export class ElementCall extends Call {
params.append("preload", "false");
}
} else {
params.append("sendNotificationType", "notification");
if (hasCallStarted) {
params.append("intent", ElementCallIntent.JoinExisting);
params.append("preload", "false");
@@ -618,21 +627,25 @@ export class ElementCall extends Call {
}
}
const rageshakeSubmitUrl = SdkConfig.get("bug_report_endpoint_url");
if (rageshakeSubmitUrl) {
params.append("rageshakeSubmitUrl", rageshakeSubmitUrl);
/**
* Calculate the correct analytics parameters for an Element Call room. Paarameters
* will be applied to the `params` instance.
*
* @param params Existing URL parameters
* @param client The current client.
*/
private static appendAnalyticsParams(params: URLSearchParams, client: MatrixClient): void {
const posthogConfig = SdkConfig.get("posthog");
if (!posthogConfig || PosthogAnalytics.instance.getAnonymity() === Anonymity.Disabled) {
return;
}
const posthogConfig = SdkConfig.get("posthog");
if (posthogConfig && PosthogAnalytics.instance.getAnonymity() !== Anonymity.Disabled) {
const accountAnalyticsData = client.getAccountData(PosthogAnalytics.ANALYTICS_EVENT_TYPE)?.getContent();
// The analyticsID is passed directly to element call (EC) since this codepath is only for EC and no other widget.
// We really don't want the same analyticID's for the EC and EW posthog instances (Data on posthog should be limited/anonymized as much as possible).
// This is prohibited in EC where a hashed version of the analyticsID is used for the actual posthog identification.
// We can pass the raw EW analyticsID here since we need to trust EC with not sending sensitive data to posthog (EC has access to more sensible data than the analyticsID e.g. the username)
const analyticsID: string = accountAnalyticsData?.pseudonymousAnalyticsOptIn
? accountAnalyticsData?.id
: "";
const analyticsID: string = accountAnalyticsData?.pseudonymousAnalyticsOptIn ? accountAnalyticsData?.id : "";
params.append("analyticsID", analyticsID); // Legacy, deprecated in favour of posthogUserId
params.append("posthogUserId", analyticsID);
@@ -648,9 +661,49 @@ export class ElementCall extends Call {
}
}
/**
* Generate the correct Element Call widget URL for creating or joining a call in this room.
* Unless `Developer.elementCallUrl` is set, the widget will use the embedded Element Call package.
*
* @param client
* @param roomId
* @param opts
* @returns
*/
private static generateWidgetUrl(client: MatrixClient, roomId: string, opts: WidgetGenerationParameters = {}): URL {
const elementCallUrlOverride = SettingsStore.getValue("Developer.elementCallUrl");
const url = elementCallUrlOverride
? new URL(elementCallUrlOverride)
: // this strips hash fragment from baseUrl
new URL("./widgets/element-call/index.html#", window.location.href);
// Splice together the Element Call URL for this call
// Parameters can be found in https://github.com/element-hq/element-call/blob/livekit/src/UrlParams.ts.
const params = new URLSearchParams({
// Template variables are used, so that this can be configured using the widget data.
perParticipantE2EE: "$perParticipantE2EE",
userId: client.getUserId()!,
deviceId: client.getDeviceId()!,
roomId: roomId,
baseUrl: client.baseUrl,
lang: getCurrentLanguage().replace("_", "-"),
fontScale: (FontWatcher.getRootFontSize() / FontWatcher.getBrowserDefaultFontSize()).toString(),
theme: "$org.matrix.msc2873.client_theme",
});
if (typeof opts.skipLobby === "boolean") {
params.set("skipLobby", opts.skipLobby.toString());
}
const rageshakeSubmitUrl = SdkConfig.get("bug_report_endpoint_url");
if (rageshakeSubmitUrl) {
params.append("rageshakeSubmitUrl", rageshakeSubmitUrl);
}
if (SettingsStore.getValue("fallbackICEServerAllowed")) {
params.append("allowIceFallback", "true");
}
if (SettingsStore.getValue("feature_allow_screen_share_only_mode")) {
params.append("allowVoipWithNoMedia", "true");
}
@@ -667,6 +720,8 @@ export class ElementCall extends Call {
})
.forEach((font) => params.append("font", font));
}
this.appendAnalyticsParams(params, client);
this.appendRoomParams(params, client, roomId);
const replacedUrl = params.toString().replace(/%24/g, "$");
url.hash = `#?${replacedUrl}`;
@@ -674,27 +729,12 @@ export class ElementCall extends Call {
}
// Creates a new widget if there isn't any widget of typ Call in this room.
// Defaults for creating a new widget are: skipLobby = false
// When there is already a widget the current widget configuration will be used or can be overwritten
// by passing the according parameters (skipLobby).
private static createOrGetCallWidget(
roomId: string,
client: MatrixClient,
skipLobby: boolean | undefined,
returnToLobby: boolean | undefined,
): IApp {
private static createOrGetCallWidget(roomId: string, client: MatrixClient): IApp {
const ecWidget = WidgetStore.instance.getApps(roomId).find((app) => WidgetType.CALL.matches(app.type));
if (ecWidget) {
// Always update the widget data because even if the widget is already created,
// we might have settings changes that update the widget.
const overwrites: IWidgetData = {};
if (skipLobby !== undefined) {
overwrites.skipLobby = skipLobby;
}
if (returnToLobby !== undefined) {
overwrites.returnToLobby = returnToLobby;
}
ecWidget.data = ElementCall.getWidgetData(client, roomId, ecWidget?.data ?? {}, overwrites);
ecWidget.data = ElementCall.getWidgetData(client, roomId, ecWidget?.data ?? {}, {});
return ecWidget;
}
@@ -709,15 +749,7 @@ export class ElementCall extends Call {
type: WidgetType.CALL.preferred,
url: url.toString(),
waitForIframeLoad: false,
data: ElementCall.getWidgetData(
client,
roomId,
{},
{
skipLobby: skipLobby ?? false,
returnToLobby: returnToLobby ?? false,
},
),
data: ElementCall.getWidgetData(client, roomId, {}, {}),
},
roomId,
);
@@ -774,23 +806,26 @@ export class ElementCall extends Call {
// - or this is a call room. Then we also always want to show a call.
if (hasEcWidget || session.memberships.length !== 0 || room.isCallRoom()) {
// create a widget for the case we are joining a running call and don't have on yet.
const availableOrCreatedWidget = ElementCall.createOrGetCallWidget(
room.roomId,
room.client,
undefined,
isVideoRoom(room),
);
const availableOrCreatedWidget = ElementCall.createOrGetCallWidget(room.roomId, room.client);
return new ElementCall(session, availableOrCreatedWidget, room.client);
}
return null;
}
public static create(room: Room, skipLobby = false): void {
ElementCall.createOrGetCallWidget(room.roomId, room.client, skipLobby, isVideoRoom(room));
public static create(room: Room): void {
ElementCall.createOrGetCallWidget(room.roomId, room.client);
}
public async start(): Promise<void> {
public async start(widgetGenerationParameters: WidgetGenerationParameters): Promise<void> {
// Some parameters may only be set once the user has chosen to interact with the call, regenerate the URL
// at this point in case any of the parameters have changed.
this.widgetGenerationParameters = { ...this.widgetGenerationParameters, ...widgetGenerationParameters };
this.widget.url = ElementCall.generateWidgetUrl(
this.client,
this.roomId,
this.widgetGenerationParameters,
).toString();
await super.start();
this.messaging!.on(`action:${ElementWidgetActions.JoinCall}`, this.onJoin);
this.messaging!.on(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);

View File

@@ -109,10 +109,6 @@ interface State {
* Whether we're viewing a call or call lobby in this room
*/
viewingCall: boolean;
/**
* If we want the call to skip the lobby and immediately join
*/
skipLobby?: boolean;
promptAskToJoin: boolean;
@@ -359,13 +355,13 @@ export class RoomViewStore extends EventEmitter {
let call = CallStore.instance.getCall(payload.room_id);
// Start a call if not already there
if (call === null) {
ElementCall.create(room, false);
ElementCall.create(room);
call = CallStore.instance.getCall(payload.room_id)!;
}
call.presented = true;
// Immediately start the call. This will connect to all required widget events
// and allow the widget to show the lobby.
if (call.connectionState === ConnectionState.Disconnected) call.start();
if (call.connectionState === ConnectionState.Disconnected) call.start({ skipLobby: payload.skipLobby });
}
// If we switch to a different room from the call, we are no longer presenting it
const prevRoomCall = this.state.roomId ? CallStore.instance.getCall(this.state.roomId) : null;
@@ -413,7 +409,6 @@ export class RoomViewStore extends EventEmitter {
replyingToEvent: null,
viaServers: payload.via_servers ?? [],
wasContextSwitch: payload.context_switch ?? false,
skipLobby: payload.skipLobby,
viewingCall:
payload.view_call ??
(payload.room_id === this.state.roomId
@@ -470,7 +465,6 @@ export class RoomViewStore extends EventEmitter {
viaServers: payload.via_servers,
wasContextSwitch: payload.context_switch,
viewingCall: payload.view_call ?? false,
skipLobby: payload.skipLobby,
});
try {
const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(payload.room_alias);
@@ -739,10 +733,6 @@ export class RoomViewStore extends EventEmitter {
return this.state.viewingCall;
}
public skipCallLobby(): boolean | undefined {
return this.state.skipLobby;
}
/**
* Gets the current state of the 'promptForAskToJoin' property.
*

View File

@@ -242,7 +242,7 @@ export function IncomingCallToast({ notificationEvent }: Props): JSX.Element {
action: Action.ViewRoom,
room_id: room?.roomId,
view_call: true,
skipLobby: skipLobbyToggle ?? ("shiftKey" in e ? e.shiftKey : false),
skipLobby: ("shiftKey" in e && e.shiftKey) || skipLobbyToggle,
metricsTrigger: undefined,
});
},

View File

@@ -21,12 +21,13 @@ import PosthogTrackers from "../../PosthogTrackers";
* @param room the room to place the call in
* @param callType the type of call
* @param platformCallType the platform to pass the call on
* @param skipLobby Has the user indicated they would like to skip the lobby. Otherwise, defer to platform defaults.
*/
export const placeCall = async (
room: Room,
callType: CallType,
platformCallType: PlatformCallType,
skipLobby: boolean,
skipLobby?: boolean,
): Promise<void> => {
const { analyticsName } = getPlatformCallTypeProps(platformCallType);
PosthogTrackers.trackInteraction(analyticsName);

View File

@@ -82,13 +82,13 @@ describe("CallView", () => {
client.reEmitter.stopReEmitting(room, [RoomStateEvent.Events]);
});
const renderView = async (skipLobby = false, role: string | undefined = undefined): Promise<void> => {
render(<CallView room={room} resizing={false} skipLobby={skipLobby} role={role} onClose={() => {}} />);
const renderView = async (role: string | undefined = undefined): Promise<void> => {
render(<CallView room={room} resizing={false} role={role} onClose={() => {}} />);
await act(() => Promise.resolve()); // Let effects settle
};
it("accepts an accessibility role", async () => {
await renderView(undefined, "main");
await renderView("main");
screen.getByRole("main");
});
@@ -97,9 +97,4 @@ describe("CallView", () => {
await renderView();
expect(cleanSpy).toHaveBeenCalled();
});
it("updates the call's skipLobby parameter", async () => {
await renderView(true);
expect(call.widget.data?.skipLobby).toBe(true);
});
});

View File

@@ -679,7 +679,7 @@ describe("ElementCall", () => {
expect(urlParams.get("analyticsID")).toBeFalsy();
});
it("requests ringing notifications and correct intent in DMs", async () => {
it("requests correct intent in DMs", async () => {
getUserIdForRoomIdSpy.mockImplementation((roomId: string) =>
room.roomId === roomId ? "any-user" : undefined,
);
@@ -688,7 +688,6 @@ describe("ElementCall", () => {
if (!(call instanceof ElementCall)) throw new Error("Failed to create call");
const urlParams = new URLSearchParams(new URL(call.widget.url).hash.slice(1));
expect(urlParams.get("sendNotificationType")).toBe("ring");
expect(urlParams.get("intent")).toBe(ElementCallIntent.StartCallDM);
});
@@ -724,15 +723,6 @@ describe("ElementCall", () => {
const urlParams = new URLSearchParams(new URL(call.widget.url).hash.slice(1));
expect(urlParams.get("intent")).toBe(ElementCallIntent.JoinExisting);
});
it("requests visual notifications in non-DMs", async () => {
ElementCall.create(room);
const call = Call.get(room);
if (!(call instanceof ElementCall)) throw new Error("Failed to create call");
const urlParams = new URLSearchParams(new URL(call.widget.url).hash.slice(1));
expect(urlParams.get("sendNotificationType")).toBe("notification");
});
});
describe("instance in a non-video room", () => {
@@ -744,7 +734,7 @@ describe("ElementCall", () => {
jest.useFakeTimers();
jest.setSystemTime(0);
ElementCall.create(room, true);
ElementCall.create(room);
const maybeCall = ElementCall.get(room);
if (maybeCall === null) throw new Error("Failed to create call");
call = maybeCall;
@@ -762,7 +752,7 @@ describe("ElementCall", () => {
WidgetMessagingStore.instance.stopMessaging(widget, room.roomId);
expect(call.connectionState).toBe(ConnectionState.Disconnected);
const startup = call.start();
const startup = call.start({});
WidgetMessagingStore.instance.storeMessaging(widget, room.roomId, messaging);
await startup;
await connect(call, messaging, false);