Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Allow setting knock room visibility #11464

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions res/css/views/dialogs/_CreateRoomDialog.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
5 changes: 5 additions & 0 deletions res/css/views/settings/_JoinRuleSettings.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,8 @@ limitations under the License.
}
}
}

.mx_JoinRuleSettings_labelledCheckbox {
font: var(--cpd-font-body-md-regular);
margin-top: var(--cpd-space-2x);
}
19 changes: 19 additions & 0 deletions src/components/views/dialogs/CreateRoomDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -215,6 +217,10 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
return result;
};

private onIsPublicChange = (isPublic: boolean): void => {
this.setState({ isPublic });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for reference: I was confused about what this isPublic state was doing. The answer is: nothing. It appears to have been introduced by #5698, probably due to an incorrect merge resolution due to a conflict with #6212 (which removed it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was also my conclusion. So, I decided to give it a purpose.

};

private static validateRoomName = withValidation({
rules: [
{
Expand Down Expand Up @@ -298,6 +304,18 @@ export default class CreateRoomDialog extends React.Component<IProps, IState> {
);
}

let visibilitySection: JSX.Element | undefined;
if (this.state.joinRule === JoinRule.Knock) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we only expose this control for knockable rooms? Isn't it relevant to other rooms too?

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;
Expand Down Expand Up @@ -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">
Expand Down
36 changes: 32 additions & 4 deletions src/components/views/dialogs/spotlight/SpotlightDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
RoomType,
Room,
HierarchyRoom,
JoinRule,
} from "matrix-js-sdk/src/matrix";
import { normalize } from "matrix-js-sdk/src/utils";
import React, { ChangeEvent, RefObject, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react";
Expand Down Expand Up @@ -276,6 +277,10 @@ const roomAriaUnreadLabel = (room: Room, notification: RoomNotificationState): s
}
};

const canAskToJoin = (joinRule?: JoinRule): boolean => {
return SettingsStore.getValue("feature_ask_to_join") && JoinRule.Knock === joinRule;
};

interface IDirectoryOpts {
limit: number;
query: string;
Expand Down Expand Up @@ -514,7 +519,14 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
}, [results, filter]);

const viewRoom = (
room: { roomId: string; roomAlias?: string; autoJoin?: boolean; shouldPeek?: boolean; viaServers?: string[] },
room: {
roomId: string;
roomAlias?: string;
autoJoin?: boolean;
shouldPeek?: boolean;
viaServers?: string[];
joinRule?: IPublicRoomsChunkRoom["join_rule"];
},
persist = false,
viaKeyboard = false,
): void => {
Expand All @@ -538,10 +550,15 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
metricsViaKeyboard: viaKeyboard,
room_id: room.roomId,
room_alias: room.roomAlias,
auto_join: room.autoJoin,
auto_join: room.autoJoin && !canAskToJoin(room.joinRule),
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
should_peek: room.shouldPeek,
via_servers: room.viaServers,
});

if (canAskToJoin(room.joinRule)) {
defaultDispatcher.dispatch({ action: Action.PromptAskToJoin });
}

onFinished();
};

Expand Down Expand Up @@ -651,12 +668,15 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
}
if (isPublicRoomResult(result)) {
const clientRoom = cli.getRoom(result.publicRoom.room_id);
const joinRule = result.publicRoom.join_rule;
// Element Web currently does not allow guests to join rooms, so we
// instead show them view buttons for all rooms. If the room is not
// world readable, a modal will appear asking you to register first. If
// it is readable, the preview appears as normal.
const showViewButton =
clientRoom?.getMyMembership() === "join" || result.publicRoom.world_readable || cli.isGuest();
clientRoom?.getMyMembership() === "join" ||
(result.publicRoom.world_readable && !canAskToJoin(joinRule)) ||
cli.isGuest();

const listener = (ev: ButtonEvent): void => {
ev.stopPropagation();
Expand All @@ -669,12 +689,20 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
autoJoin: !result.publicRoom.world_readable && !cli.isGuest(),
shouldPeek: result.publicRoom.world_readable || cli.isGuest(),
viaServers: config ? [config.roomServer] : undefined,
joinRule,
},
true,
ev.type !== "click",
);
};

let buttonLabel;
if (showViewButton) {
buttonLabel = _t("action|view");
} else {
buttonLabel = canAskToJoin(joinRule) ? _t("action|ask_to_join") : _t("action|join");
}

