Open room settings on room header avatar click (#88)

* Open room settings on room header avatar click Signed-off-by: Michael Telatynski <[email protected]> * Fix nested interactive elements aria fail Signed-off-by: Michael Telatynski <[email protected]> * Update things for a11y and update snapshots Signed-off-by: Michael Telatynski <[email protected]> * Fix tests Signed-off-by: Michael Telatynski <[email protected]> * Iterate tests Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]>
element-hq · Sep 26, 2024 · 34d1875 · 34d1875
1 parent 3f67819
commit 34d1875
Show file tree

Hide file tree

Showing 12 changed files with 103 additions and 58 deletions.
diff --git a/playwright/e2e/room/room-header.spec.ts b/playwright/e2e/room/room-header.spec.ts
@@ -45,8 +45,8 @@ test.describe("Room Header", () => {
             await expect(header.getByRole("button", { name: "Threads" })).toBeVisible();
             await expect(header.getByRole("button", { name: "Notifications" })).toBeVisible();
 
-            // Assert that there are six buttons in total
-            await expect(header.getByRole("button")).toHaveCount(7);
+            // Assert that there are eight buttons in total
+            await expect(header.getByRole("button")).toHaveCount(8);
 
             await expect(header).toMatchScreenshot("room-header.png");
         });
@@ -119,7 +119,7 @@ test.describe("Room Header", () => {
                 await expect(header.getByRole("button", { name: "Notifications" })).toBeVisible();
 
                 // Assert that there is not a button except those buttons
-                await expect(header.getByRole("button")).toHaveCount(6);
+                await expect(header.getByRole("button")).toHaveCount(7);
 
                 await expect(header).toMatchScreenshot("room-header-video-room.png");
             });

diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts
@@ -345,6 +345,7 @@ export const expect = baseExpect.extend({
 
         if (!options?.showTooltips) {
             css += `
+                [role="tooltip"],
                 .mx_Tooltip_visible {
                     visibility: hidden !important;
                 }

diff --git a/playwright/snapshots/editing/editing.spec.ts/message-edit-history-dialog-linux.png b/playwright/snapshots/editing/editing.spec.ts/message-edit-history-dialog-linux.png
diff --git a/playwright/snapshots/room/room-header.spec.ts/room-header-long-name-linux.png b/playwright/snapshots/room/room-header.spec.ts/room-header-long-name-linux.png
diff --git a/...snapshots/threads/threads.spec.ts/Reply-to-the-location-on-ThreadView-linux.png b/...snapshots/threads/threads.spec.ts/Reply-to-the-location-on-ThreadView-linux.png
diff --git a/res/css/views/rooms/_RoomHeader.pcss b/res/css/views/rooms/_RoomHeader.pcss
@@ -9,7 +9,7 @@ Please see LICENSE files in the repository root for full details.
 .mx_RoomHeader {
     height: 64px;
     box-sizing: border-box;
-    padding: 0 var(--cpd-space-3x);
+    padding: 0 var(--cpd-space-3x) 0 calc(var(--cpd-space-3x) + var(--cpd-space-1-5x));
     border-bottom: 1px solid $separator;
     background-color: $background;
     transition: all 0.2s ease;
@@ -31,6 +31,8 @@ Please see LICENSE files in the repository root for full details.
     cursor: pointer;
     gap: var(--cpd-space-3x);
     text-align: left;
+    height: 100%;
+    padding: 0;
 }
 
 .mx_RoomHeader_info {

diff --git a/src/components/views/avatars/BaseAvatar.tsx b/src/components/views/avatars/BaseAvatar.tsx
@@ -9,7 +9,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
-import React, { forwardRef, useCallback, useContext, useEffect, useState } from "react";
+import React, { AriaRole, forwardRef, useCallback, useContext, useEffect, useState } from "react";
 import classNames from "classnames";
 import { ClientEvent, SyncState } from "matrix-js-sdk/src/matrix";
 import { Avatar } from "@vector-im/compound-web";
@@ -33,6 +33,7 @@ interface IProps {
     className?: string;
     tabIndex?: number;
     altText?: string;
+    role?: AriaRole;
 }
 
 const calculateUrls = (url?: string | null, urls?: string[], lowBandwidth = false): string[] => {

diff --git a/src/components/views/rooms/RoomHeader.tsx b/src/components/views/rooms/RoomHeader.tsx
@@ -50,6 +50,8 @@ import WithPresenceIndicator, { useDmMember } from "../avatars/WithPresenceIndic
 import { IOOBData } from "../../../stores/ThreepidInviteStore";
 import RoomContext from "../../../contexts/RoomContext";
 import { MainSplitContentType } from "../../structures/RoomView";
+import defaultDispatcher from "../../../dispatcher/dispatcher.ts";
+import { RoomSettingsTab } from "../dialogs/RoomSettingsDialog.tsx";
 
 export default function RoomHeader({
     room,
@@ -229,18 +231,33 @@ export default function RoomHeader({
         roomContext.mainSplitContentType === MainSplitContentType.MaximisedWidget ||
         roomContext.mainSplitContentType === MainSplitContentType.Call;
 
+    const onAvatarClick = (): void => {
+        defaultDispatcher.dispatch({
+            action: "open_room_settings",
+            initial_tab_id: RoomSettingsTab.General,
+        });
+    };
+
     return (
         <>
             <Flex as="header" align="center" gap="var(--cpd-space-3x)" className="mx_RoomHeader light-panel">
+                <WithPresenceIndicator room={room} size="8px">
+                    {/* We hide this from the tabIndex list as it is a pointer shortcut and superfluous for a11y */}
+                    <RoomAvatar
+                        room={room}
+                        size="40px"
+                        oobData={oobData}
+                        onClick={onAvatarClick}
+                        tabIndex={-1}
+                        aria-label={_t("room|header_avatar_open_settings_label")}
+                    />
+                </WithPresenceIndicator>
                 <button
                     aria-label={_t("right_panel|room_summary_card|title")}
                     tabIndex={0}
                     onClick={() => RightPanelStore.instance.showOrHidePanel(RightPanelPhases.RoomSummary)}
                     className="mx_RoomHeader_infoWrapper"
                 >
-                    <WithPresenceIndicator room={room} size="8px">
-                        <RoomAvatar room={room} size="40px" oobData={oobData} />
-                    </WithPresenceIndicator>
                     <Box flex="1" className="mx_RoomHeader_info">
                         <BodyText
                             as="div"

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -1983,6 +1983,7 @@
             "video_call_ec_layout_spotlight": "Spotlight",
             "video_room_view_chat_button": "View chat timeline"
         },
+        "header_avatar_open_settings_label": "Open room settings",
         "header_face_pile_tooltip": "People",
         "header_untrusted_label": "Untrusted",
         "inaccessible": "This room or space is not accessible at this time.",

diff --git a/test/components/structures/__snapshots__/RoomView-test.tsx.snap b/test/components/structures/__snapshots__/RoomView-test.tsx.snap
@@ -9,21 +9,24 @@ exports[`RoomView for a local room in state CREATING should match the snapshot 1
       class="mx_Flex mx_RoomHeader light-panel"
       style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x);"
     >
+      <button
+        aria-label="Open room settings"
+        aria-live="off"
+        class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
+        data-color="3"
+        data-testid="avatar-img"
+        data-type="round"
+        role="button"
+        style="--cpd-avatar-size: 40px;"
+        tabindex="-1"
+      >
+        u
+      </button>
       <button
         aria-label="Room info"
         class="mx_RoomHeader_infoWrapper"
         tabindex="0"
       >
-        <span
-          class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
-          data-color="3"
-          data-testid="avatar-img"
-          data-type="round"
-          role="presentation"
-          style="--cpd-avatar-size: 40px;"
-        >
-          u
-        </span>
         <div
           class="mx_Box mx_RoomHeader_info mx_Box--flex"
           style="--mx-box-flex: 1;"
@@ -179,21 +182,24 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
       class="mx_Flex mx_RoomHeader light-panel"
       style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x);"
     >
+      <button
+        aria-label="Open room settings"
+        aria-live="off"
+        class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
+        data-color="3"
+        data-testid="avatar-img"
+        data-type="round"
+        role="button"
+        style="--cpd-avatar-size: 40px;"
+        tabindex="-1"
+      >
+        u
+      </button>
       <button
         aria-label="Room info"
         class="mx_RoomHeader_infoWrapper"
         tabindex="0"
       >
-        <span
-          class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
-          data-color="3"
-          data-testid="avatar-img"
-          data-type="round"
-          role="presentation"
-          style="--cpd-avatar-size: 40px;"
-        >
-          u
-        </span>
         <div
           class="mx_Box mx_RoomHeader_info mx_Box--flex"
           style="--mx-box-flex: 1;"
@@ -434,21 +440,24 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] =
       class="mx_Flex mx_RoomHeader light-panel"
       style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x);"
     >
