Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open room settings on room header avatar click #88

Merged
merged 5 commits into from
Sep 26, 2024
Merged

Open room settings on room header avatar click #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e221a14
Open room settings on room header avatar click
t3chguy Sep 24, 2024
0d35857
Fix nested interactive elements aria fail
t3chguy Sep 24, 2024
476bc0e
Update things for a11y and update snapshots
t3chguy Sep 25, 2024
8e6192e
Fix tests
t3chguy Sep 25, 2024
219fbe2
Iterate tests
t3chguy Sep 25, 2024
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
6 changes: 3 additions & 3 deletions playwright/e2e/room/room-header.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
Expand Down Expand Up @@ -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");
});
Expand Down
1 change: 1 addition & 0 deletions playwright/element-web-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ export const expect = baseExpect.extend({

if (!options?.showTooltips) {
css += `
[role="tooltip"],
.mx_Tooltip_visible {
visibility: hidden !important;
}
Expand Down
Binary file modified playwright/snapshots/editing/editing.spec.ts/message-edit-history-dialog-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified playwright/snapshots/room/room-header.spec.ts/room-header-long-name-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...snapshots/threads/threads.spec.ts/Reply-to-the-location-on-ThreadView-linux.png
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

You've also lost a tooltip: is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah the showTooltips option was broken, in order to stabilise one of the tests here I fixed the option which made one of the other screenshots lose its tooltip

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion res/css/views/rooms/_RoomHeader.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/components/views/avatars/BaseAvatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -33,6 +33,7 @@ interface IProps {
className?: string;
tabIndex?: number;
altText?: string;
role?: AriaRole;
}

const calculateUrls = (url?: string | null, urls?: string[], lowBandwidth = false): string[] => {
Expand Down
23 changes: 20 additions & 3 deletions src/components/views/rooms/RoomHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1976,6 +1976,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.",
Expand Down
92 changes: 52 additions & 40 deletions test/components/structures/__snapshots__/RoomView-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;"
Expand Down Expand Up @@ -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;"
Expand Down Expand Up @@ -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;"
Expand Down Expand Up @@ -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;"
Expand Down
8 changes: 8 additions & 0 deletions test/components/views/rooms/RoomHeader-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }));
});
});

/**
Expand Down
23 changes: 13 additions & 10 deletions test/components/views/rooms/__snapshots__/RoomHeader-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;"
Expand Down
Loading