return (
<Option
id={`mx_SpotlightDialog_button_result_${result.publicRoom.room_id}`}
Expand All @@ -687,7 +715,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
onClick={listener}
tabIndex={-1}
>
{showViewButton ? _t("action|view") : _t("action|join")}
{buttonLabel}
</AccessibleButton>
}
aria-labelledby={`mx_SpotlightDialog_button_result_${result.publicRoom.room_id}_name`}
Expand Down
7 changes: 5 additions & 2 deletions src/components/views/elements/LabelledCheckbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
*/

import React from "react";
import classnames from "classnames";

import StyledCheckbox from "./StyledCheckbox";

Expand All @@ -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>
Expand Down
46 changes: 43 additions & 3 deletions src/components/views/settings/JoinRuleSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,15 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { ReactNode } from "react";
import { IJoinRuleEventContent, JoinRule, RestrictedAllowType, Room, EventType } from "matrix-js-sdk/src/matrix";
import React, { ReactNode, useEffect, useState } from "react";
import {
IJoinRuleEventContent,
JoinRule,
RestrictedAllowType,
Room,
EventType,
Visibility,
} from "matrix-js-sdk/src/matrix";

import StyledRadioGroup, { IDefinition } from "../elements/StyledRadioGroup";
import { _t } from "../../../languageHandler";
Expand All @@ -34,6 +41,7 @@ import { Action } from "../../../dispatcher/actions";
import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
import { doesRoomVersionSupport, PreferredRoomVersions } from "../../../utils/PreferredRoomVersions";
import SettingsStore from "../../../settings/SettingsStore";
import LabelledCheckbox from "../elements/LabelledCheckbox";

export interface JoinRuleSettingsProps {
room: Room;
Expand Down Expand Up @@ -76,6 +84,23 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
? content?.allow?.filter((o) => o.type === RestrictedAllowType.RoomMembership).map((o) => o.room_id)
: undefined;

const [visibility, setVisibility] = useState(Visibility.Private);

useEffect(() => {
if (joinRule === JoinRule.Knock) {
cli.getRoomDirectoryVisibility(room.roomId)
.then(({ visibility }) => setVisibility(visibility))
.catch(onError);
}
}, [cli, joinRule, onError, room.roomId]);

const onVisibilityChange = (checked: boolean): void => {
const visibility = checked ? Visibility.Public : Visibility.Private;
cli.setRoomDirectoryVisibility(room.roomId, visibility)
.then(() => setVisibility(visibility))
.catch(onError);
};

const editRestrictedRoomIds = async (): Promise<string[] | undefined> => {
let selected = restrictedAllowRoomIds;
if (!selected?.length && SpaceStore.instance.activeSpaceRoom) {
Expand Down Expand Up @@ -297,7 +322,22 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
{preferredKnockVersion && upgradeRequiredPill}
</>
),
description: _t("People cannot join unless access is granted."),
description: (
<>
{_t("People cannot join unless access is granted.")}
<LabelledCheckbox
className="mx_JoinRuleSettings_labelledCheckbox"
disabled={!!preferredKnockVersion || joinRule !== JoinRule.Knock}
label={
room.isSpaceRoom()
? _t("Make this space visible in the public room directory.")
: _t("Make this room visible in the public room directory.")
}
onChange={onVisibilityChange}
value={visibility === Visibility.Public}
/>
</>
),
});
}

Expand Down
3 changes: 3 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"resend": "Resend",
"next": "Next",
"view": "View",
"ask_to_join": "Ask to join",
"forward": "Forward",
"copy_link": "Copy link",
"logout": "Logout",
Expand Down Expand Up @@ -1192,6 +1193,8 @@
"Space members": "Space members",
"Ask to join": "Ask to join",
"People cannot join unless access is granted.": "People cannot join unless access is granted.",
"Make this space visible in the public room directory.": "Make this space visible in the public room directory.",
"Make this room visible in the public room directory.": "Make this room visible in the public room directory.",
"This room is in some spaces you're not an admin of. In those spaces, the old room will still be shown, but people will be prompted to join the new one.": "This room is in some spaces you're not an admin of. In those spaces, the old room will still be shown, but people will be prompted to join the new one.",
"This upgrade will allow members of selected spaces access to this room without an invite.": "This upgrade will allow members of selected spaces access to this room without an invite.",
"IRC (Experimental)": "IRC (Experimental)",
Expand Down
87 changes: 57 additions & 30 deletions test/components/views/dialogs/CreateRoomDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
});
});
});
Expand Down
Loading