From e06b9c4a68b548c2f044698221c03d93aa23b0a4 Mon Sep 17 00:00:00 2001
From: Charly Nguyen <charly.nguyen@nordeck.net>
Date: Tue, 29 Aug 2023 16:18:08 +0200
Subject: [PATCH 1/3] Allow creating public knock rooms

Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net>
---
 res/css/views/dialogs/_CreateRoomDialog.pcss  |  5 ++
 .../views/dialogs/CreateRoomDialog.tsx        | 19 ++++
 .../views/elements/LabelledCheckbox.tsx       |  7 +-
 src/i18n/strings/en_EN.json                   |  1 +
 .../views/dialogs/CreateRoomDialog-test.tsx   | 87 ++++++++++++-------
 .../views/elements/LabelledCheckbox-test.tsx  | 12 +++
 6 files changed, 99 insertions(+), 32 deletions(-)

diff --git a/res/css/views/dialogs/_CreateRoomDialog.pcss b/res/css/views/dialogs/_CreateRoomDialog.pcss
index 437044cc8fe..4e3d90cffe6 100644
--- 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
index 276f1195b3d..6546d06a43e 100644
--- 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;
@@ -129,6 +130,7 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
 
         if (this.state.joinRule === JoinRule.Knock) {
             opts.joinRule = JoinRule.Knock;
+            createOpts.visibility = this.state.isPublic ? Visibility.Public : Visibility.Private;
         }
 
         return opts;
@@ -215,6 +217,10 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
         return result;
     };
 
+    private onIsPublicChange = (isPublic: boolean): void => {
+        this.setState({ isPublic });
+    };
+
     private static validateRoomName = withValidation({
         rules: [
             {
@@ -298,6 +304,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.onIsPublicChange}
+                    value={this.state.isPublic}
+                />
+            );
+        }
+
         let e2eeSection: JSX.Element | undefined;
         if (this.state.joinRule !== JoinRule.Public) {
             let microcopy: string;
@@ -383,6 +401,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
index 86604d0989a..72eded8203a 100644
--- 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 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
index 963e5093bde..17a6742e1a3 100644
--- a/src/i18n/strings/en_EN.json
+++ b/src/i18n/strings/en_EN.json
@@ -2688,6 +2688,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
index 9a43cc63dea..d51b9b298da 100644
--- a/test/components/views/dialogs/CreateRoomDialog-test.tsx
+++ b/test/components/views/dialogs/CreateRoomDialog-test.tsx
@@ -210,45 +210,72 @@ 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 disabling feature", () => {
+            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 enabling feature", () => {
             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 () => {
+                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 private knock room", 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 public knock room", 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
index c619794d90e..3e5917cb522 100644
--- 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);
+    });
 });

From 0f0cf65f4163b56fd0706e28baa3dd9daafb2a77 Mon Sep 17 00:00:00 2001
From: Charly Nguyen <charly.nguyen@nordeck.net>
Date: Fri, 1 Sep 2023 09:27:59 +0200
Subject: [PATCH 2/3] Apply PR feedback

Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net>
---
 .../views/dialogs/CreateRoomDialog.tsx        | 24 +++++++++++++------
 .../views/elements/LabelledCheckbox.tsx       |  2 +-
 .../views/dialogs/CreateRoomDialog-test.tsx   |  9 +++----
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/components/views/dialogs/CreateRoomDialog.tsx b/src/components/views/dialogs/CreateRoomDialog.tsx
index 6546d06a43e..973827e482d 100644
--- a/src/components/views/dialogs/CreateRoomDialog.tsx
+++ b/src/components/views/dialogs/CreateRoomDialog.tsx
@@ -46,15 +46,25 @@ interface IProps {
 }
 
 interface IState {
+    // The selected room join rule.
     joinRule: JoinRule;
-    isPublic: boolean;
+    // Indicates whether the knock room is public visible. Applies only for rooms with knock join rule.
+    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;
 }
 
@@ -79,7 +89,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 || "",
@@ -130,7 +140,7 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
 
         if (this.state.joinRule === JoinRule.Knock) {
             opts.joinRule = JoinRule.Knock;
-            createOpts.visibility = this.state.isPublic ? Visibility.Public : Visibility.Private;
+            createOpts.visibility = this.state.isPublicKnockRoom ? Visibility.Public : Visibility.Private;
         }
 
         return opts;
@@ -217,8 +227,8 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
         return result;
     };
 
