From 73fa27887d3073e0f5d01ab646397bd09f5d5cdd Mon Sep 17 00:00:00 2001 From: Skye Elliot Date: Fri, 31 Oct 2025 12:20:02 +0000 Subject: [PATCH] Move room name, avatar, and topic to IOpts. (#30981) --- src/components/structures/SpaceRoomView.tsx | 8 +- .../views/dialogs/CreateRoomDialog.tsx | 4 +- src/createRoom.ts | 22 ++- .../structures/SpaceRoomView-test.tsx | 179 +++++++++++++++++- .../views/dialogs/CreateRoomDialog-test.tsx | 160 +++++++++++++++- .../CreateRoomDialog-test.tsx.snap | 28 +-- test/unit-tests/createRoom-test.ts | 11 ++ 7 files changed, 380 insertions(+), 32 deletions(-) diff --git a/src/components/structures/SpaceRoomView.tsx b/src/components/structures/SpaceRoomView.tsx index c2aa7f2f10..f9210ec96b 100644 --- a/src/components/structures/SpaceRoomView.tsx +++ b/src/components/structures/SpaceRoomView.tsx @@ -329,8 +329,8 @@ const SpaceSetupFirstRooms: React.FC<{ return createRoom(space.client, { createOpts: { preset: isPublic ? Preset.PublicChat : Preset.PrivateChat, - name, }, + name, spinner: false, encryption: false, andView: false, @@ -423,7 +423,7 @@ const SpaceSetupPublicShare: React.FC = ({

{_t("create_space|share_heading", { - name: justCreatedOpts?.createOpts?.name || space.name, + name: justCreatedOpts?.name || space.name, })}

{_t("create_space|share_description")}
@@ -449,7 +449,7 @@ const SpaceSetupPrivateScope: React.FC<{

{_t("create_space|private_personal_heading")}

{_t("create_space|private_personal_description", { - name: justCreatedOpts?.createOpts?.name || space.name, + name: justCreatedOpts?.name || space.name, })}
@@ -686,7 +686,7 @@ export default class SpaceRoomView extends React.PureComponent { diff --git a/src/components/views/dialogs/CreateRoomDialog.tsx b/src/components/views/dialogs/CreateRoomDialog.tsx index d3771d102d..821182aff0 100644 --- a/src/components/views/dialogs/CreateRoomDialog.tsx +++ b/src/components/views/dialogs/CreateRoomDialog.tsx @@ -126,7 +126,7 @@ export default class CreateRoomDialog extends React.Component { const opts: IOpts = {}; const createOpts: IOpts["createOpts"] = (opts.createOpts = {}); opts.roomType = this.props.type; - createOpts.name = this.state.name; + opts.name = this.state.name; if (this.state.joinRule === JoinRule.Public) { createOpts.visibility = Visibility.Public; @@ -139,7 +139,7 @@ export default class CreateRoomDialog extends React.Component { } if (this.state.topic) { - createOpts.topic = this.state.topic; + opts.topic = this.state.topic; } if (this.state.noFederate) { createOpts.creation_content = { "m.federate": false }; diff --git a/src/createRoom.ts b/src/createRoom.ts index c3bc1e6b1a..8e39260921 100644 --- a/src/createRoom.ts +++ b/src/createRoom.ts @@ -49,7 +49,19 @@ import { ElementCallEventType, ElementCallMemberEventType } from "./call-types"; export interface IOpts { dmUserId?: string; - createOpts?: ICreateRoomOpts; + /** + * The name of the room to be created. + */ + name?: string; + /** + * The topic for the room. + */ + topic?: string; + /** + * Additional options to pass to the room creation API. + * Note: "name", "topic", and "avatar" should be set via their respective properties in IOpts. + */ + createOpts?: Omit; spinner?: boolean; guestAccess?: boolean; encryption?: boolean; @@ -251,6 +263,14 @@ export default async function createRoom(client: MatrixClient, opts: IOpts): Pro }); } + if (opts.name) { + createOpts.name = opts.name; + } + + if (opts.topic) { + createOpts.topic = opts.topic; + } + if (opts.avatar) { let url = opts.avatar; if (opts.avatar instanceof File) { diff --git a/test/unit-tests/components/structures/SpaceRoomView-test.tsx b/test/unit-tests/components/structures/SpaceRoomView-test.tsx index f181b90576..6610587a50 100644 --- a/test/unit-tests/components/structures/SpaceRoomView-test.tsx +++ b/test/unit-tests/components/structures/SpaceRoomView-test.tsx @@ -7,8 +7,8 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { mocked, type MockedObject } from "jest-mock"; -import { type MatrixClient, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; -import { render, cleanup, screen, fireEvent } from "jest-matrix-react"; +import { type MatrixClient, MatrixEvent, Preset, Room } from "matrix-js-sdk/src/matrix"; +import { render, cleanup, screen, fireEvent, waitFor, act } from "jest-matrix-react"; import { stubClient, mockPlatformPeg, unmockPlatformPeg, withClientContextRenderOptions } from "../../../test-utils"; import { RightPanelPhases } from "../../../../src/stores/right-panel/RightPanelStorePhases"; @@ -17,6 +17,8 @@ import ResizeNotifier from "../../../../src/utils/ResizeNotifier.ts"; import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks.ts"; import RightPanelStore from "../../../../src/stores/right-panel/RightPanelStore.ts"; import DMRoomMap from "../../../../src/utils/DMRoomMap.ts"; +import { type IOpts } from "../../../../src/createRoom.ts"; +import SpaceStore from "../../../../src/stores/spaces/SpaceStore.ts"; describe("SpaceRoomView", () => { let cli: MockedObject; @@ -86,7 +88,7 @@ describe("SpaceRoomView", () => { cleanup(); }); - const renderSpaceRoomView = async (): Promise> => { + const renderSpaceRoomView = async (justCreatedOpts?: IOpts): Promise> => { const resizeNotifier = new ResizeNotifier(); const permalinkCreator = new RoomPermalinkCreator(space); @@ -97,6 +99,7 @@ describe("SpaceRoomView", () => { permalinkCreator={permalinkCreator} onJoinButtonClicked={jest.fn()} onRejectButtonClicked={jest.fn()} + justCreatedOpts={justCreatedOpts} />, withClientContextRenderOptions(cli), ); @@ -113,5 +116,175 @@ describe("SpaceRoomView", () => { expect(spy).toHaveBeenCalledWith({ phase: RightPanelPhases.MemberList }); }); + + it("shows SpaceLandingAddButton context menu when Add button is clicked", async () => { + await renderSpaceRoomView(); + await expect(screen.findByText("Welcome to")).resolves.toBeVisible(); + + const addButton = screen.getByRole("button", { name: /add/i }); + fireEvent.click(addButton); + + expect(await screen.findByText(/new room/i)).toBeInTheDocument(); + expect(screen.getByText(/add existing room/i)).toBeInTheDocument(); + }); + }); + + describe("Spaces: creating a new community space", () => { + it("asks what topics you want to discuss, creates rooms for them and offers to share", async () => { + cli.createRoom.mockResolvedValueOnce({ room_id: "room1" }).mockResolvedValueOnce({ room_id: "room2" }); + SpaceStore.instance.addRoomToSpace = jest.fn(); + + // Given we are creating a space + const view = await renderSpaceRoomView({ + createOpts: { preset: Preset.PublicChat }, + name: "My MySpace Space", + }); + + // Then we are asked what topics we want + expect( + view.getByRole("heading", { name: "What are some things you want to discuss in My MySpace Space?" }), + ).toBeInTheDocument(); + + // And some defaults are suggested + expect(view.getByPlaceholderText(/general/i)).toBeInTheDocument(); + expect(view.getByPlaceholderText(/random/i)).toBeInTheDocument(); + expect(view.getByPlaceholderText(/support/i)).toBeInTheDocument(); + + // When we enter some room names + const input1 = view.getAllByRole("textbox")[0]; + const input2 = view.getAllByRole("textbox")[1]; + fireEvent.change(input1, { target: { value: "Room 1" } }); + fireEvent.change(input2, { target: { value: "Room 2" } }); + + // And click "Continue" + const button = view.getByRole("button", { name: "Continue" }); + fireEvent.click(button); + + // Then we create 2 rooms + await waitFor(() => { + expect(cli.createRoom).toHaveBeenCalledTimes(2); + }); + + // And offer the user to share this space + await waitFor(() => + expect(view.getByRole("heading", { name: "Share My MySpace Space" })).toBeInTheDocument(), + ); + expect(view.getByRole("button", { name: /Share invite link/ })).toBeInTheDocument(); + + // And allow them to continue to the first room + expect(view.getByRole("button", { name: "Go to my first room" })).toBeInTheDocument(); + }); + + it("shows 'Skip for now' when all fields are empty, 'Continue' when any field is filled", async () => { + // Given we are creating a space + const view = await renderSpaceRoomView({ + createOpts: { preset: Preset.PublicChat }, + }); + + // When we clear all the topics + view.getAllByRole("textbox").forEach((input) => fireEvent.change(input, { target: { value: "" } })); + + // Then the button reads "Skip for now" + expect(view.getByRole("button", { name: "Skip for now" })).toBeVisible(); + + // But when we enter a topic + fireEvent.change(view.getAllByRole("textbox")[0], { target: { value: "Room" } }); + + // Then the button says "Continue" + expect(view.getByRole("button", { name: "Continue" })).toBeVisible(); + }); + + it("shows error message if room creation fails", async () => { + // Given we are creating a space + const view = await renderSpaceRoomView({ + createOpts: { preset: Preset.PublicChat }, + }); + + // And when we create a room it will fail + cli.createRoom.mockRejectedValue(new Error("fail")); + + // When we create the space + fireEvent.change(view.getAllByRole("textbox")[0], { target: { value: "Room A" } }); + fireEvent.click(view.getByRole("button", { name: "Continue" })); + + // Then we display an error message because it failed + await waitFor(() => { + expect( + view.getByText((content) => content.toLowerCase().includes("failed to create initial space rooms")), + ).toBeInTheDocument(); + }); + }); + + it("disables button and shows 'Creating rooms' while busy", async () => { + // Given we are creating a space + const view = await renderSpaceRoomView({ + createOpts: { preset: Preset.PublicChat }, + }); + + // And creating a room will be slow + cli.createRoom.mockImplementation( + () => + new Promise(() => { + // This promise never resolves + }), + ); + + // When we create the space + fireEvent.change(view.getAllByRole("textbox")[0], { target: { value: "Room A" } }); + fireEvent.click(view.getByRole("button", { name: "Continue" })); + + // Then the "Creating rooms..." message is displayed + const button = view.getByRole("button"); + expect(button).toBeDisabled(); + expect(button).toHaveValue("Creating rooms…"); // Note the ellipsis + }); + }); + + describe("Spaces: creating a new private space", () => { + it("creates rooms inside a private space for a team", async () => { + cli.createRoom.mockResolvedValueOnce({ room_id: "room1" }).mockResolvedValueOnce({ room_id: "room2" }); + SpaceStore.instance.addRoomToSpace = jest.fn(); + + // When I create a private space + const view = await renderSpaceRoomView({ + createOpts: { preset: Preset.PrivateChat }, + name: "Private space", + topic: "a private space for team A", + }); + + // Then I am asked whether it's individual or team + expect(view.getByRole("heading", { name: "Who are you working with?" })).toBeInTheDocument(); + + // And when I say team + act(() => + view + .getByRole("button", { + name: "Me and my teammates A private space for you and your teammates", + }) + .click(), + ); + + // Then I am asked what rooms to create + expect(view.getByRole("heading", { name: "What projects are your team working on?" })).toBeInTheDocument(); + + expect(view.getByPlaceholderText(/general/i)).toBeInTheDocument(); + expect(view.getByPlaceholderText(/random/i)).toBeInTheDocument(); + expect(view.getByPlaceholderText(/support/i)).toBeInTheDocument(); + + // And when I enter some room names + const input1 = view.getAllByRole("textbox")[0]; + const input2 = view.getAllByRole("textbox")[1]; + fireEvent.change(input1, { target: { value: "Room 1" } }); + fireEvent.change(input2, { target: { value: "Room 2" } }); + + // And click "Continue" + const button = view.getByRole("button", { name: "Continue" }); + fireEvent.click(button); + + // Then the rooms are created + await waitFor(() => { + expect(cli.createRoom).toHaveBeenCalledTimes(2); + }); + }); }); }); diff --git a/test/unit-tests/components/views/dialogs/CreateRoomDialog-test.tsx b/test/unit-tests/components/views/dialogs/CreateRoomDialog-test.tsx index 5fa0251129..6ac423570f 100644 --- a/test/unit-tests/components/views/dialogs/CreateRoomDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/CreateRoomDialog-test.tsx @@ -8,10 +8,10 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { fireEvent, render, screen, within } from "jest-matrix-react"; -import { JoinRule, MatrixError, Preset, Visibility } from "matrix-js-sdk/src/matrix"; +import { type Room, JoinRule, MatrixError, Preset, Visibility } from "matrix-js-sdk/src/matrix"; import CreateRoomDialog from "../../../../../src/components/views/dialogs/CreateRoomDialog"; -import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser } from "../../../../test-utils"; +import { flushPromises, getMockClientWithEventEmitter, mkSpace, mockClientMethodsUser } from "../../../../test-utils"; import SettingsStore from "../../../../../src/settings/SettingsStore"; import { UIFeature } from "../../../../../src/settings/UIFeature"; @@ -58,6 +58,55 @@ describe("", () => { expect(screen.getByLabelText("Name")).toHaveDisplayValue(defaultName); }); + it("should include topic in room creation options", async () => { + const onFinished = jest.fn(); + render(); + await flushPromises(); + + const topic = "This is a test topic"; + + // Set room name and topic. + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Room with topic" } }); + fireEvent.change(screen.getByLabelText("Topic (optional)"), { target: { value: topic } }); + + // Create the room. + fireEvent.click(screen.getByText("Create room")); + await flushPromises(); + + expect(onFinished).toHaveBeenCalledWith( + true, + expect.objectContaining({ + name: "Room with topic", + topic, + }), + ); + }); + + it("should include no federate option in room creation options when enabled", async () => { + const onFinished = jest.fn(); + render(); + await flushPromises(); + + // Set room name, and disable federation. + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "NoFederate Room" } }); + fireEvent.click(screen.getByLabelText("Block anyone not part of server.org from ever joining this room.")); + + fireEvent.click(screen.getByText("Create room")); + await flushPromises(); + + expect(onFinished).toHaveBeenCalledWith( + true, + expect.objectContaining({ + name: "NoFederate Room", + createOpts: expect.objectContaining({ + creation_content: expect.objectContaining({ + "m.federate": false, + }), + }), + }), + ); + }); + describe("for a private room", () => { // default behaviour is a private room @@ -198,9 +247,8 @@ describe("", () => { await flushPromises(); expect(onFinished).toHaveBeenCalledWith(true, { - createOpts: { - name: roomName, - }, + createOpts: {}, + name: roomName, encryption: true, parentSpace: undefined, roomType: undefined, @@ -259,9 +307,9 @@ describe("", () => { await flushPromises(); expect(onFinished).toHaveBeenCalledWith(true, { createOpts: { - name: roomName, visibility: Visibility.Private, }, + name: roomName, encryption: true, joinRule: JoinRule.Knock, parentSpace: undefined, @@ -277,9 +325,9 @@ describe("", () => { await flushPromises(); expect(onFinished).toHaveBeenCalledWith(true, { createOpts: { - name: roomName, visibility: Visibility.Public, }, + name: roomName, encryption: true, joinRule: JoinRule.Knock, parentSpace: undefined, @@ -349,15 +397,111 @@ describe("", () => { expect(onFinished).toHaveBeenCalledWith(true, { createOpts: { - name: roomName, preset: Preset.PublicChat, room_alias_name: roomAlias, visibility: Visibility.Public, }, + name: roomName, guestAccess: false, parentSpace: undefined, roomType: undefined, }); }); }); + + describe("for a room in a space", () => { + let parentSpace: Room; + beforeEach(() => { + parentSpace = mkSpace(mockClient, "!space:server") as unknown as Room; + }); + + it("should create a room with restricted join rule when selected", async () => { + const onFinished = jest.fn(); + render(); + await flushPromises(); + + // Set room name and visibility. + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Restricted Room" } }); + fireEvent.click(screen.getByLabelText("Room visibility")); + fireEvent.click(screen.getByRole("option", { name: "Visible to space members" })); + + fireEvent.click(screen.getByText("Create room")); + await flushPromises(); + + expect(onFinished).toHaveBeenCalledWith( + true, + expect.objectContaining({ + name: "Restricted Room", + joinRule: JoinRule.Restricted, + }), + ); + }); + + it("should create a room with public join rule when selected", async () => { + const onFinished = jest.fn(); + render(); + await flushPromises(); + + // Set room name and visibility. Rooms in spaces also need an address. + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Public Room" } }); + fireEvent.click(screen.getByLabelText("Room visibility")); + fireEvent.click(screen.getByRole("option", { name: "Public room" })); + fireEvent.change(screen.getByLabelText("Room address"), { target: { value: "testroom" } }); + + // Create the room. + fireEvent.click(screen.getByText("Create room")); + await flushPromises(); + + expect(onFinished).toHaveBeenCalledWith( + true, + expect.objectContaining({ + name: "Public Room", + createOpts: expect.objectContaining({ + room_alias_name: "testroom", + visibility: Visibility.Public, + preset: Preset.PublicChat, + }), + guestAccess: false, + roomType: undefined, + }), + ); + }); + }); + + describe("keyboard shortcuts", () => { + it("should submit the form when Enter is pressed", async () => { + const onFinished = jest.fn(); + render(); + await flushPromises(); + + // Simulate pressing the Enter key. + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Keyboard Room" } }); + fireEvent.keyDown(screen.getByLabelText("Name"), { key: "Enter", code: "Enter", charCode: 13 }); + + await flushPromises(); + + expect(onFinished).toHaveBeenCalledWith( + true, + expect.objectContaining({ + name: "Keyboard Room", + }), + ); + }); + + it("should cancel the dialog when Escape is pressed", async () => { + const onFinished = jest.fn(); + render(); + await flushPromises(); + + // Simulate pressing the Escape key. + fireEvent.keyDown(screen.getByLabelText("Name"), { key: "Escape", code: "Escape", charCode: 27 }); + + await flushPromises(); + + // BaseDialog passes no arguments, but DialogButtons pass false - might not be desirable? + expect(onFinished).toHaveBeenCalled(); + const callArgs = onFinished.mock.calls[0]; + expect(callArgs.length === 0 || callArgs[0] === false).toBe(true); + }); + }); }); diff --git a/test/unit-tests/components/views/dialogs/__snapshots__/CreateRoomDialog-test.tsx.snap b/test/unit-tests/components/views/dialogs/__snapshots__/CreateRoomDialog-test.tsx.snap index 1873ac6e8f..486ecce500 100644 --- a/test/unit-tests/components/views/dialogs/__snapshots__/CreateRoomDialog-test.tsx.snap +++ b/test/unit-tests/components/views/dialogs/__snapshots__/CreateRoomDialog-test.tsx.snap @@ -32,14 +32,14 @@ exports[` for a private room should create a private room 1` class="mx_Field mx_Field_input mx_CreateRoomDialog_name" > @@ -48,14 +48,14 @@ exports[` for a private room should create a private room 1` class="mx_Field mx_Field_input mx_CreateRoomDialog_topic" > @@ -98,7 +98,7 @@ exports[` for a private room should create a private room 1` class="mx_SettingsFlag_label" >
Enable end-to-end encryption
@@ -106,7 +106,7 @@ exports[` for a private room should create a private room 1`
for a private room should create a private room 1` class="mx_SettingsFlag_label" >
Block anyone not part of server.org from ever joining this room.
@@ -142,7 +142,7 @@ exports[` for a private room should create a private room 1`
for a private room should render not the advanced class="mx_Field mx_Field_input mx_CreateRoomDialog_name" > @@ -242,14 +242,14 @@ exports[` for a private room should render not the advanced class="mx_Field mx_Field_input mx_CreateRoomDialog_topic" > @@ -292,7 +292,7 @@ exports[` for a private room should render not the advanced class="mx_SettingsFlag_label" >
Enable end-to-end encryption
@@ -300,7 +300,7 @@ exports[` for a private room should render not the advanced
{ }); }); + it("creates a public room with a topic", async () => { + await createRoom(client, { createOpts: { preset: Preset.PublicChat }, topic: "My topic" }); + + expect(client.createRoom).toHaveBeenCalledWith({ + preset: "public_chat", + visibility: "private", + topic: "My topic", + initial_state: [{ state_key: "", type: "m.room.guest_access", content: { guest_access: "can_join" } }], + }); + }); + it("creates a public room in a space", async () => { const roomId = await createRoom(client, { roomType: RoomType.Space }); const parentSpace = client.getRoom(roomId!)!;