Ditch right panel tabs and re-add close button (#99)

* Add extra buttons to room summary card Signed-off-by: Michael Telatynski <[email protected]> * Remove right panel tabs in favour of X button on each panel Signed-off-by: Michael Telatynski <[email protected]> * Update room summary card header to align close button correctly Signed-off-by: Michael Telatynski <[email protected]> * Fix typo in pinned messages heading Signed-off-by: Michael Telatynski <[email protected]> * Update snapshots Signed-off-by: Michael Telatynski <[email protected]> * Update tests Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Update snapshots Signed-off-by: Michael Telatynski <[email protected]> * Update screenshot Signed-off-by: Michael Telatynski <[email protected]> * Improve coverage Signed-off-by: Michael Telatynski <[email protected]> * i18n Signed-off-by: Michael Telatynski <[email protected]> * Fix base card title colours Signed-off-by: Michael Telatynski <[email protected]> * Update snapshots Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]>
element-hq · Oct 3, 2024 · 2dbaf00 · 2dbaf00
1 parent 67cb8b7
commit 2dbaf00
Show file tree

Hide file tree

Showing 44 changed files with 864 additions and 499 deletions.
diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts
@@ -220,7 +220,7 @@ test.describe("Cryptography", function () {
 
         // Assert that verified icon is rendered
         await page.getByTestId("base-card-back-button").click();
-        await page.locator(".mx_RightPanelTabs").getByText("Info").click();
+        await page.getByLabel("Room info").nth(1).click();
         await expect(page.locator('.mx_RoomSummaryCard_badges [data-kind="success"]')).toContainText("Encrypted");
 
         // Take a snapshot of RoomSummaryCard with a verified E2EE icon

diff --git a/playwright/e2e/crypto/dehydration.spec.ts b/playwright/e2e/crypto/dehydration.spec.ts
@@ -94,7 +94,7 @@ test.describe("Dehydration", () => {
 
         await viewRoomSummaryByName(page, app, ROOM_NAME);
 
-        await page.locator(".mx_RightPanelTabs").getByText("People").click();
+        await page.locator(".mx_RightPanel").getByRole("menuitem", { name: "People" }).click();
         await expect(page.locator(".mx_MemberList")).toBeVisible();
 
         await getMemberTileByName(page, NAME).click();

diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts
@@ -345,7 +345,7 @@ export const verify = async (app: ElementAppPage, bob: Bot) => {
     const bobsVerificationRequestPromise = waitForVerificationRequest(bob);
 
     const roomInfo = await app.toggleRoomInfoPanel();
-    await page.locator(".mx_RightPanelTabs").getByText("People").click();
+    await page.locator(".mx_RightPanel").getByRole("menuitem", { name: "People" }).click();
     await roomInfo.getByText("Bob").click();
     await roomInfo.getByRole("button", { name: "Verify" }).click();
     await roomInfo.getByRole("button", { name: "Start Verification" }).click();

diff --git a/playwright/e2e/integration-manager/utils.ts b/playwright/e2e/integration-manager/utils.ts
@@ -11,6 +11,6 @@ import type { ElementAppPage } from "../../pages/ElementAppPage";
 export async function openIntegrationManager(app: ElementAppPage) {
     const { page } = app;
     await app.toggleRoomInfoPanel();
-    await page.getByRole("tab", { name: "Extensions" }).click();
+    await page.getByRole("menuitem", { name: "Extensions" }).click();
     await page.getByRole("button", { name: "Add extensions" }).click();
 }
diff --git a/playwright/e2e/lazy-loading/lazy-loading.spec.ts b/playwright/e2e/lazy-loading/lazy-loading.spec.ts
@@ -73,7 +73,7 @@ test.describe("Lazy Loading", () => {
     async function openMemberlist(app: ElementAppPage): Promise<void> {
         await app.toggleRoomInfoPanel();
         const { page } = app;
-        await page.locator(".mx_RightPanelTabs").getByText("People").click();
+        await page.locator(".mx_RightPanel").getByRole("menuitem", { name: "People" }).click();
     }
 
     function getMemberInMemberlist(page: Page, name: string): Locator {

diff --git a/playwright/e2e/read-receipts/index.ts b/playwright/e2e/read-receipts/index.ts
@@ -394,10 +394,7 @@ class Helpers {
      * Close the threads panel.
      */
     async closeThreadsPanel() {
-        await this.page.locator(".mx_RoomHeader").getByLabel("Threads").click();
-        if (await this.page.locator("#thread-panel").isVisible()) {
-            await this.page.locator(".mx_RoomHeader").getByLabel("Threads").click();
-        }
+        await this.page.locator(".mx_RightPanel").getByTestId("base-card-close-button").click();
         await expect(this.page.locator(".mx_RightPanel")).not.toBeVisible();
     }
 

diff --git a/playwright/e2e/right-panel/right-panel.spec.ts b/playwright/e2e/right-panel/right-panel.spec.ts
@@ -65,7 +65,7 @@ test.describe("RightPanel", () => {
         test("should handle clicking add widgets", async ({ page, app }) => {
             await viewRoomSummaryByName(page, app, ROOM_NAME);
 
-            await page.getByRole("tab", { name: "Extensions" }).click();
+            await page.getByRole("menuitem", { name: "Extensions" }).click();
             await page.getByRole("button", { name: "Add extensions" }).click();
             await expect(page.locator(".mx_IntegrationManager")).toBeVisible();
         });
@@ -106,7 +106,7 @@ test.describe("RightPanel", () => {
         test("should handle viewing room member", async ({ page, app }) => {
             await viewRoomSummaryByName(page, app, ROOM_NAME);
 
-            await page.locator(".mx_RightPanelTabs").getByText("People").click();
+            await page.locator(".mx_RightPanel").getByRole("menuitem", { name: "People" }).click();
             await expect(page.locator(".mx_MemberList")).toBeVisible();
 
             await getMemberTileByName(page, NAME).click();
@@ -116,7 +116,7 @@ test.describe("RightPanel", () => {
             await page.getByTestId("base-card-back-button").click();
             await expect(page.locator(".mx_MemberList")).toBeVisible();
 
-            await page.locator(".mx_RightPanelTabs").getByText("Info").click();
+            await page.getByLabel("Room info").nth(1).click();
             await checkRoomSummaryCard(page, ROOM_NAME);
         });
     });

diff --git a/playwright/e2e/spaces/threads-activity-centre/index.ts b/playwright/e2e/spaces/threads-activity-centre/index.ts
@@ -328,13 +328,6 @@ export class Helpers {
         return expect(this.page.locator(".mx_ThreadPanel")).toBeVisible();
     }
 
-    /**
-     * Assert that the thread tab is focused
-     */
-    assertThreadTabFocused() {
-        return expect(this.page.locator("#thread-panel-tab")).toBeFocused();
-    }
-
     /**
      * Populate the rooms with messages and threads
      * @param room1

diff --git a/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts b/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts
@@ -157,6 +157,6 @@ test.describe("Threads Activity Centre", () => {
         await util.openTac();
         await util.clickRoomInTac(room1.name);
 
-        await util.assertThreadTabFocused();
+        await util.assertThreadPanelIsOpened();
     });
 });
diff --git a/...ht/snapshots/crypto/crypto.spec.ts/RoomSummaryCard-with-verified-e2ee-linux.png b/...ht/snapshots/crypto/crypto.spec.ts/RoomSummaryCard-with-verified-e2ee-linux.png
diff --git a/...ts/pinned-messages/pinned-messages.spec.ts/pinned-messages-list-empty-linux.png b/...ts/pinned-messages/pinned-messages.spec.ts/pinned-messages-list-empty-linux.png
diff --git a/playwright/snapshots/right-panel/file-panel.spec.ts/empty-linux.png b/playwright/snapshots/right-panel/file-panel.spec.ts/empty-linux.png
diff --git a/playwright/snapshots/right-panel/notification-panel.spec.ts/empty-linux.png b/playwright/snapshots/right-panel/notification-panel.spec.ts/empty-linux.png
diff --git a/...right/snapshots/right-panel/right-panel.spec.ts/with-name-and-address-linux.png b/...right/snapshots/right-panel/right-panel.spec.ts/with-name-and-address-linux.png
diff --git a/playwright/snapshots/user-view/user-view.spec.ts/user-info-linux.png b/playwright/snapshots/user-view/user-view.spec.ts/user-info-linux.png
diff --git a/res/css/_components.pcss b/res/css/_components.pcss
@@ -265,7 +265,6 @@
 @import "./views/right_panel/_EncryptionInfo.pcss";
 @import "./views/right_panel/_ExtensionsCard.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";

diff --git a/res/css/views/right_panel/_ExtensionsCard.pcss b/res/css/views/right_panel/_ExtensionsCard.pcss
@@ -10,21 +10,17 @@ Please see LICENSE files in the repository root for full details.
     --cpd-separator-inset: var(--cpd-space-4x);
     --cpd-separator-spacing: var(--cpd-space-4x);
 
-    .mx_BaseCard_header {
-        /* Hide the line between the header and the body of the card */
-        border-block-end: none;
+    .mx_AutoHideScrollbar {
+        padding: 0 var(--cpd-space-4x);
+        margin-top: var(--cpd-space-3x);
+        box-sizing: border-box;
 
         /* Styling for the "Add extensions" button */
-        button {
+        & > button {
             width: 100%;
         }
     }
 
-    .mx_AutoHideScrollbar {
-        padding: 0 var(--cpd-space-4x);
-        box-sizing: border-box;
-    }
-
     .mx_ExtensionsCard_container {
         text-align: center;
         margin: $spacing-20 var(--cpd-space-4x) 0;

diff --git a/res/css/views/right_panel/_RightPanelTabs.pcss b/res/css/views/right_panel/_RightPanelTabs.pcss
diff --git a/res/css/views/right_panel/_RoomSummaryCard.pcss b/res/css/views/right_panel/_RoomSummaryCard.pcss
@@ -88,10 +88,6 @@ Please see LICENSE files in the repository root for full details.
     margin: var(--cpd-space-4x) 0;
 }
 
-.mx_RoomSummaryCard_header {
-    padding: 24px 12px 15px;
-}
-
 .mx_RoomSummaryCard_search {
     flex-grow: 1;
     min-width: 0;

diff --git a/res/css/views/rooms/_MemberList.pcss b/res/css/views/rooms/_MemberList.pcss
@@ -11,7 +11,6 @@ Please see LICENSE files in the repository root for full details.
     display: flex;
     flex-direction: column;
     min-height: 0;
-    margin-top: 24px;
 
     .mx_Spinner {
         flex: 1 0 auto;
@@ -34,6 +33,7 @@ Please see LICENSE files in the repository root for full details.
 
     .mx_AutoHideScrollbar {
         flex: 1 1 0;
+        margin-top: var(--cpd-space-3x);
     }
 }
 

diff --git a/src/components/structures/FilePanel.tsx b/src/components/structures/FilePanel.tsx
@@ -223,7 +223,11 @@ class FilePanel extends React.Component<IProps, IState> {
     public render(): React.ReactNode {
         if (MatrixClientPeg.safeGet().isGuest()) {
             return (
-                <BaseCard className="mx_FilePanel mx_RoomView_messageListWrapper" onClose={this.props.onClose}>
+                <BaseCard
+                    className="mx_FilePanel mx_RoomView_messageListWrapper"
+                    onClose={this.props.onClose}
+                    header={_t("right_panel|files_button")}
+                >
                     <div className="mx_RoomView_empty">
                         {_t(
                             "file_panel|guest_note",
@@ -241,7 +245,11 @@ class FilePanel extends React.Component<IProps, IState> {
             );
         } else if (this.noRoom) {
             return (
-                <BaseCard className="mx_FilePanel mx_RoomView_messageListWrapper" onClose={this.props.onClose}>
+                <BaseCard
+                    className="mx_FilePanel mx_RoomView_messageListWrapper"
+                    onClose={this.props.onClose}
+                    header={_t("right_panel|files_button")}
+                >
                     <div className="mx_RoomView_empty">{_t("file_panel|peek_note")}</div>
                 </BaseCard>
             );
@@ -273,6 +281,7 @@ class FilePanel extends React.Component<IProps, IState> {
                         onClose={this.props.onClose}
                         withoutScrollContainer
                         ref={this.card}
+                        header={_t("right_panel|files_button")}
                     >
                         {this.card.current && (
                             <Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />
@@ -299,7 +308,11 @@ class FilePanel extends React.Component<IProps, IState> {
                         timelineRenderingType: TimelineRenderingType.File,
                     }}
                 >
-                    <BaseCard className="mx_FilePanel" onClose={this.props.onClose}>
+                    <BaseCard
+                        className="mx_FilePanel"
+                        onClose={this.props.onClose}
+                        header={_t("right_panel|files_button")}
+                    >
                         <Spinner />
                     </BaseCard>
                 </RoomContext.Provider>

diff --git a/src/components/structures/NotificationPanel.tsx b/src/components/structures/NotificationPanel.tsx
@@ -18,7 +18,6 @@ import Spinner from "../views/elements/Spinner";
 import { Layout } from "../../settings/enums/Layout";
 import RoomContext, { TimelineRenderingType } from "../../contexts/RoomContext";
 import Measured from "../views/elements/Measured";
-import Heading from "../views/typography/Heading";
 import EmptyState from "../views/right_panel/EmptyState";
 
 interface IProps {
@@ -88,13 +87,7 @@ export default class NotificationPanel extends React.PureComponent<IProps, IStat
                 }}
             >
                 <BaseCard
-                    header={
-                        <div className="mx_BaseCard_header_title">
-                            <Heading size="4" className="mx_BaseCard_header_title_heading">
-                                {_t("notifications|enable_prompt_toast_title")}
-                            </Heading>
-                        </div>
-                    }
+                    header={_t("notifications|enable_prompt_toast_title")}
                     /**
                      * Need to rename this CSS class to something more generic
                      * Will be done once all the panels are using a similar layout

diff --git a/src/components/structures/RightPanel.tsx b/src/components/structures/RightPanel.tsx
@@ -33,7 +33,6 @@ 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";
 import ExtensionsCard from "../views/right_panel/ExtensionsCard";
 
 interface BaseProps {
@@ -164,7 +163,6 @@ 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}
@@ -313,7 +311,6 @@ export default class RightPanel extends React.Component<Props, IState> {
 
         return (
             <aside className="mx_RightPanel" id="mx_RightPanel">
-                {phase && <RightPanelTabs room={this.props.room} phase={phase} />}
                 {card}
             </aside>
         );

diff --git a/src/components/structures/ThreadPanel.tsx b/src/components/structures/ThreadPanel.tsx
@@ -193,7 +193,6 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
             }}
         >
             <BaseCard
-                hideHeaderButtons
                 header={
                     hasThreads && <ThreadPanelHeader filterOption={filterOption} setFilterOption={setFilterOption} />
                 }

diff --git a/src/components/views/right_panel/BaseCard.tsx b/src/components/views/right_panel/BaseCard.tsx
@@ -38,6 +38,12 @@ interface IProps {
     children: ReactNode;
 }
 
+function closeRightPanel(ev: MouseEvent<HTMLButtonElement>): void {
+    ev.preventDefault();
+    ev.stopPropagation();
+    RightPanelStore.instance.popCard();
+}
+
 const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
     (
         {
@@ -81,12 +87,12 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
         }
 
         let closeButton;
-        if (onClose && !hideHeaderButtons) {
+        if (!hideHeaderButtons) {
             closeButton = (
                 <IconButton
                     size="28px"
                     data-testid="base-card-close-button"
-                    onClick={onClose}
+                    onClick={onClose ?? closeRightPanel}
                     ref={closeButtonRef}
                     tooltip={closeLabel ?? _t("action|close")}
                     subtleBackground
@@ -116,9 +122,16 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
                         <div className="mx_BaseCard_header">
                             {backButton}
                             {typeof header === "string" ? (
-                                <Text size="md" weight="medium" className="mx_BaseCard_header_title">
-                                    {header}
-                                </Text>
+                                <div className="mx_BaseCard_header_title">
+                                    <Text
+                                        size="md"
+                                        weight="medium"
+                                        className="mx_BaseCard_header_title_heading"
+                                        role="heading"
+                                    >
+                                        {header}
+                                    </Text>
+                                </div>
                             ) : (
                                 (header ?? <div className="mx_BaseCard_header_spacer" />)
                             )}

diff --git a/src/components/views/right_panel/ExtensionsCard.tsx b/src/components/views/right_panel/ExtensionsCard.tsx
@@ -157,13 +157,6 @@ const ExtensionsCard: React.FC<Props> = ({ room, onClose }) => {
         }
     };
 
-    // The button is in the header to keep it outside the scrollable region
-    const header = (
-        <Button size="sm" onClick={onManageIntegrations} kind="secondary" Icon={PlusIcon}>
-            {_t("right_panel|add_integrations")}
-        </Button>
-    );
-
     let body: JSX.Element;
     if (realApps.length < 1) {
         body = (
@@ -197,7 +190,10 @@ const ExtensionsCard: React.FC<Props> = ({ room, onClose }) => {
     }
 
     return (
-        <BaseCard header={header} className="mx_ExtensionsCard" onClose={onClose} hideHeaderButtons>
+        <BaseCard header={_t("right_panel|extensions_button")} className="mx_ExtensionsCard" onClose={onClose}>
+            <Button size="sm" onClick={onManageIntegrations} kind="secondary" Icon={PlusIcon}>
+                {_t("right_panel|add_integrations")}
+            </Button>
             {body}
         </BaseCard>
     );

diff --git a/src/components/views/right_panel/PinnedMessagesCard.tsx b/src/components/views/right_panel/PinnedMessagesCard.tsx
@@ -20,7 +20,6 @@ import { PinnedEventTile } from "../rooms/PinnedEventTile";
 import { useRoomState } from "../../../hooks/useRoomState";
 import RoomContext, { TimelineRenderingType, useRoomContext } from "../../../contexts/RoomContext";
 import { ReadPinsEventId } from "./types";
-import Heading from "../typography/Heading";
 import { RoomPermalinkCreator } from "../../../utils/permalinks/Permalinks";
 import { filterBoolean } from "../../../utils/arrays";
 import Modal from "../../../Modal";
@@ -86,13 +85,7 @@ export function PinnedMessagesCard({ room, onClose, permalinkCreator }: PinnedMe
 
     return (
         <BaseCard
-            header={
-                <div className="mx_BaseCard_header_title">
-                    <Heading size="4" className="mx_BaseCard_header_title_heading">
-                        {_t("right_panel|pinned_messages|header", { count: pinnedEventIds.length })}
-                    </Heading>
-                </div>
-            }
+            header={_t("right_panel|pinned_messages|header", { count: pinnedEventIds.length })}
             className="mx_PinnedMessagesCard"
             onClose={onClose}
         >