-    private onIsPublicChange = (isPublic: boolean): void => {
-        this.setState({ isPublic });
+    private onIsPublicKnockRoomChange = (isPublicKnockRoom: boolean): void => {
+        this.setState({ isPublicKnockRoom });
     };
 
     private static validateRoomName = withValidation({
@@ -310,8 +320,8 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
                 <LabelledCheckbox
                     className="mx_CreateRoomDialog_labelledCheckbox"
                     label={_t("Make this room visible in the public room directory.")}
-                    onChange={this.onIsPublicChange}
-                    value={this.state.isPublic}
+                    onChange={this.onIsPublicKnockRoomChange}
+                    value={this.state.isPublicKnockRoom}
                 />
             );
         }
diff --git a/src/components/views/elements/LabelledCheckbox.tsx b/src/components/views/elements/LabelledCheckbox.tsx
index 72eded8203a..e6f7499ecf0 100644
--- a/src/components/views/elements/LabelledCheckbox.tsx
+++ b/src/components/views/elements/LabelledCheckbox.tsx
@@ -30,7 +30,7 @@ interface IProps {
     disabled?: boolean;
     // The function to call when the value changes
     onChange(checked: boolean): void;
-    // Optional CSS class to apply to the label
+    // Optional additional CSS class to apply to the label
     className?: string;
 }
 
diff --git a/test/components/views/dialogs/CreateRoomDialog-test.tsx b/test/components/views/dialogs/CreateRoomDialog-test.tsx
index d51b9b298da..68bcef48c8f 100644
--- a/test/components/views/dialogs/CreateRoomDialog-test.tsx
+++ b/test/components/views/dialogs/CreateRoomDialog-test.tsx
@@ -210,7 +210,7 @@ describe("<CreateRoomDialog />", () => {
     });
 
     describe("for a knock room", () => {
-        describe("when disabling feature", () => {
+        describe("when feature is disabled", () => {
             it("should not have the option to create a knock room", async () => {
                 jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
                 getComponent();
@@ -219,11 +219,12 @@ describe("<CreateRoomDialog />", () => {
             });
         });
 
-        describe("when enabling feature", () => {
+        describe("when feature is enabled", () => {
             const onFinished = jest.fn();
             const roomName = "Test Room Name";
 
             beforeEach(async () => {
+                onFinished.mockReset();
                 jest.spyOn(SettingsStore, "getValue").mockImplementation(
                     (setting) => setting === "feature_ask_to_join",
                 );
@@ -245,7 +246,7 @@ describe("<CreateRoomDialog />", () => {
                 ).toBeInTheDocument();
             });
 
-            it("should create a private knock room", async () => {
+            it("should create a knock room with private visibility", async () => {
                 fireEvent.click(screen.getByText("Create room"));
                 await flushPromises();
                 expect(onFinished).toHaveBeenCalledWith(true, {
@@ -260,7 +261,7 @@ describe("<CreateRoomDialog />", () => {
                 });
             });
 
-            it("should create a public knock room", async () => {
+            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." }),
                 );

From 20745cecc0c9134fd59d4e2196286a951fb65f92 Mon Sep 17 00:00:00 2001
From: Charly Nguyen <charly.nguyen@nordeck.net>
Date: Mon, 4 Sep 2023 17:17:46 +0200
Subject: [PATCH 3/3] Apply PR feedback

Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net>
---
 .../views/dialogs/CreateRoomDialog.tsx        | 41 ++++++++++++++-----
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/components/views/dialogs/CreateRoomDialog.tsx b/src/components/views/dialogs/CreateRoomDialog.tsx
index 973827e482d..459d54d0370 100644
--- a/src/components/views/dialogs/CreateRoomDialog.tsx
+++ b/src/components/views/dialogs/CreateRoomDialog.tsx
@@ -46,25 +46,46 @@ interface IProps {
 }
 
 interface IState {
-    // The selected room join rule.
+    /**
+     * The selected room join rule.
+     */
     joinRule: JoinRule;
-    // Indicates whether the knock room is public visible. Applies only for rooms with knock join rule.
+    /**
+     * 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.
+    /**
+     * Indicates whether end-to-end encryption is enabled for the room.
+     */
     isEncrypted: boolean;
-    // The room name.
+    /**
+     * The room name.
+     */
     name: string;
-    // The room topic.
+    /**
+     * The room topic.
+     */
     topic: string;
-    // The room alias.
+    /**
+     * The room alias.
+     */
     alias: string;
-    // Indicates whether the details section is open.
+    /**
+     * Indicates whether the details section is open.
+     */
     detailsOpen: boolean;
-    // Indicates whether federation is disabled for the room.
+    /**
+     * Indicates whether federation is disabled for the room.
+     */
     noFederate: boolean;
-    // Indicates whether the room name is valid.
+    /**
+     * Indicates whether the room name is valid.
+     */
     nameIsValid: boolean;
-    // Indicates whether the user can change encryption settings for the room.
+    /**
+     * Indicates whether the user can change encryption settings for the room.
+     */
     canChangeEncryption: boolean;
 }