+      <button
+        aria-label="Open room settings"
+        aria-live="off"
+        class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
+        data-color="3"
+        data-testid="avatar-img"
+        data-type="round"
+        role="button"
+        style="--cpd-avatar-size: 40px;"
+        tabindex="-1"
+      >
+        u
+      </button>
       <button
         aria-label="Room info"
         class="mx_RoomHeader_infoWrapper"
         tabindex="0"
       >
-        <span
-          class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
-          data-color="3"
-          data-testid="avatar-img"
-          data-type="round"
-          role="presentation"
-          style="--cpd-avatar-size: 40px;"
-        >
-          u
-        </span>
         <div
           class="mx_Box mx_RoomHeader_info mx_Box--flex"
           style="--mx-box-flex: 1;"
@@ -766,21 +775,24 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
       class="mx_Flex mx_RoomHeader light-panel"
       style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x);"
     >
+      <button
+        aria-label="Open room settings"
+        aria-live="off"
+        class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
+        data-color="3"
+        data-testid="avatar-img"
+        data-type="round"
+        role="button"
+        style="--cpd-avatar-size: 40px;"
+        tabindex="-1"
+      >
+        u
+      </button>
       <button
         aria-label="Room info"
         class="mx_RoomHeader_infoWrapper"
         tabindex="0"
       >
-        <span
-          class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
-          data-color="3"
-          data-testid="avatar-img"
-          data-type="round"
-          role="presentation"
-          style="--cpd-avatar-size: 40px;"
-        >
-          u
-        </span>
         <div
           class="mx_Box mx_RoomHeader_info mx_Box--flex"
           style="--mx-box-flex: 1;"

diff --git a/test/components/views/rooms/RoomHeader-test.tsx b/test/components/views/rooms/RoomHeader-test.tsx
@@ -683,6 +683,14 @@ describe("RoomHeader", () => {
             expect(screen.getByRole("heading", { name: "Asking to join" })).toBeInTheDocument();
         });
     });
+
+    it("should open room settings when clicking the room avatar", async () => {
+        const { container } = render(<RoomHeader room={room} />, getWrapper());
+
+        const dispatcherSpy = jest.spyOn(dispatcher, "dispatch");
+        fireEvent.click(getByLabelText(container, "Open room settings"));
+        expect(dispatcherSpy).toHaveBeenCalledWith(expect.objectContaining({ action: "open_room_settings" }));
+    });
 });
 
 /**

diff --git a/test/components/views/rooms/__snapshots__/RoomHeader-test.tsx.snap b/test/components/views/rooms/__snapshots__/RoomHeader-test.tsx.snap
@@ -6,21 +6,24 @@ exports[`RoomHeader dm does not show the face pile for DMs 1`] = `
     class="mx_Flex mx_RoomHeader light-panel"
     style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x);"
   >
+    <button
+      aria-label="Open room settings"
+      aria-live="off"
+      class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
+      data-color="3"
+      data-testid="avatar-img"
+      data-type="round"
+      role="button"
+      style="--cpd-avatar-size: 40px;"
+      tabindex="-1"
+    >
+      !
+    </button>
     <button
       aria-label="Room info"
       class="mx_RoomHeader_infoWrapper"
       tabindex="0"
     >
-      <span
-        class="_avatar_mcap2_17 mx_BaseAvatar _avatar-imageless_mcap2_61"
-        data-color="3"
-        data-testid="avatar-img"
-        data-type="round"
-        role="presentation"
-        style="--cpd-avatar-size: 40px;"
-      >
-        !
-      </span>
       <div
         class="mx_Box mx_RoomHeader_info mx_Box--flex"
         style="--mx-box-flex: 1;"