Update right panel base card styling to match Compound (#12768)

* Update base card styling to match Compound Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update screenshot Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
2024-11-21 16:55:34 +03:00 · 2024-07-16 10:03:35 +01:00 · 2024-07-16 10:03:35 +01:00 · f7a078d250
commit f7a078d250
parent 5f10ccb5e4
24 changed files with 118 additions and 130 deletions
--- a/package.json
+++ b/package.json
@ -81,7 +81,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.2.3",
+        "@vector-im/compound-web": "^5.4.0",
        "@zxcvbn-ts/core": "^3.0.4",
        "@zxcvbn-ts/language-common": "^3.0.4",
        "@zxcvbn-ts/language-en": "^3.0.2",
--- a/playwright/e2e/crypto/crypto.spec.ts
+++ b/playwright/e2e/crypto/crypto.spec.ts
@ -227,7 +227,7 @@ test.describe("Cryptography", function () {
        await verify(page, bob);

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

--- a/playwright/e2e/crypto/event-shields.spec.ts
+++ b/playwright/e2e/crypto/event-shields.spec.ts
@ -132,7 +132,7 @@ test.describe("Cryptography", function () {
            // wait for the logout to propagate. Workaround for https://github.com/vector-im/element-web/issues/26263 by repeatedly closing and reopening Bob's user info.
            async function awaitOneDevice(iterations = 1) {
                const rightPanel = page.locator(".mx_RightPanel");
-                await rightPanel.getByRole("button", { name: "Room members" }).click();
+                await rightPanel.getByTestId("base-card-back-button").click();
                await rightPanel.getByText("Bob").click();
                const sessionCountText = await rightPanel
                    .locator(".mx_UserInfo_devices")
--- a/playwright/e2e/read-receipts/index.ts
+++ b/playwright/e2e/read-receipts/index.ts
@ -538,7 +538,7 @@ class Helpers {
        const threadPanel = this.page.locator(".mx_ThreadPanel");
        await expect(threadPanel).toBeVisible();
        await threadPanel.evaluate(($panel) => {
-            const $button = $panel.querySelector<HTMLElement>('.mx_BaseCard_back[aria-label="Threads"]');
+            const $button = $panel.querySelector<HTMLElement>('[data-testid="base-card-back-button"]');
            // If the Threads back button is present then click it - the
            // threads button can open either threads list or thread panel
            if ($button) {
--- a/playwright/e2e/right-panel/right-panel.spec.ts
+++ b/playwright/e2e/right-panel/right-panel.spec.ts
@ -106,7 +106,7 @@ test.describe("RightPanel", () => {
            await expect(page.locator(".mx_FilePanel")).toBeVisible();
            await expect(page.locator(".mx_FilePanel_empty")).toBeVisible();

-            await page.getByRole("button", { name: "Room information" }).click();
+            await page.getByTestId("base-card-back-button").click();
            await checkRoomSummaryCard(page, ROOM_NAME);
        });

@ -120,7 +120,7 @@ test.describe("RightPanel", () => {
            await expect(page.locator(".mx_UserInfo")).toBeVisible();
            await expect(page.locator(".mx_UserInfo_profile").getByText(NAME)).toBeVisible();

-            await page.getByRole("button", { name: "Room members" }).click();
+            await page.getByTestId("base-card-back-button").click();
            await expect(page.locator(".mx_MemberList")).toBeVisible();

            await page.locator(".mx_RightPanelTabs").getByText("Info").click();
@ -145,7 +145,7 @@ test.describe("RightPanel", () => {
            await expect(page.locator(".mx_UserInfo_profile").getByText(NAME)).toBeVisible();
            await expect(page.locator(".mx_SpaceScopeHeader").getByText(SPACE_NAME)).toBeVisible();

-            await page.getByRole("button", { name: "Back" }).click();
+            await page.getByTestId("base-card-back-button").click();
            await expect(page.locator(".mx_MemberList")).toBeVisible();
        });
    });
--- a/playwright/e2e/threads/threads.spec.ts
+++ b/playwright/e2e/threads/threads.spec.ts
@ -433,7 +433,7 @@ test.describe("Threads", () => {
        await textbox.press("Enter");
        await expect(locator.locator(".mx_EventTile_last").getByText("Hello Mr. User")).toBeAttached();
        // Close thread
-        await locator.getByRole("button", { name: "Close" }).click();
+        await locator.getByTestId("base-card-close-button").click();

        // Open existing thread
        locator = page
@ -486,7 +486,7 @@ test.describe("Threads", () => {
            await textbox.press("Enter");
            await expect(threadPanel.locator(".mx_EventTile_last").getByText(threadMessage)).toBeVisible();
            // Close thread
-            await threadPanel.getByRole("button", { name: "Close" }).click();
+            await threadPanel.getByTestId("base-card-close-button").click();
        };

        await sendMessage("Hello Mr. Bot");
@ -502,7 +502,7 @@ test.describe("Threads", () => {
        ).toBeVisible();

        // Open threads list
-        await page.locator(".mx_BaseCard_back").click();
+        await page.getByTestId("base-card-back-button").click();
        const rightPanel = page.locator(".mx_RightPanel");
        // Check that the threads are listed
        await expect(rightPanel.locator(".mx_EventTile").getByText("Hello Mr. User in a thread")).toBeVisible();
--- a/playwright/snapshots/polls/polls.spec.ts/ThreadView-with-a-poll-on-bubble-layout-linux.png
+++ b/playwright/snapshots/polls/polls.spec.ts/ThreadView-with-a-poll-on-bubble-layout-linux.png
--- a/playwright/snapshots/polls/polls.spec.ts/ThreadView-with-a-poll-on-group-layout-linux.png
+++ b/playwright/snapshots/polls/polls.spec.ts/ThreadView-with-a-poll-on-group-layout-linux.png
--- a/playwright/snapshots/right-panel/file-panel.spec.ts/empty-linux.png
+++ b/playwright/snapshots/right-panel/file-panel.spec.ts/empty-linux.png
--- a/playwright/snapshots/right-panel/notification-panel.spec.ts/empty-linux.png
+++ b/playwright/snapshots/right-panel/notification-panel.spec.ts/empty-linux.png
--- a/playwright/snapshots/threads/threads.spec.ts/Reply-to-the-location-on-ThreadView-linux.png
+++ b/playwright/snapshots/threads/threads.spec.ts/Reply-to-the-location-on-ThreadView-linux.png
--- 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
--- a/res/css/views/right_panel/_BaseCard.pcss
+++ b/res/css/views/right_panel/_BaseCard.pcss
@ -27,7 +27,7 @@ limitations under the License.

    .mx_BaseCard_header {
        height: 64px;
-        padding: var(--cpd-space-3x);
+        padding: var(--cpd-space-4x);
        box-sizing: border-box;
        /* changing the color from $separator to transparent as it is
           the best visual output during the transition period. This will be
@ -36,8 +36,13 @@ limitations under the License.
        display: flex;
        align-items: center;
        justify-content: space-between;
-        gap: var(--cpd-space-2x);
+        gap: var(--cpd-space-3x);
        flex-shrink: 0;
+        border-block-end: var(--cpd-border-width-1) solid $separator;
+
+        .mx_BaseCard_header_spacer {
+            flex: 1;
+        }

        > h2 {
            margin: 0 44px;
@ -155,52 +160,6 @@ limitations under the License.
    }
 }

-.mx_BaseCard_back,
-.mx_BaseCard_close {
-    flex-shrink: 0;
-    position: relative;
-    /* @TODO(kerrya) background colours here are not semantic
-       these buttons to be replaced with IconButton after secondary variant is added
-       https://github.com/vector-im/compound/issues/279 */
-    background-color: var(--cpd-color-bg-subtle-secondary);
-    width: var(--BaseCard_header-button-size);
-    height: var(--BaseCard_header-button-size);
-    border-radius: 50%;
-
-    &:hover {
-        background-color: var(--cpd-color-bg-subtle-primary);
-    }
-
-    &::before {
-        content: "";
-        position: absolute;
-        height: inherit;
-        width: inherit;
-        top: 0;
-        left: 0;
-        mask-repeat: no-repeat;
-        mask-position: center;
-        mask-size: 20px;
-        background-color: var(--cpd-color-icon-secondary);
-    }
-}
-
-.mx_BaseCard_back {
-    order: 0; /* always first! */
-    &::before {
-        transform: rotate(90deg);
-        mask-size: 22px;
-        mask-image: url("$(res)/img/feather-customised/chevron-down.svg");
-    }
-}
-
-.mx_BaseCard_close {
-    order: 999; /* always last */
-    &::before {
-        mask-image: url("@vector-im/compound-design-tokens/icons/close.svg");
-    }
-}
-
 .mx_ContextualMenu_wrapper.mx_BaseCard_header_title {
    .mx_ContextualMenu {
        position: initial;
@ -235,7 +194,3 @@ limitations under the License.
        }
    }
 }
-
-.mx_BaseCard_headerProp {
-    flex: 1 1 100%;
-}
--- a/res/themes/dark/css/_dark.pcss
+++ b/res/themes/dark/css/_dark.pcss
@ -14,7 +14,7 @@ $overlay-background: var(--cpd-color-alpha-gray-1300);
 $panels: var(--cpd-color-bg-subtle-secondary);
 $panel-actions: var(--cpd-color-alpha-gray-300);

-$separator: var(--cpd-color-alpha-gray-400);
+$separator: var(--cpd-color-gray-400);

 /* ******************** */

--- a/res/themes/legacy-dark/css/_legacy-dark.pcss
+++ b/res/themes/legacy-dark/css/_legacy-dark.pcss
@ -105,7 +105,7 @@ $overlay-background: rgba($background, 0.85);
 $panels: rgba($system, 0.9);
 $panel-actions: $roomtile-selected-bg-color;

-$separator: var(--cpd-color-alpha-gray-400);
+$separator: var(--cpd-color-gray-400);

 /**
 * Creating a `semantic` color scale. This will not be needed with the new
--- a/res/themes/legacy-light/css/_legacy-light.pcss
+++ b/res/themes/legacy-light/css/_legacy-light.pcss
@ -163,7 +163,7 @@ $overlay-background: rgba($background, 0.85);
 $panels: rgba($system, 0.9);
 $panel-actions: $roomtile-selected-bg-color;

-$separator: var(--cpd-color-alpha-gray-400);
+$separator: var(--cpd-color-gray-400);

 /* Legacy theme backports */

--- a/res/themes/light/css/_light.pcss
+++ b/res/themes/light/css/_light.pcss
@ -32,7 +32,7 @@ $overlay-background: var(--cpd-color-alpha-gray-1300);
 $panels: var(--cpd-color-bg-subtle-secondary);
 $panel-actions: var(--cpd-color-alpha-gray-300);

-$separator: var(--cpd-color-alpha-gray-400);
+$separator: var(--cpd-color-gray-400);

 $accent: var(--cpd-color-text-action-accent);
 $alert: var(--cpd-color-text-critical-primary);
--- a/src/components/structures/ThreadPanel.tsx
+++ b/src/components/structures/ThreadPanel.tsx
@ -229,7 +229,7 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
    const roomContext = useContext(RoomContext);
    const timelinePanel = useRef<TimelinePanel | null>(null);
    const card = useRef<HTMLDivElement | null>(null);
-    const closeButonRef = useRef<HTMLDivElement | null>(null);
+    const closeButonRef = useRef<HTMLButtonElement | null>(null);

    const [filterOption, setFilterOption] = useState<ThreadFilterType>(ThreadFilterType.All);
    const [room, setRoom] = useState<Room | null>(null);
--- a/src/components/views/right_panel/BaseCard.tsx
+++ b/src/components/views/right_panel/BaseCard.tsx
@ -14,12 +14,14 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */

-import React, { forwardRef, ReactNode, KeyboardEvent, Ref } from "react";
+import React, { forwardRef, ReactNode, KeyboardEvent, Ref, MouseEvent } from "react";
 import classNames from "classnames";
+import { IconButton, Text } from "@vector-im/compound-web";
+import { Icon as CloseIcon } from "@vector-im/compound-design-tokens/icons/close.svg";
+import { Icon as ChevronLeftIcon } from "@vector-im/compound-design-tokens/icons/chevron-left.svg";

 import AutoHideScrollbar from "../../structures/AutoHideScrollbar";
 import { _t } from "../../../languageHandler";
-import AccessibleButton, { ButtonEvent } from "../elements/AccessibleButton";
 import RightPanelStore from "../../../stores/right-panel/RightPanelStore";
 import { backLabelForPhase } from "../../../stores/right-panel/RightPanelStorePhases";
 import { CardContext } from "./context";
@ -34,13 +36,13 @@ interface IProps {
    ariaLabelledBy?: string;
    withoutScrollContainer?: boolean;
    closeLabel?: string;
-    onClose?(ev: ButtonEvent): void;
-    onBack?(ev: ButtonEvent): void;
+    onClose?(ev: MouseEvent<HTMLButtonElement>): void;
+    onBack?(ev: MouseEvent<HTMLButtonElement>): void;
    onKeyDown?(ev: KeyboardEvent): void;
    cardState?: any;
    ref?: Ref<HTMLDivElement>;
    // Ref for the 'close' button the the card
-    closeButtonRef?: Ref<HTMLDivElement>;
+    closeButtonRef?: Ref<HTMLButtonElement>;
    children: ReactNode;
 }

@ -81,26 +83,39 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
    ) => {
        let backButton;
        const cardHistory = RightPanelStore.instance.roomPhaseHistory;
-        if (cardHistory.length > 1) {
+        if (cardHistory.length > 1 && !hideHeaderButtons) {
            const prevCard = cardHistory[cardHistory.length - 2];
-            const onBackClick = (ev: ButtonEvent): void => {
+            const onBackClick = (ev: MouseEvent<HTMLButtonElement>): void => {
                onBack?.(ev);
                RightPanelStore.instance.popCard();
            };
            const label = backLabelForPhase(prevCard.phase) ?? _t("action|back");
-            backButton = <AccessibleButton className="mx_BaseCard_back" onClick={onBackClick} title={label} />;
+            backButton = (
+                <IconButton
+                    size="28px"
+                    data-testid="base-card-back-button"
+                    onClick={onBackClick}
+                    tooltip={label}
+                    subtleBackground
+                >
+                    <ChevronLeftIcon />
+                </IconButton>
+            );
        }

        let closeButton;
-        if (onClose) {
+        if (onClose && !hideHeaderButtons) {
            closeButton = (
-                <AccessibleButton
+                <IconButton
+                    size="28px"
                    data-testid="base-card-close-button"
-                    className="mx_BaseCard_close"
                    onClick={onClose}
-                    title={closeLabel || _t("action|close")}
                    ref={closeButtonRef}
-                />
+                    tooltip={closeLabel ?? _t("action|close")}
+                    subtleBackground
+                >
+                    <CloseIcon />
+                </IconButton>
            );
        }

@ -108,16 +123,6 @@ 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 (
@ -132,8 +137,15 @@ const BaseCard: React.FC<IProps> = forwardRef<HTMLDivElement, IProps>(
                >
                    {shouldRenderHeader && (
                        <div className="mx_BaseCard_header">
-                            {headerButtons}
-                            <div className="mx_BaseCard_headerProp">{header}</div>
+                            {backButton}
+                            {typeof header === "string" ? (
+                                <Text size="md" weight="medium" className="mx_BaseCard_header_title">
+                                    {header}
+                                </Text>
+                            ) : (
+                                header ?? <div className="mx_BaseCard_header_spacer" />
+                            )}
+                            {closeButton}
                        </div>
                    )}
                    {children}
--- a/src/components/views/right_panel/UserInfo.tsx
+++ b/src/components/views/right_panel/UserInfo.tsx
@ -1778,7 +1778,7 @@ const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPha
    return (
        <BaseCard
            className={classes.join(" ")}
-            header={createSpaceScopeHeader(room)}
+            header={createSpaceScopeHeader(room) ?? _t("common|profile")}
            onClose={onClose}
            closeLabel={closeLabel}
            cardState={cardState}
--- a/test/components/views/elements/snapshots/AppTile-test.tsx.snap
+++ b/test/components/views/elements/snapshots/AppTile-test.tsx.snap
@ -13,33 +13,36 @@ exports[`AppTile destroys non-persisted right panel widget on room change 1`] =
        class="mx_BaseCard_header"
      >
        <div
-          aria-label="Close"
-          class="mx_AccessibleButton mx_BaseCard_close"
+          class="mx_BaseCard_header_title"
+        >
+          <h4
+            class="mx_Heading_h4 mx_BaseCard_header_title_heading"
+          >
+            Example 1
+          </h4>
+          <div
+            aria-expanded="false"
+            aria-haspopup="true"
+            aria-label="Options"
+            class="mx_AccessibleButton mx_BaseCard_header_title_button--option"
+            role="button"
+            tabindex="0"
+          />
+        </div>
+        <button
+          class="_icon-button_bh2qc_17 _subtle-bg_bh2qc_38"
          data-testid="base-card-close-button"
          role="button"
+          style="--cpd-icon-button-size: 28px;"
          tabindex="0"
-        />
-        <div
-          class="mx_BaseCard_headerProp"
        >
          <div
-            class="mx_BaseCard_header_title"
+            class="_indicator-icon_133tf_26"
+            style="--cpd-icon-button-size: 100%;"
          >
-            <h4
-              class="mx_Heading_h4 mx_BaseCard_header_title_heading"
-            >
-              Example 1
-            </h4>
-            <div
-              aria-expanded="false"
-              aria-haspopup="true"
-              aria-label="Options"
-              class="mx_AccessibleButton mx_BaseCard_header_title_button--option"
-              role="button"
-              tabindex="0"
-            />
+            <div />
          </div>
-        </div>
+        </button>
      </div>
      <div
        class="mx_AppTileFullWidth"
--- a/test/components/views/right_panel/snapshots/UserInfo-test.tsx.snap
+++ b/test/components/views/right_panel/snapshots/UserInfo-test.tsx.snap
@ -77,16 +77,25 @@ exports[`<UserInfo /> with crypto enabled renders <BasicUserInfo /> 1`] = `
    <div
      class="mx_BaseCard_header"
    >
-      <div
-        aria-label="Close"
-        class="mx_AccessibleButton mx_BaseCard_close"
+      <p
+        class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 mx_BaseCard_header_title"
+      >
+        Profile
+      </p>
+      <button
+        class="_icon-button_bh2qc_17 _subtle-bg_bh2qc_38"
        data-testid="base-card-close-button"
        role="button"
+        style="--cpd-icon-button-size: 28px;"
        tabindex="0"
-      />
-      <div
-        class="mx_BaseCard_headerProp"
-      />
+      >
+        <div
+          class="_indicator-icon_133tf_26"
+          style="--cpd-icon-button-size: 100%;"
+        >
+          <div />
+        </div>
+      </button>
    </div>
    <div
      class="mx_AutoHideScrollbar"
@ -232,16 +241,25 @@ exports[`<UserInfo /> with crypto enabled should render a deactivate button for
    <div
      class="mx_BaseCard_header"
    >
-      <div
-        aria-label="Close"
-        class="mx_AccessibleButton mx_BaseCard_close"
+      <p
+        class="_typography_yh5dq_162 _font-body-md-medium_yh5dq_69 mx_BaseCard_header_title"
+      >
+        Profile
+      </p>
+      <button
+        class="_icon-button_bh2qc_17 _subtle-bg_bh2qc_38"
        data-testid="base-card-close-button"
        role="button"
+        style="--cpd-icon-button-size: 28px;"
        tabindex="0"
-      />
-      <div
-        class="mx_BaseCard_headerProp"
-      />
+      >
+        <div
+          class="_indicator-icon_133tf_26"
+          style="--cpd-icon-button-size: 100%;"
+        >
+          <div />
+        </div>
+      </button>
    </div>
    <div
      class="mx_AutoHideScrollbar"
--- a/test/components/views/rooms/snapshots/ThirdPartyMemberInfo-test.tsx.snap
+++ b/test/components/views/rooms/snapshots/ThirdPartyMemberInfo-test.tsx.snap
@ -9,7 +9,7 @@ exports[`<ThirdPartyMemberInfo /> should render invite 1`] = `
      class="mx_BaseCard_header"
    >
      <div
-        class="mx_BaseCard_headerProp"
+        class="mx_BaseCard_header_spacer"
      />
    </div>
    <div
@ -51,7 +51,7 @@ exports[`<ThirdPartyMemberInfo /> should render invite when room in not availabl
      class="mx_BaseCard_header"
    >
      <div
-        class="mx_BaseCard_headerProp"
+        class="mx_BaseCard_header_spacer"
      />
    </div>
    <div
--- a/yarn.lock
+++ b/yarn.lock
@ -2977,7 +2977,7 @@
  dependencies:
    svg2vectordrawable "^2.9.1"

-"@vector-im/compound-web@^5.2.3":
+"@vector-im/compound-web@^5.4.0":
  version "5.4.0"
  resolved "https://registry.yarnpkg.com/@vector-im/compound-web/-/compound-web-5.4.0.tgz#b95262197199c11931a8c6f5269514eb9461f187"
  integrity sha512-+EPbr8HzlGEWSePEcPs2iQEBnjXvHGWK177SKF8IO2C7Z2Ygddxa2VTQ7oqtrUfgT+NB5IBTLyXV4Nx7FLgmMA==