Use the JoinRuleSettings component for the guest link access prompt. (#28614)

* Use the JoinRuleSettings component for the guest link access prompt.

Co-authored-by: fkwp <fkwp@users.noreply.github.com>

* increase timeout

* fix tests

---------

Co-authored-by: fkwp <fkwp@users.noreply.github.com>

res/css/views/settings/_JoinRuleSettings.pcss

@@ -53,6 +53,14 @@ Please see LICENSE files in the repository root for full details.
         display: block;
     }
 
+    &.mx_StyledRadioButton_disabled {
+        opacity: 0.5;
+    }
+
+    &.mx_StyledRadioButton_disabled + span {
+        opacity: 0.5;
+    }
+
     & + span {
         display: inline-block;
         margin-left: 34px;
@@ -71,3 +79,7 @@ Please see LICENSE files in the repository root for full details.
     font: var(--cpd-font-body-md-regular);
     margin-top: var(--cpd-space-2x);
 }
+
+.mx_JoinRuleSettings_recommended {
+    color: $accent-1000;
+}

src/components/views/rooms/RoomHeader/CallGuestLinkButton.tsx

@@ -14,10 +14,11 @@ import { EventType, JoinRule, type Room } from "matrix-js-sdk/src/matrix";
 import Modal from "../../../../Modal";
 import { ShareDialog } from "../../dialogs/ShareDialog";
 import { _t } from "../../../../languageHandler";
-import SettingsStore from "../../../../settings/SettingsStore";
 import { calculateRoomVia } from "../../../../utils/permalinks/Permalinks";
 import BaseDialog from "../../dialogs/BaseDialog";
 import { useGuestAccessInformation } from "../../../../hooks/room/useGuestAccessInformation";
