Add tabs to the right panel (#12672)

* 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 https://github.com/matrix-org/matrix-react-sdk/pull/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
This commit is contained in:
R Midhun Suresh 2024-07-09 17:06:50 +05:30 committed by GitHub
parent cd39d91c15
commit cf8b87fd14
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
41 changed files with 501 additions and 294 deletions

View file

@ -36,8 +36,6 @@ import ResizeNotifier from "../../../src/utils/ResizeNotifier";
import { createTestClient, getRoomContext, mkRoom, mockPlatformPeg, stubClient } from "../../test-utils";
import { mkThread } from "../../test-utils/threads";
import { IRoomState } from "../../../src/components/structures/RoomView";
import defaultDispatcher from "../../../src/dispatcher/dispatcher";
import { Action } from "../../../src/dispatcher/actions";
jest.mock("../../../src/utils/Feedback");
@ -148,43 +146,6 @@ describe("ThreadPanel", () => {
fireEvent.click(getByRole(container, "button", { name: "Mark all as read" }));
await waitFor(() => expect(mockClient.sendReadReceipt).not.toHaveBeenCalled());
});
it("focuses the close button on FocusThreadsPanel dispatch", () => {
const ROOM_ID = "!roomId:example.org";
stubClient();
mockPlatformPeg();
const mockClient = mocked(MatrixClientPeg.safeGet());
const room = new Room(ROOM_ID, mockClient, mockClient.getUserId() ?? "", {
pendingEventOrdering: PendingEventOrdering.Detached,
});
render(
<MatrixClientContext.Provider value={mockClient}>
<RoomContext.Provider
value={getRoomContext(room, {
canSendMessages: true,
})}
>
<ThreadPanel
roomId={ROOM_ID}
onClose={jest.fn()}
resizeNotifier={new ResizeNotifier()}
permalinkCreator={new RoomPermalinkCreator(room)}
/>
</RoomContext.Provider>
</MatrixClientContext.Provider>,
);
// Unfocus it first so we know it's not just focused by coincidence
screen.getByTestId("base-card-close-button").blur();
expect(screen.getByTestId("base-card-close-button")).not.toHaveFocus();
defaultDispatcher.dispatch({ action: Action.FocusThreadsPanel }, true);
expect(screen.getByTestId("base-card-close-button")).toHaveFocus();
});
});
describe("Filtering", () => {

View file

@ -0,0 +1,72 @@
/*
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.
*/
import React from "react";
import { render, fireEvent } from "@testing-library/react";
import dis from "../../../../src/dispatcher/dispatcher";
import RightPanelStore from "../../../../src/stores/right-panel/RightPanelStore";
import { RightPanelPhases } from "../../../../src/stores/right-panel/RightPanelStorePhases";
import { RightPanelTabs } from "../../../../src/components/views/right_panel/RightPanelTabs";
import { Action } from "../../../../src/dispatcher/actions";
describe("<RightPanelTabs />", () => {
it("Component renders the correct tabs", () => {
const { container, getByRole } = render(<RightPanelTabs phase={RightPanelPhases.RoomSummary} />);
expect(container).toMatchSnapshot();
// We expect Info, People and Threads as tabs
expect(getByRole("tab", { name: "Info" })).toBeDefined();
expect(getByRole("tab", { name: "People" })).toBeDefined();
expect(getByRole("tab", { name: "Threads" })).toBeDefined();
});
it("Correct tab is active", () => {
const { container } = render(<RightPanelTabs phase={RightPanelPhases.RoomMemberList} />);
expect(container).toMatchSnapshot();
// Assert that the active tab is Info
expect(container.querySelectorAll("[aria-selected='true'").length).toEqual(1);
expect(container.querySelector("[aria-selected='true'")).toHaveAccessibleName("People");
});
it("Renders nothing for some phases, eg: FilePanel", () => {
const { container } = render(<RightPanelTabs phase={RightPanelPhases.FilePanel} />);
expect(container).toBeEmptyDOMElement();
});
it("onClick behaviors work as expected", () => {
const spy = jest.spyOn(RightPanelStore.instance, "pushCard");
const { getByRole } = render(<RightPanelTabs phase={RightPanelPhases.RoomSummary} />);
// Info -> People
fireEvent.click(getByRole("tab", { name: "People" }));
expect(spy).toHaveBeenLastCalledWith({ phase: RightPanelPhases.RoomMemberList }, true);
// People -> Threads
fireEvent.click(getByRole("tab", { name: "Threads" }));
expect(spy).toHaveBeenLastCalledWith({ phase: RightPanelPhases.ThreadPanel }, true);
// Threads -> Info
fireEvent.click(getByRole("tab", { name: "Info" }));
expect(spy).toHaveBeenLastCalledWith({ phase: RightPanelPhases.RoomSummary }, true);
});
it("Threads tab is focused on action", () => {
const { getByRole } = render(<RightPanelTabs phase={RightPanelPhases.ThreadPanel} />);
dis.dispatch({ action: Action.FocusThreadsPanel }, true);
expect(getByRole("tab", { name: "Threads" })).toHaveFocus();
});
});

View file

@ -35,7 +35,6 @@ import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser } f
import { PollHistoryDialog } from "../../../../src/components/views/dialogs/PollHistoryDialog";
import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
import { _t } from "../../../../src/languageHandler";
import SettingsStore from "../../../../src/settings/SettingsStore";
import { tagRoom } from "../../../../src/utils/room/tagRoom";
import { DefaultTagID } from "../../../../src/stores/room-list/models";
import { Action } from "../../../../src/dispatcher/actions";
@ -195,7 +194,6 @@ describe("<RoomSummaryCard />", () => {
<RoomSummaryCard
room={room}
permalinkCreator={new RoomPermalinkCreator(room)}
onClose={jest.fn()}
onSearchChange={onSearchChange}
focusRoomSearch={true}
/>
@ -212,7 +210,6 @@ describe("<RoomSummaryCard />", () => {
<RoomSummaryCard
room={room}
permalinkCreator={new RoomPermalinkCreator(room)}
onClose={jest.fn()}
onSearchChange={onSearchChange}
/>
</RoomContext.Provider>
@ -270,18 +267,6 @@ describe("<RoomSummaryCard />", () => {
expect(defaultDispatcher.dispatch).toHaveBeenCalledWith({ action: "open_room_settings" });
});
it("renders room members options when new room UI is not enabled", () => {
jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
const { getByText } = getComponent();
fireEvent.click(getByText(_t("common|people")));
expect(RightPanelStore.instance.pushCard).toHaveBeenCalledWith(
{ phase: RightPanelPhases.RoomMemberList },
true,
);
});
describe("pinning", () => {
it("renders pins options when pinning feature is enabled", () => {
mocked(settingsHooks.useFeatureEnabled).mockImplementation((feature) => feature === "feature_pinning");

View file

@ -0,0 +1,119 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`<RightPanelTabs /> Component renders the correct tabs 1`] = `
<div>
<nav
class="mx_RightPanelTabs _nav-bar_135dy_16"
role="presentation"
>
<ul
aria-label="right panel"
class="_nav-bar-items_135dy_22"
role="tablist"
>
<li
class="_nav-tab_135dy_33"
data-current="true"
role="presentation"
>
<button
aria-controls="room-summary-panel"
aria-selected="true"
class="_nav-item_135dy_55"
id="room-summary-panel-tab"
role="tab"
>
Info
</button>
</li>
<li
class="_nav-tab_135dy_33"
role="presentation"
>
<button
aria-controls="memberlist-panel"
aria-selected="false"
class="_nav-item_135dy_55"
id="memberlist-panel-tab"
role="tab"
>
People
</button>
</li>
<li
class="_nav-tab_135dy_33"
role="presentation"
>
<button
aria-controls="thread-panel"
aria-selected="false"
class="_nav-item_135dy_55"
id="thread-panel-tab"
role="tab"
>
Threads
</button>
</li>
</ul>
</nav>
</div>
`;
exports[`<RightPanelTabs /> Correct tab is active 1`] = `
<div>
<nav
class="mx_RightPanelTabs _nav-bar_135dy_16"
role="presentation"
>
<ul
aria-label="right panel"
class="_nav-bar-items_135dy_22"
role="tablist"
>
<li
class="_nav-tab_135dy_33"
role="presentation"
>
<button
aria-controls="room-summary-panel"
aria-selected="false"
class="_nav-item_135dy_55"
id="room-summary-panel-tab"
role="tab"
>
Info
</button>
</li>
<li
class="_nav-tab_135dy_33"
data-current="true"
role="presentation"
>
<button
aria-controls="memberlist-panel"
aria-selected="true"
class="_nav-item_135dy_55"
id="memberlist-panel-tab"
role="tab"
>
People
</button>
</li>
<li
class="_nav-tab_135dy_33"
role="presentation"
>
<button
aria-controls="thread-panel"
aria-selected="false"
class="_nav-item_135dy_55"
id="thread-panel-tab"
role="tab"
>
Threads
</button>
</li>
</ul>
</nav>
</div>
`;

View file

@ -3,7 +3,10 @@
exports[`<RoomSummaryCard /> has button to edit topic when expanded 1`] = `
<div>
<div
aria-labelledby="room-summary-panel-tab"
class="mx_BaseCard mx_RoomSummaryCard"
id="room-summary-panel"
role="tabpanel"
>
<div
class="mx_AutoHideScrollbar"
@ -12,15 +15,7 @@ exports[`<RoomSummaryCard /> has button to edit topic when expanded 1`] = `
<header
class="mx_Flex mx_RoomSummaryCard_header"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: space-between; --mx-flex-gap: var(--cpd-space-3x);"
>
<div
aria-label="Close"
class="mx_AccessibleButton mx_BaseCard_close"
data-testid="base-card-close-button"
role="button"
tabindex="0"
/>
</header>
/>
<header
class="mx_RoomSummaryCard_container"
>
@ -244,36 +239,6 @@ exports[`<RoomSummaryCard /> has button to edit topic when expanded 1`] = `
data-orientation="horizontal"
role="separator"
/>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
role="menuitem"
>
<div
aria-hidden="true"
class="_icon_1gwvj_44"
height="24"
width="24"
/>
<span
class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 _label_1gwvj_53"
>
People
</span>
<svg
aria-hidden="true"
class="_nav-hint_1gwvj_60"
fill="currentColor"
height="24"
viewBox="8 0 8 24"
width="8"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M8.7 17.3a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7l3.9-3.9-3.9-3.9a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275l4.6 4.6c.1.1.17.208.213.325.041.117.062.242.062.375s-.02.258-.063.375a.876.876 0 0 1-.212.325l-4.6 4.6a.948.948 0 0 1-.7.275.948.948 0 0 1-.7-.275Z"
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
@ -423,7 +388,10 @@ exports[`<RoomSummaryCard /> has button to edit topic when expanded 1`] = `
exports[`<RoomSummaryCard /> renders the room summary 1`] = `
<div>
<div
aria-labelledby="room-summary-panel-tab"
class="mx_BaseCard mx_RoomSummaryCard"
id="room-summary-panel"
role="tabpanel"
>
<div
class="mx_AutoHideScrollbar"
@ -432,15 +400,7 @@ exports[`<RoomSummaryCard /> renders the room summary 1`] = `
<header
class="mx_Flex mx_RoomSummaryCard_header"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: space-between; --mx-flex-gap: var(--cpd-space-3x);"
>
<div
aria-label="Close"
class="mx_AccessibleButton mx_BaseCard_close"
data-testid="base-card-close-button"
role="button"
tabindex="0"
/>
</header>
/>
<header
class="mx_RoomSummaryCard_container"
>
@ -637,36 +597,6 @@ exports[`<RoomSummaryCard /> renders the room summary 1`] = `
data-orientation="horizontal"
role="separator"
/>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
role="menuitem"
>
<div
aria-hidden="true"
class="_icon_1gwvj_44"
height="24"
width="24"
/>
<span
class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 _label_1gwvj_53"
>
People
</span>
<svg
aria-hidden="true"
class="_nav-hint_1gwvj_60"
fill="currentColor"
height="24"
viewBox="8 0 8 24"
width="8"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M8.7 17.3a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7l3.9-3.9-3.9-3.9a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275l4.6 4.6c.1.1.17.208.213.325.041.117.062.242.062.375s-.02.258-.063.375a.876.876 0 0 1-.212.325l-4.6 4.6a.948.948 0 0 1-.7.275.948.948 0 0 1-.7-.275Z"
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
@ -816,7 +746,10 @@ exports[`<RoomSummaryCard /> renders the room summary 1`] = `
exports[`<RoomSummaryCard /> renders the room topic in the summary 1`] = `
<div>
<div
aria-labelledby="room-summary-panel-tab"
class="mx_BaseCard mx_RoomSummaryCard"
id="room-summary-panel"
role="tabpanel"
>
<div
class="mx_AutoHideScrollbar"
@ -825,15 +758,7 @@ exports[`<RoomSummaryCard /> renders the room topic in the summary 1`] = `
<header
class="mx_Flex mx_RoomSummaryCard_header"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: space-between; --mx-flex-gap: var(--cpd-space-3x);"
>
<div
aria-label="Close"
class="mx_AccessibleButton mx_BaseCard_close"
data-testid="base-card-close-button"
role="button"
tabindex="0"
/>
</header>
/>
<header
class="mx_RoomSummaryCard_container"
>
@ -1041,36 +966,6 @@ exports[`<RoomSummaryCard /> renders the room topic in the summary 1`] = `
data-orientation="horizontal"
role="separator"
/>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"
role="menuitem"
>
<div
aria-hidden="true"
class="_icon_1gwvj_44"
height="24"
width="24"
/>
<span
class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 _label_1gwvj_53"
>
People
</span>
<svg
aria-hidden="true"
class="_nav-hint_1gwvj_60"
fill="currentColor"
height="24"
viewBox="8 0 8 24"
width="8"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M8.7 17.3a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7l3.9-3.9-3.9-3.9a.948.948 0 0 1-.275-.7.95.95 0 0 1 .275-.7.948.948 0 0 1 .7-.275.95.95 0 0 1 .7.275l4.6 4.6c.1.1.17.208.213.325.041.117.062.242.062.375s-.02.258-.063.375a.876.876 0 0 1-.212.325l-4.6 4.6a.948.948 0 0 1-.7.275.948.948 0 0 1-.7-.275Z"
/>
</svg>
</button>
<button
class="_item_1gwvj_17 _interactive_1gwvj_36"
data-kind="primary"

View file

@ -216,6 +216,13 @@ describe("RoomHeader", () => {
expect(setCardSpy).toHaveBeenCalledWith({ phase: RightPanelPhases.RoomMemberList });
});
it("has room info icon that opens the room info panel", async () => {
const { getAllByRole } = render(<RoomHeader room={room} />, getWrapper());
const infoButton = getAllByRole("button", { name: "Room info" })[1];
fireEvent.click(infoButton);
expect(setCardSpy).toHaveBeenCalledWith({ phase: RightPanelPhases.RoomSummary });
});
it("opens the thread panel", async () => {
const { container } = render(<RoomHeader room={room} />, getWrapper());

View file

@ -73,6 +73,20 @@ exports[`RoomHeader does not show the face pile for DMs 1`] = `
<div />
</div>
</button>
<button
aria-label="Room info"
class="_icon-button_rijzz_17"
role="button"
style="--cpd-icon-button-size: 32px;"
tabindex="0"
>
<div
class="_indicator-icon_133tf_26"
style="--cpd-icon-button-size: 100%;"
>
<div />
</div>
</button>
<button
aria-label="Threads"
class="_icon-button_rijzz_17"

View file

@ -225,4 +225,22 @@ describe("RightPanelStore", () => {
await viewRoom("!1:example.org");
expect(store.currentCardForRoom("!1:example.org").phase).toEqual(RightPanelPhases.RoomMemberList);
});
it("showOrHidePhase works as expected", async () => {
await viewRoom("!1:example.org");
// Open the memberlist panel
store.showOrHidePanel(RightPanelPhases.RoomMemberList);
expect(store.isOpenForRoom("!1:example.org")).toEqual(true);
expect(store.currentCardForRoom("!1:example.org").phase).toEqual(RightPanelPhases.RoomMemberList);
// showOrHide with RoomSummary should now close the panel
store.showOrHidePanel(RightPanelPhases.RoomSummary);
expect(store.isOpenForRoom("!1:example.org")).toEqual(false);
// showOrHide with RoomSummary should now open the panel
store.showOrHidePanel(RightPanelPhases.RoomSummary);
expect(store.isOpenForRoom("!1:example.org")).toEqual(true);
expect(store.currentCardForRoom("!1:example.org").phase).toEqual(RightPanelPhases.RoomSummary);
});
});

View file

@ -28,7 +28,7 @@ describe("onView3pidInvite()", () => {
beforeEach(() => {
rightPanelStore = {
pushCard: jest.fn(),
showOrHidePanel: jest.fn(),
showOrHidePhase: jest.fn(),
} as unknown as MockedObject<RightPanelStore>;
});
@ -38,7 +38,7 @@ describe("onView3pidInvite()", () => {
};
onView3pidInvite(payload, rightPanelStore);
expect(rightPanelStore.showOrHidePanel).toHaveBeenCalledWith(RightPanelPhases.RoomMemberList);
expect(rightPanelStore.showOrHidePhase).toHaveBeenCalledWith(RightPanelPhases.RoomMemberList);
expect(rightPanelStore.pushCard).not.toHaveBeenCalled();
});
@ -49,7 +49,7 @@ describe("onView3pidInvite()", () => {
};
onView3pidInvite(payload, rightPanelStore);
expect(rightPanelStore.showOrHidePanel).not.toHaveBeenCalled();
expect(rightPanelStore.showOrHidePhase).not.toHaveBeenCalled();
expect(rightPanelStore.pushCard).toHaveBeenCalledWith({
phase: RightPanelPhases.Room3pidMemberInfo,
state: { memberInfoEvent: payload.event },