Allow creating public knock rooms (matrix-org#11481)

* Allow creating public knock rooms Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net> * Apply PR feedback Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net> * Apply PR feedback Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net> --------- Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net>
nordeck · Sep 6, 2023 · 4ded69e · 4ded69e
1 parent 555faa2
commit 4ded69e
Show file tree

Hide file tree

Showing 6 changed files with 133 additions and 34 deletions.
diff --git a/res/css/views/dialogs/_CreateRoomDialog.pcss b/res/css/views/dialogs/_CreateRoomDialog.pcss
@@ -113,3 +113,8 @@ limitations under the License.
         font-size: $font-12px;
     }
 }
+
+.mx_CreateRoomDialog_labelledCheckbox {
+    color: $muted-fg-color;
+    margin-top: var(--cpd-space-6x);
+}
diff --git a/src/components/views/dialogs/CreateRoomDialog.tsx b/src/components/views/dialogs/CreateRoomDialog.tsx
@@ -33,6 +33,7 @@ import { getKeyBindingsManager } from "../../../KeyBindingsManager";
 import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts";
 import { privateShouldBeEncrypted } from "../../../utils/rooms";
 import SettingsStore from "../../../settings/SettingsStore";
+import LabelledCheckbox from "../elements/LabelledCheckbox";
 
 interface IProps {
     type?: RoomType;
@@ -45,15 +46,46 @@ interface IProps {
 }
 
 interface IState {
+    /**
+     * The selected room join rule.
+     */
     joinRule: JoinRule;
-    isPublic: boolean;
+    /**
+     * Indicates whether the created room should have public visibility (ie, it should be
+     * shown in the public room list). Only applicable if `joinRule` == `JoinRule.Knock`.
+     */
+    isPublicKnockRoom: boolean;
+    /**
+     * Indicates whether end-to-end encryption is enabled for the room.
+     */
     isEncrypted: boolean;
+    /**
+     * The room name.
+     */
     name: string;
+    /**
+     * The room topic.
+     */
     topic: string;
+    /**
+     * The room alias.
+     */
     alias: string;
+    /**
+     * Indicates whether the details section is open.
+     */
     detailsOpen: boolean;
+    /**
+     * Indicates whether federation is disabled for the room.
+     */
     noFederate: boolean;
+    /**
+     * Indicates whether the room name is valid.
+     */
     nameIsValid: boolean;
+    /**
+     * Indicates whether the user can change encryption settings for the room.
+     */
     canChangeEncryption: boolean;
 }
 
@@ -78,7 +110,7 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
 
         const cli = MatrixClientPeg.safeGet();
         this.state = {
-            isPublic: this.props.defaultPublic || false,
+            isPublicKnockRoom: this.props.defaultPublic || false,
             isEncrypted: this.props.defaultEncrypted ?? privateShouldBeEncrypted(cli),
             joinRule,
             name: this.props.defaultName || "",
@@ -129,6 +161,7 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
 
         if (this.state.joinRule === JoinRule.Knock) {
             opts.joinRule = JoinRule.Knock;
+            createOpts.visibility = this.state.isPublicKnockRoom ? Visibility.Public : Visibility.Private;
         }
 
         return opts;
@@ -215,6 +248,10 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
         return result;
     };
 
+    private onIsPublicKnockRoomChange = (isPublicKnockRoom: boolean): void => {
+        this.setState({ isPublicKnockRoom });
+    };
+
     private static validateRoomName = withValidation({
         rules: [
             {
@@ -298,6 +335,18 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
             );
         }
 
+        let visibilitySection: JSX.Element | undefined;
+        if (this.state.joinRule === JoinRule.Knock) {
+            visibilitySection = (
+                <LabelledCheckbox
+                    className="mx_CreateRoomDialog_labelledCheckbox"
+                    label={_t("Make this room visible in the public room directory.")}
+                    onChange={this.onIsPublicKnockRoomChange}
+                    value={this.state.isPublicKnockRoom}
+                />
+            );
+        }
+
         let e2eeSection: JSX.Element | undefined;
         if (this.state.joinRule !== JoinRule.Public) {
             let microcopy: string;
@@ -383,6 +432,7 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
                         />
 
                         {publicPrivateLabel}
+                        {visibilitySection}
                         {e2eeSection}
                         {aliasField}
                         <details onToggle={this.onDetailsToggled} className="mx_CreateRoomDialog_details">

diff --git a/src/components/views/elements/LabelledCheckbox.tsx b/src/components/views/elements/LabelledCheckbox.tsx
@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import React from "react";
+import classnames from "classnames";
 
 import StyledCheckbox from "./StyledCheckbox";
 
@@ -29,11 +30,13 @@ interface IProps {
     disabled?: boolean;
     // The function to call when the value changes
     onChange(checked: boolean): void;
+    // Optional additional CSS class to apply to the label
+    className?: string;
 }
 
-const LabelledCheckbox: React.FC<IProps> = ({ value, label, byline, disabled, onChange }) => {
+const LabelledCheckbox: React.FC<IProps> = ({ value, label, byline, disabled, onChange, className }) => {
     return (
-        <label className="mx_LabelledCheckbox">
+        <label className={classnames("mx_LabelledCheckbox", className)}>
             <StyledCheckbox disabled={disabled} checked={value} onChange={(e) => onChange(e.target.checked)} />
             <div className="mx_LabelledCheckbox_labels">
                 <span className="mx_LabelledCheckbox_label">{label}</span>

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -2693,6 +2693,7 @@
     "Anyone will be able to find and join this room.": "Anyone will be able to find and join this room.",
     "Only people invited will be able to find and join this room.": "Only people invited will be able to find and join this room.",
     "Anyone can request to join, but admins or moderators need to grant access. You can change this later.": "Anyone can request to join, but admins or moderators need to grant access. You can change this later.",
+    "Make this room visible in the public room directory.": "Make this room visible in the public room directory.",
     "You can't disable this later. The room will be encrypted but the embedded call will not.": "You can't disable this later. The room will be encrypted but the embedded call will not.",
     "You can't disable this later. Bridges & most bots won't work yet.": "You can't disable this later. Bridges & most bots won't work yet.",
     "Your server requires encryption to be enabled in private rooms.": "Your server requires encryption to be enabled in private rooms.",

diff --git a/test/components/views/dialogs/CreateRoomDialog-test.tsx b/test/components/views/dialogs/CreateRoomDialog-test.tsx
@@ -210,45 +210,73 @@ describe("<CreateRoomDialog />", () => {
     });
 
     describe("for a knock room", () => {
-        it("should not have the option to create a knock room", async () => {
-            jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
-            getComponent();
-            fireEvent.click(screen.getByLabelText("Room visibility"));
-
-            expect(screen.queryByRole("option", { name: "Ask to join" })).not.toBeInTheDocument();
+        describe("when feature is disabled", () => {
+            it("should not have the option to create a knock room", async () => {
+                jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
+                getComponent();
+                fireEvent.click(screen.getByLabelText("Room visibility"));
+                expect(screen.queryByRole("option", { name: "Ask to join" })).not.toBeInTheDocument();
+            });
         });
 
-        it("should create a knock room", async () => {
-            jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => setting === "feature_ask_to_join");
+        describe("when feature is enabled", () => {
             const onFinished = jest.fn();
-            getComponent({ onFinished });
-            await flushPromises();
-
             const roomName = "Test Room Name";
-            fireEvent.change(screen.getByLabelText("Name"), { target: { value: roomName } });
 
-            fireEvent.click(screen.getByLabelText("Room visibility"));
-            fireEvent.click(screen.getByRole("option", { name: "Ask to join" }));
+            beforeEach(async () => {
+                onFinished.mockReset();
+                jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                    (setting) => setting === "feature_ask_to_join",
+                );
+                getComponent({ onFinished });
+                fireEvent.change(screen.getByLabelText("Name"), { target: { value: roomName } });
+                fireEvent.click(screen.getByLabelText("Room visibility"));
+                fireEvent.click(screen.getByRole("option", { name: "Ask to join" }));
+            });
 
-            fireEvent.click(screen.getByText("Create room"));
-            await flushPromises();
+            it("should have a heading", () => {
+                expect(screen.getByRole("heading")).toHaveTextContent("Create a room");
+            });
 
-            expect(screen.getByText("Create a room")).toBeInTheDocument();
+            it("should have a hint", () => {
+                expect(
+                    screen.getByText(
+                        "Anyone can request to join, but admins or moderators need to grant access. You can change this later.",
+                    ),
+                ).toBeInTheDocument();
+            });
 
-            expect(
-                screen.getByText(
-                    "Anyone can request to join, but admins or moderators need to grant access. You can change this later.",
-                ),
-            ).toBeInTheDocument();
+            it("should create a knock room with private visibility", async () => {
+                fireEvent.click(screen.getByText("Create room"));
+                await flushPromises();
+                expect(onFinished).toHaveBeenCalledWith(true, {
+                    createOpts: {
+                        name: roomName,
+                        visibility: Visibility.Private,
+                    },
+                    encryption: true,
+                    joinRule: JoinRule.Knock,
+                    parentSpace: undefined,
+                    roomType: undefined,
+                });
+            });
 
-            expect(onFinished).toHaveBeenCalledWith(true, {
-                createOpts: {
-                    name: roomName,
-                },
-                encryption: true,
-                joinRule: JoinRule.Knock,
-                parentSpace: undefined,
-                roomType: undefined,
+            it("should create a knock room with public visibility", async () => {
+                fireEvent.click(
+                    screen.getByRole("checkbox", { name: "Make this room visible in the public room directory." }),
+                );
+                fireEvent.click(screen.getByText("Create room"));
+                await flushPromises();
+                expect(onFinished).toHaveBeenCalledWith(true, {
+                    createOpts: {
+                        name: roomName,
+                        visibility: Visibility.Public,
+                    },
+                    encryption: true,
+                    joinRule: JoinRule.Knock,
+                    parentSpace: undefined,
+                    roomType: undefined,
+                });
             });
         });
     });

diff --git a/test/components/views/elements/LabelledCheckbox-test.tsx b/test/components/views/elements/LabelledCheckbox-test.tsx
@@ -89,4 +89,16 @@ describe("<LabelledCheckbox />", () => {
         expect(checkbox).toBeChecked();
         expect(checkbox).toBeDisabled();
     });
+
+    it("should render with a custom class name", () => {
+        const className = "some class name";
+        const props: CompProps = {
+            label: "Hello world",
+            value: false,
+            onChange: jest.fn(),
+            className,
+        };
+        const { container } = render(getComponent(props));
+        expect(container.firstElementChild?.className).toContain(className);
+    });
 });