Apply PR feedback

Signed-off-by: Charly Nguyen <charly.nguyen@nordeck.net>

Author: Charly Nguyen

src/components/views/rooms/RoomKnocksBar.tsx

@@ -30,41 +30,48 @@ import AccessibleButton from "../elements/AccessibleButton";
 import Heading from "../typography/Heading";
 
 export const RoomKnocksBar: VFC<{ room: Room }> = ({ room }) => {
+ const [disabled, setDisabled] = useState(false);
+ const knockMembers = useTypedEventEmitterState(
+ room,
+ RoomStateEvent.Members,
+ useCallback(() => room.getMembersWithMembership("knock"), [room]),
+ );
+ const knockMembersCount = knockMembers.length;
+
+ if (room.getJoinRule() !== JoinRule.Knock || knockMembersCount === 0) return null;
+
  const client = room.client;
  const userId = client.getUserId() || "";
  const canInvite = room.canInvite(userId);
  const member = room.getMember(userId);
  const state = room.getLiveTimeline().getState(EventTimeline.FORWARDS);
  const canKick = member && state ? state.hasSufficientPowerLevelFor("kick", member.powerLevel) : false;
 
+ if (!canInvite && !canKick) return null;
+
+ const onError = (error: MatrixError): void => {
+ Modal.createDialog(ErrorDialog, { title: error.name, description: error.message });
+ };
+
  const handleApprove = (userId: string): void => {
  setDisabled(true);
- client.invite(room.roomId, userId).catch(onError);
+ client
+ .invite(room.roomId, userId)
+ .catch(onError)
+ .finally(() => setDisabled(false));
  };
 
  const handleDeny = (userId: string): void => {
  setDisabled(true);
- client.kick(room.roomId, userId).catch(onError);
+ client
+ .kick(room.roomId, userId)
+ .catch(onError)
+ .finally(() => setDisabled(false));
  };
 
  const handleOpenRoomSettings = (): void =>
  dis.dispatch({ action: "open_room_settings", room_id: room.roomId, initial_tab_id: RoomSettingsTab.People });
 
- const onError = (error: MatrixError): void => {
- setDisabled(false);
- Modal.createDialog(ErrorDialog, { title: error.name, description: error.message });
- };
-
- const [disabled, setDisabled] = useState(false);
- const knockMembers = useTypedEventEmitterState(
- room,
- RoomStateEvent.Members,
- useCallback(() => room.getMembersWithMembership("knock"), [room]),
- );
- const knockMembersCount = knockMembers.length;
-
- if (room.getJoinRule() !== JoinRule.Knock || knockMembersCount === 0 || (!canInvite && !canKick)) return null;
-
  let buttons: ReactElement = (
  <AccessibleButton
  className="mx_RoomKnocksBar_action"
@@ -89,7 +96,7 @@ export const RoomKnocksBar: VFC<{ room: Room }> = ({ room }) => {
  disabled={!canKick || disabled}
  kind="icon_primary_outline"
  onClick={() => handleDeny(knockMembers[0].userId)}
- title={_t("Deny")}
+ title={_t("action|deny")}
  >
  <XIcon width={18} height={18} />
  </AccessibleButton>
@@ -98,7 +105,7 @@ export const RoomKnocksBar: VFC<{ room: Room }> = ({ room }) => {
  disabled={!canInvite || disabled}
  kind="icon_primary"
  onClick={() => handleApprove(knockMembers[0].userId)}
- title={_t("Approve")}
+ title={_t("action|approve")}
  >
  <CheckIcon width={18} height={18} />
  </AccessibleButton>

src/components/views/settings/tabs/room/PeopleRoomSettingsTab.tsx

@@ -94,7 +94,7 @@ const Knock: VFC<{
  disabled={!canKick || disabled}
  kind="icon_primary_outline"
  onClick={() => handleDeny(roomMember.userId)}
- title={_t("Deny")}
+ title={_t("action|deny")}
  >
  <XIcon width={18} height={18} />
  </AccessibleButton>
@@ -103,7 +103,7 @@ const Knock: VFC<{
  disabled={!canInvite || disabled}
  kind="icon_primary"
  onClick={() => handleApprove(roomMember.userId)}
- title={_t("Approve")}
+ title={_t("action|approve")}
  >
  <CheckIcon width={18} height={18} />
  </AccessibleButton>

src/i18n/strings/en_EN.json

@@ -48,6 +48,8 @@
  "save": "Save",
  "add": "Add",
  "sign_out": "Sign out",
+ "deny": "Deny",
+ "approve": "Approve",
  "share": "Share",
  "invite": "Invite",
  "search": "Search",
@@ -1468,8 +1470,6 @@
  "Browse": "Browse",
  "See less": "See less",
  "See more": "See more",
- "Deny": "Deny",
- "Approve": "Approve",
  "Asking to join": "Asking to join",
  "No requests": "No requests",
  "Failed to unban": "Failed to unban",
@@ -2985,6 +2985,7 @@
  "Verification Request": "Verification Request",
  "Approve widget permissions": "Approve widget permissions",
  "This widget would like to:": "This widget would like to:",
+ "Approve": "Approve",
  "Decline All": "Decline All",
  "Remember my selection for this widget": "Remember my selection for this widget",
  "Allow this widget to verify your identity": "Allow this widget to verify your identity",

test/components/views/rooms/RoomKnocksBar-test.tsx

@@ -52,7 +52,8 @@ describe("RoomKnocksBar", () => {
  const room = new Room(roomId, client, userId);
  const state = room.getLiveTimeline().getState(EventTimeline.FORWARDS)!;
 
- const getButton = (name: "Approve" | "Deny" | "View" | "View message") => screen.getByRole("button", { name });
+ type ButtonNames = "Approve" | "Deny" | "View" | "View message";
+ const getButton = (name: ButtonNames) => screen.getByRole("button", { name });
  const getComponent = (room: Room) =>
  render(
  <MatrixClientContext.Provider value={client}>
@@ -134,16 +135,33 @@ describe("RoomKnocksBar", () => {
  expect(getComponent(room).container.firstChild).toBeNull();
  });
 
+ it("unhides the bar when a new knock request appears", () => {
+ jest.spyOn(room, "getMembersWithMembership").mockReturnValue([]);
+ const { container } = getComponent(room);
+ expect(container.firstChild).toBeNull();
+ jest.spyOn(room, "getMembersWithMembership").mockReturnValue([bob]);
+ act(() => {
+ room.emit(RoomStateEvent.Members, new MatrixEvent(), state, bob);
+ });
+ expect(container.firstChild).not.toBeNull();
+ });
+
+ it("updates when the list of knocking users changes", () => {
+ getComponent(room);
+ expect(screen.getByRole("heading")).toHaveTextContent("Asking to join");
+ jest.spyOn(room, "getMembersWithMembership").mockReturnValue([bob, jane]);
+ act(() => {
+ room.emit(RoomStateEvent.Members, new MatrixEvent(), state, jane);
+ });
+ expect(screen.getByRole("heading")).toHaveTextContent("2 people asking to join");
+ });
+
  describe("when knock members count is 1", () => {
  beforeEach(() => jest.spyOn(room, "getMembersWithMembership").mockReturnValue([bob]));
 
- it("renders a heading", () => {
+ it("renders a heading and a paragraph with name and user ID", () => {
  getComponent(room);
  expect(screen.getByRole("heading")).toHaveTextContent("Asking to join");
- });
-
- it("renders a paragraph", () => {
- getComponent(room);
  expect(screen.getByRole("paragraph")).toHaveTextContent(`${bob.name} (${bob.userId})`);
  });
 
@@ -157,20 +175,38 @@ describe("RoomKnocksBar", () => {
  });
  });
 
+ type TestCase = [string, ButtonNames, () => void];
+ it.each<TestCase>([
+ ["deny request fails", "Deny", () => jest.spyOn(client, "kick").mockRejectedValue(error)],
+ ["deny request succeeds", "Deny", () => jest.spyOn(client, "kick").mockResolvedValue({})],
+ ["approve request fails", "Approve", () => jest.spyOn(client, "invite").mockRejectedValue(error)],
+ ["approve request succeeds", "Approve", () => jest.spyOn(client, "invite").mockResolvedValue({})],
+ ])("toggles the disabled attribute for the buttons when a %s", async (_, buttonName, setup) => {
+ setup();
+ getComponent(room);
+ fireEvent.click(getButton(buttonName));
+ expect(getButton("Deny")).toHaveAttribute("disabled");
+ expect(getButton("Approve")).toHaveAttribute("disabled");
+ await act(() => flushPromises());
+ expect(getButton("Deny")).not.toHaveAttribute("disabled");
+ expect(getButton("Approve")).not.toHaveAttribute("disabled");
+ });
+
  it("disables the deny button if the power level is insufficient", () => {
  jest.spyOn(state, "hasSufficientPowerLevelFor").mockReturnValue(false);
  getComponent(room);
  expect(getButton("Deny")).toHaveAttribute("disabled");
  });
 
- it("calls kick on deny", () => {
+ it("calls kick on deny", async () => {
  jest.spyOn(client, "kick").mockResolvedValue({});
  getComponent(room);
  fireEvent.click(getButton("Deny"));
+ await act(() => flushPromises());
  expect(client.kick).toHaveBeenCalledWith(roomId, bob.userId);
  });
 
- it("fails to deny a request", async () => {
+ it("displays an error when a deny request fails", async () => {
  jest.spyOn(client, "kick").mockRejectedValue(error);
  getComponent(room);
  fireEvent.click(getButton("Deny"));
@@ -187,14 +223,15 @@ describe("RoomKnocksBar", () => {
  expect(getButton("Approve")).toHaveAttribute("disabled");
  });
 
- it("calls invite on approve", () => {
+ it("calls invite on approve", async () => {
  jest.spyOn(client, "invite").mockResolvedValue({});
  getComponent(room);
  fireEvent.click(getButton("Approve"));
- expect(client.kick).toHaveBeenCalledWith(roomId, bob.userId);
+ await act(() => flushPromises());
+ expect(client.invite).toHaveBeenCalledWith(roomId, bob.userId);
  });
 
- it("fails to approve a request", async () => {
+ it("displays an error when an approval fails", async () => {
  jest.spyOn(client, "invite").mockRejectedValue(error);
  getComponent(room);
  fireEvent.click(getButton("Approve"));
@@ -205,7 +242,7 @@ describe("RoomKnocksBar", () => {
  });
  });
 
- it("succeeds to deny/approve a request", () => {
+ it("hides the bar when someone else approves or denies the waiting person", () => {
  getComponent(room);
  jest.spyOn(room, "getMembersWithMembership").mockReturnValue([]);
  act(() => {