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

Commit

Permalink
Add tabs to the right panel (#12672)
Browse files Browse the repository at this point in the history
* Create new method for header button behaviour

With the introduction of tabs, the behaviour of the header buttons is
changed as follows:
- Close any right panel if open
- Open the correct right panel if no panel was open before

The old method (and behaviour) is retained as showOrHidePhase.

* Implement tabs in the right panel

There are three tabs: Info, People and Threads

* Remove unwanted code from RoomSummaryCard

- Remove the menu item for opening the memberlist since that is now
  taken of by the tabs.
- Remove the close button

* Remove code for focusing close button from tac item

See #12410

There's no longer a close button to focus so we instead focus the thread
tab. This is done in RightPaneltabs.tsx so we just need to remove this
code.

* Introduce a room info icon to the header

This was previously present in the legacy room header but not in the new
header.

* BaseCard changes

- Adds id, ariaLabelledBy and role props to implement tab accessibility.
- Adds hideHeaderButtons prop to hide header buttons (think back and
  close buttons).
- Change confusing header rendering code:
  header is not rendered ONLY when no header is passed AND
  hideHeaderButtons is true.

* Refactor repeated code into function

Created a new function createSpaceScopeHeader which returns the
component if the room is a space room. Previously this code was
duplicated in every component that uses SpaceScopeHeader component.

* Pass BaseCard attributes and use helper function

Actually using the code from the last two commits

* Add, update and remove tests/screenshots/snapshots

* Fix distance between search bar and tabs

* Update compound

* Update screenshots/snapshots
  • Loading branch information
MidhunSureshR authored Jul 9, 2024
1 parent cd39d91 commit cf8b87f
Show file tree
Hide file tree
Showing 41 changed files with 501 additions and 294 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"@sentry/browser": "^8.0.0",
"@testing-library/react-hooks": "^8.0.1",
"@vector-im/compound-design-tokens": "^1.2.0",
"@vector-im/compound-web": "^5.1.2",
"@vector-im/compound-web": "^5.2.3",
"@zxcvbn-ts/core": "^3.0.4",
"@zxcvbn-ts/language-common": "^3.0.4",
"@zxcvbn-ts/language-en": "^3.0.2",
Expand Down
4 changes: 2 additions & 2 deletions playwright/e2e/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const verify = async (page: Page, bob: Bot) => {
const bobsVerificationRequestPromise = waitForVerificationRequest(bob);

const roomInfo = await openRoomInfo(page);
await roomInfo.getByRole("menuitem", { name: "People" }).click();
await page.locator(".mx_RightPanelTabs").getByText("People").click();
await roomInfo.getByText("Bob").click();
await roomInfo.getByRole("button", { name: "Verify" }).click();
await roomInfo.getByRole("button", { name: "Start Verification" }).click();
Expand Down Expand Up @@ -279,7 +279,7 @@ test.describe("Cryptography", function () {

// Assert that verified icon is rendered
await page.getByRole("button", { name: "Room members" }).click();
await page.getByRole("button", { name: "Room information" }).click();
await page.locator(".mx_RightPanelTabs").getByText("Info").click();
await expect(page.locator('.mx_RoomSummaryCard_badges [data-kind="success"]')).toContainText("Encrypted");

// Take a snapshot of RoomSummaryCard with a verified E2EE icon
Expand Down
2 changes: 1 addition & 1 deletion playwright/e2e/crypto/dehydration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ test.describe("Dehydration", () => {

await viewRoomSummaryByName(page, app, ROOM_NAME);

await page.getByRole("menuitem", { name: "People" }).click();
await page.locator(".mx_RightPanelTabs").getByText("People").click();
await expect(page.locator(".mx_MemberList")).toBeVisible();

await getMemberTileByName(page, NAME).click();
Expand Down
2 changes: 1 addition & 1 deletion playwright/e2e/lazy-loading/lazy-loading.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ test.describe("Lazy Loading", () => {

async function openMemberlist(page: Page): Promise<void> {
await page.locator(".mx_LegacyRoomHeader").getByRole("button", { name: "Room info" }).click();
await page.locator(".mx_RoomSummaryCard").getByRole("menuitem", { name: "People" }).click(); // \d represents the number of the room members
await page.locator(".mx_RightPanelTabs").getByText("People").click();
}

function getMemberInMemberlist(page: Page, name: string): Locator {
Expand Down
7 changes: 3 additions & 4 deletions playwright/e2e/read-receipts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,19 +399,18 @@ class Helpers {
}

/**
* Close the threads panel. (Actually, close any right panel, but for these
* tests we only open the threads panel.)
* Close the threads panel.
*/
async closeThreadsPanel() {
await this.page.locator(".mx_RightPanel").getByLabel("Close").click();
await this.page.locator(".mx_LegacyRoomHeader").getByLabel("Threads").click();
await expect(this.page.locator(".mx_RightPanel")).not.toBeVisible();
}

/**
* Return to the list of threads, given we are viewing a single thread.
*/
async backToThreadsList() {
await this.page.locator(".mx_RightPanel").getByLabel("Threads").click();
await this.page.locator(".mx_LegacyRoomHeader").getByLabel("Threads").click();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions playwright/e2e/right-panel/right-panel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ test.describe("RightPanel", () => {
test("should handle viewing room member", async ({ page, app }) => {
await viewRoomSummaryByName(page, app, ROOM_NAME);

await page.getByRole("menuitem", { name: "People" }).click();
await page.locator(".mx_RightPanelTabs").getByText("People").click();
await expect(page.locator(".mx_MemberList")).toBeVisible();

await getMemberTileByName(page, NAME).click();
Expand All @@ -123,7 +123,7 @@ test.describe("RightPanel", () => {
await page.getByRole("button", { name: "Room members" }).click();
await expect(page.locator(".mx_MemberList")).toBeVisible();

await page.getByRole("button", { name: "Room information" }).click();
await page.locator(".mx_RightPanelTabs").getByText("Info").click();
await checkRoomSummaryCard(page, ROOM_NAME);
});
});
Expand Down
8 changes: 3 additions & 5 deletions playwright/e2e/spaces/threads-activity-centre/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,10 @@ export class Helpers {
}

/**
* Assert that the thread panel is focused (actually the 'close' button, specifically)
* Assert that the thread tab is focused
*/
assertThreadPanelFocused() {
return expect(
this.page.locator(".mx_ThreadPanel").locator(".mx_BaseCard_header").getByLabel("Close"),
).toBeFocused();
assertThreadTabFocused() {
return expect(this.page.locator("#thread-panel-tab")).toBeFocused();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,12 @@ test.describe("Threads Activity Centre", () => {
await util.assertNoTacIndicator();
});

test("should focus the thread panel close button when clicking an item in the TAC", async ({
room1,
room2,
util,
msg,
}) => {
test("should focus the thread tab when clicking an item in the TAC", async ({ room1, room2, util, msg }) => {
await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);

await util.openTac();
await util.clickRoomInTac(room1.name);

await util.assertThreadPanelFocused();
await util.assertThreadTabFocused();
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions res/css/_components.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@
@import "./views/right_panel/_BaseCard.pcss";
@import "./views/right_panel/_EncryptionInfo.pcss";
@import "./views/right_panel/_PinnedMessagesCard.pcss";
@import "./views/right_panel/_RightPanelTabs.pcss";
@import "./views/right_panel/_RoomSummaryCard.pcss";
@import "./views/right_panel/_ThreadPanel.pcss";
@import "./views/right_panel/_TimelineCard.pcss";
Expand Down
25 changes: 25 additions & 0 deletions res/css/views/right_panel/_RightPanelTabs.pcss
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
Copyright 2024 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

.mx_RightPanelTabs {
margin: 0;
height: 64px;
box-sizing: border-box;

ul {
margin-left: 16px;
}
}
2 changes: 1 addition & 1 deletion res/css/views/right_panel/_RoomSummaryCard.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ limitations under the License.
}

.mx_RoomSummaryCard_header {
padding: 15px 12px;
padding: 24px 12px 15px;
}

.mx_RoomSummaryCard_search {
Expand Down
1 change: 1 addition & 0 deletions res/css/views/rooms/_MemberList.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
display: flex;
flex-direction: column;
min-height: 0;
margin-top: 24px;

.mx_Spinner {
flex: 1 0 auto;
Expand Down
4 changes: 3 additions & 1 deletion src/components/structures/RightPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { UPDATE_EVENT } from "../../stores/AsyncStore";
import { IRightPanelCard, IRightPanelCardState } from "../../stores/right-panel/RightPanelStoreIPanelState";
import { Action } from "../../dispatcher/actions";
import { XOR } from "../../@types/common";
import { RightPanelTabs } from "../views/right_panel/RightPanelTabs";

interface BaseProps {
overwriteCard?: IRightPanelCard; // used to display a custom card and ignoring the RightPanelStore (used for UserView)
Expand Down Expand Up @@ -171,6 +172,7 @@ export default class RightPanel extends React.Component<Props, IState> {
<MemberList
roomId={roomId}
key={roomId}
hideHeaderButtons
onClose={this.onClose}
searchQuery={this.state.searchQuery}
onSearchQueryChanged={this.onSearchQueryChanged}
Expand Down Expand Up @@ -294,7 +296,6 @@ export default class RightPanel extends React.Component<Props, IState> {
card = (
<RoomSummaryCard
room={this.props.room}
onClose={this.onClose}
// whenever RightPanel is passed a room it is passed a permalinkcreator
permalinkCreator={this.props.permalinkCreator!}
onSearchChange={this.props.onSearchChange}
Expand All @@ -314,6 +315,7 @@ export default class RightPanel extends React.Component<Props, IState> {

return (
<aside className="mx_RightPanel" id="mx_RightPanel">
{phase && <RightPanelTabs phase={phase} />}
{card}
</aside>
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1287,7 +1287,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
]);
}
} else {
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.RoomMemberList);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.RoomMemberList);
}
break;
case Action.View3pidInvite:
Expand Down
15 changes: 4 additions & 11 deletions src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ import { ButtonEvent } from "../views/elements/AccessibleButton";
import Spinner from "../views/elements/Spinner";
import Heading from "../views/typography/Heading";
import { clearRoomNotification } from "../../utils/notifications";
import { useDispatcher } from "../../hooks/useDispatcher";
import dis from "../../dispatcher/dispatcher";
import { Action } from "../../dispatcher/actions";

interface IProps {
roomId: string;
Expand Down Expand Up @@ -259,14 +256,6 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
}
}, [timelineSet, timelinePanel]);

useDispatcher(dis, (payload) => {
// This actually foucses the close button on the threads panel, as its the only interactive element,
// but at least it puts the user in the right area of the app.
if (payload.action === Action.FocusThreadsPanel) {
closeButonRef.current?.focus();
}
});

return (
<RoomContext.Provider
value={{
Expand All @@ -277,14 +266,18 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
}}
>
<BaseCard
hideHeaderButtons
header={
<ThreadPanelHeader
filterOption={filterOption}
setFilterOption={setFilterOption}
empty={!hasThreads}
/>
}
id="thread-panel"
className="mx_ThreadPanel"
ariaLabelledBy="thread-panel-tab"
role="tabpanel"
onClose={onClose}
withoutScrollContainer={true}
ref={card}
Expand Down
34 changes: 30 additions & 4 deletions src/components/views/right_panel/BaseCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ import { CardContext } from "./context";

interface IProps {
header?: ReactNode | null;
hideHeaderButtons?: boolean;
footer?: ReactNode;
className?: string;
id?: string;
role?: "tabpanel";
ariaLabelledBy?: string;
withoutScrollContainer?: boolean;
closeLabel?: string;
onClose?(ev: ButtonEvent): void;
Expand Down Expand Up @@ -62,6 +66,10 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
onClose,
onBack,
className,
id,
ariaLabelledBy,
role,
hideHeaderButtons,
header,
footer,
withoutScrollContainer,
Expand Down Expand Up @@ -100,13 +108,31 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
children = <AutoHideScrollbar>{children}</AutoHideScrollbar>;
}

let headerButtons: React.ReactElement | undefined;
if (!hideHeaderButtons) {
headerButtons = (
<>
{backButton}
{closeButton}
</>
);
}

const shouldRenderHeader = header || !hideHeaderButtons;

return (
<CardContext.Provider value={{ isCard: true }}>
<div className={classNames("mx_BaseCard", className)} ref={ref} onKeyDown={onKeyDown}>
{header !== null && (
<div
id={id}
aria-labelledby={ariaLabelledBy}
role={role}
className={classNames("mx_BaseCard", className)}
ref={ref}
onKeyDown={onKeyDown}
>
{shouldRenderHeader && (
<div className="mx_BaseCard_header">
{backButton}
{closeButton}
{headerButtons}
<div className="mx_BaseCard_headerProp">{header}</div>
</div>
)}
Expand Down
12 changes: 6 additions & 6 deletions src/components/views/right_panel/LegacyRoomHeaderButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,27 +214,27 @@ export default class LegacyRoomHeaderButtons extends HeaderButtons<IProps> {
const currentPhase = RightPanelStore.instance.currentCard.phase;
if (currentPhase && ROOM_INFO_PHASES.includes(currentPhase)) {
if (this.state.phase === currentPhase) {
RightPanelStore.instance.showOrHidePanel(currentPhase);
RightPanelStore.instance.showOrHidePhase(currentPhase);
} else {
RightPanelStore.instance.showOrHidePanel(currentPhase, RightPanelStore.instance.currentCard.state);
RightPanelStore.instance.showOrHidePhase(currentPhase, RightPanelStore.instance.currentCard.state);
}
} else {
// This toggles for us, if needed
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.RoomSummary);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.RoomSummary);
}
};

private onNotificationsClicked = (): void => {
// This toggles for us, if needed
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.NotificationPanel);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.NotificationPanel);
};

private onPinnedMessagesClicked = (): void => {
// This toggles for us, if needed
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.PinnedMessages);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.PinnedMessages);
};
private onTimelineCardClicked = (): void => {
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.Timeline);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.Timeline);
};

private onThreadsPanelClicked = (ev: ButtonEvent): void => {
Expand Down
Loading

0 comments on commit cf8b87f

Please sign in to comment.