+import JoinRuleSettings from "../../settings/JoinRuleSettings";
+import SettingsStore from "../../../../settings/SettingsStore";
 
 /**
  * Display a button to open a dialog to share a link to the call using a element call guest spa url (`element_call:guest_spa_url` in the EW config).
@@ -114,33 +115,32 @@ export const JoinRuleDialog: React.FC<{
                 "",
             );
             // Show the dialog for a bit to give the user feedback
-            setTimeout(() => onFinished(), 500);
+            setTimeout(() => onFinished(), 1000);
         },
         [isUpdating, onFinished, room.client, room.roomId],
     );
     return (
         <BaseDialog title={_t("update_room_access_modal|title")} onFinished={onFinished} className="mx_JoinRuleDialog">
-            <p>{_t("update_room_access_modal|description")}</p>
+            <p>{_t("update_room_access_modal|description", {}, { b: (sub) => <strong>{sub}</strong> })}</p>
-            <div className="mx_JoinRuleDialogButtons">
+            <p>
-                {askToJoinEnabled && canInvite && (
+                {_t("update_room_access_modal|revert_access_description", {}, { b: (sub) => <strong>{sub}</strong> })}
-                    <Button
+            </p>
-                        kind="secondary"
+            <JoinRuleSettings
-                        className="mx_Dialog_nonDialogButton"
+                recommendedOption={JoinRule.Knock}
-                        disabled={isUpdating === JoinRule.Knock}
+                room={room}
-                        onClick={() => changeJoinRule(JoinRule.Knock)}
+                disabledOptions={new Set([JoinRule.Invite, JoinRule.Private, JoinRule.Restricted])}
-                    >
+                hiddenOptions={
-                        {_t("action|ask_to_join")}
+                    new Set([JoinRule.Restricted].concat(askToJoinEnabled && canInvite ? [] : [JoinRule.Knock]))
-                    </Button>
+                }
-                )}
+                beforeChange={async (newRule) => {
-                <Button
+                    await changeJoinRule(newRule).catch(() => {
-                    className="mx_Dialog_nonDialogButton"
+                        return false;
-                    kind="destructive"
+                    });
-                    disabled={isUpdating === JoinRule.Public}
+                    return true;
-                    onClick={() => changeJoinRule(JoinRule.Public)}
+                }}
-                >
+                closeSettingsFn={() => {}}
-                    {_t("common|public")}
+                onError={(error: unknown) => logger.error("Could not generate change access level:", error)}
-                </Button>
+            />
-            </div>
             <p>{_t("update_room_access_modal|dont_change_description")}</p>
             <div className="mx_JoinRuleDialogButtons">
                 <Button

src/components/views/settings/JoinRuleSettings.tsx

@@ -36,6 +36,9 @@ export interface JoinRuleSettingsProps {
     onError(error: unknown): void;
     beforeChange?(joinRule: JoinRule): Promise<boolean>; // if returns false then aborts the change
     aliasWarning?: ReactNode;
+    disabledOptions?: Set<JoinRule>;
+    hiddenOptions?: Set<JoinRule>;
+    recommendedOption?: JoinRule;
 }
 
 const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
@@ -45,6 +48,9 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
     onError,
     beforeChange,
     closeSettingsFn,
+    disabledOptions,
+    hiddenOptions,
+    recommendedOption,
 }) => {
     const cli = room.client;
 
@@ -147,7 +153,7 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
                     }
                 });
 
-                closeSettingsFn();
+                closeSettingsFn?.();
 
                 // switch to the new room in the background
                 dis.dispatch<ViewRoomPayload>({
@@ -170,18 +176,26 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
             {_t("room_settings|security|join_rule_upgrade_required")}
         </span>
     );
+    const withRecommendLabel = (label: string, rule: JoinRule): React.ReactNode =>
+        rule === recommendedOption ? (
+            <>
+                {label} (<span className="mx_JoinRuleSettings_recommended">{_t("common|recommended")}</span>)
+            </>
+        ) : (
+            label
+        );
 
     const definitions: IDefinition<JoinRule>[] = [
         {
             value: JoinRule.Invite,
-            label: _t("room_settings|security|join_rule_invite"),
+            label: withRecommendLabel(_t("room_settings|security|join_rule_invite"), JoinRule.Invite),
             description: _t("room_settings|security|join_rule_invite_description"),
             checked:
                 joinRule === JoinRule.Invite || (joinRule === JoinRule.Restricted && !restrictedAllowRoomIds?.length),
         },
         {
             value: JoinRule.Public,
-            label: _t("common|public"),
+            label: withRecommendLabel(_t("common|public"), JoinRule.Public),
             description: (
                 <>
                     {_t("room_settings|security|join_rule_public_description")}
@@ -292,7 +306,7 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
             value: JoinRule.Restricted,
             label: (
                 <>
-                    {_t("room_settings|security|join_rule_restricted")}
+                    {withRecommendLabel(_t("room_settings|security|join_rule_restricted"), JoinRule.Restricted)}
                     {preferredRestrictionVersion && upgradeRequiredPill}
                 </>
             ),
@@ -303,11 +317,11 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
     }
 
     if (askToJoinEnabled && (roomSupportsKnock || preferredKnockVersion)) {
-        definitions.push({
+        definitions.splice(Math.max(0, definitions.length - 1), 0, {
             value: JoinRule.Knock,
             label: (
                 <>
-                    {_t("room_settings|security|join_rule_knock")}
+                    {withRecommendLabel(_t("room_settings|security|join_rule_knock"), JoinRule.Knock)}
                     {preferredKnockVersion && upgradeRequiredPill}
                 </>
             ),
@@ -397,7 +411,9 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
             name="joinRule"
             value={joinRule}
             onChange={onChange}
-            definitions={definitions}
+            definitions={definitions
+                .map((d) => (disabledOptions?.has(d.value) ? { ...d, disabled: true } : d))
+                .filter((d) => !hiddenOptions?.has(d.value))}
             disabled={disabled}
             className="mx_JoinRuleSettings_radioButton"
         />

src/i18n/strings/en_EN.json

@@ -3805,10 +3805,11 @@
         "unavailable": "Unavailable"
     },
     "update_room_access_modal": {
-        "description": "To create a share link, you need to allow guests to join this room. This may make the room less secure. When you're done with the call, you can make the room private again.",
+        "description": "To create a share link, make this room <b>public</b> or enable the option for users to <b>ask to join</b>. This allows guests to join without being invited.",
-        "dont_change_description": "Alternatively, you can hold the call in a separate room.",
+        "dont_change_description": "If you don't want to change the access of this room, you can create a new room for the call link.",
         "no_change": "I don't want to change the access level.",
-        "title": "Change the room access level"
+        "revert_access_description": "(This can be reverted to the previous value in the Room Settings: <b>Security & Privacy</b> / <b>Access</b>)",
+        "title": "Allow guest users to join this room"
     },
     "upload_failed_generic": "The file '%(fileName)s' failed to upload.",
     "upload_failed_size": "The file '%(fileName)s' exceeds this homeserver's size limit for uploads",

test/unit-tests/components/views/rooms/RoomHeader/CallGuestLinkButton-test.tsx

@@ -46,9 +46,11 @@ describe("<CallGuestLinkButton />", () => {
      */
     const makeRoom = (isVideoRoom = true): Room => {
         const room = new Room(roomId, sdkContext.client!, sdkContext.client!.getSafeUserId());
+        sdkContext.client!.getRoomDirectoryVisibility = jest.fn().mockResolvedValue("public");
         jest.spyOn(room, "isElementVideoRoom").mockReturnValue(isVideoRoom);
         // stub
         jest.spyOn(room, "getPendingEvents").mockReturnValue([]);
+        jest.spyOn(room, "getVersion").mockReturnValue("9");
         return room;
     };
     function mockRoomMembers(room: Room, count: number) {
@@ -221,24 +223,25 @@ describe("<CallGuestLinkButton />", () => {
         });
 
         it("shows ask to join if feature is enabled", () => {
-            const { container } = getComponent(room);
+            getComponent(room);
-            expect(getByText(container, "Ask to join")).toBeInTheDocument();
+            expect(screen.getByRole("radio", { name: "Ask to join ( Recommended )" })).toBeInTheDocument();
         });
-        it("font show ask to join if feature is enabled but cannot invite", () => {
+        it("dont show ask to join if feature is enabled but cannot invite", () => {
             getComponent(room, false);
-            expect(screen.queryByText("Ask to join")).not.toBeInTheDocument();
+            expect(screen.queryByRole("radio", { name: "Ask to join ( Recommended )" })).not.toBeInTheDocument();
         });
         it("doesn't show ask to join if feature is disabled", () => {
             jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
             getComponent(room);
-            expect(screen.queryByText("Ask to join")).not.toBeInTheDocument();
+            expect(screen.queryByRole("radio", { name: "Ask to join ( Recommended )" })).not.toBeInTheDocument();
         });
 
         it("sends correct state event on click", async () => {
             const sendStateSpy = jest.spyOn(sdkContext.client!, "sendStateEvent");
+
             let container;
             container = getComponent(room).container;
-            fireEvent.click(getByText(container, "Ask to join"));
+            fireEvent.click(screen.getByRole("radio", { name: "Ask to join ( Recommended )" }));
             expect(sendStateSpy).toHaveBeenCalledWith(
                 "!room:server.org",
                 "m.room.join_rules",
@@ -246,7 +249,7 @@ describe("<CallGuestLinkButton />", () => {
                 "",
             );
             expect(sendStateSpy).toHaveBeenCalledTimes(1);
-            await waitFor(() => expect(onFinished).toHaveBeenCalledTimes(1));
+            await waitFor(() => expect(onFinished).toHaveBeenCalledTimes(1), { timeout: 3000 });
             onFinished.mockClear();
             sendStateSpy.mockClear();
 
@@ -260,7 +263,7 @@ describe("<CallGuestLinkButton />", () => {
             );
             expect(sendStateSpy).toHaveBeenCalledTimes(1);
             container = getComponent(room).container;
-            await waitFor(() => expect(onFinished).toHaveBeenCalledTimes(1));
+            await waitFor(() => expect(onFinished).toHaveBeenCalledTimes(1), { timeout: 3000 });
             onFinished.mockClear();
             sendStateSpy.mockClear();