Use Compound tooltips instead of homegrown in TextWithTooltip & InfoTooltip (#12052)

* Migrate InfoTooltip to use Compound Tooltip Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Migrate DecoratedRoomAvatar.tsx to Compound tooltips Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Small type tweaks Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Remove unused props Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Migrate TextWithTooltip.tsx to Compound tooltips This adds `contain: strict` to #matrixchat which may have side effects. Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update snapshots Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add comment Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update snapshot Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Revert accidental layout change to TextWithTooltip from inline to block Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve test coverage and simplify Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update snapshots Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Ditch the sleep call Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
2023-12-19 17:19:54 +00:00 · 2023-12-19 17:19:54 +00:00 · 2212fbadd0
parent 5f92dad273
commit 2212fbadd0
15 changed files with 209 additions and 75 deletions
--- a/res/css/_common.pcss
+++ b/res/css/_common.pcss
@ -52,6 +52,11 @@ limitations under the License.
    --dialog-zIndex-standard: calc(var(--dialog-zIndex-standard-background) + 1); /* 4012 */
 }

+#matrixchat {
+    /* This is required to ensure Compound tooltips correctly draw where they should with z-index: auto */
+    contain: strict;
+}
+
 /**
 * We need to increase the specificity of the selector to override the
 * custom property set by the design tokens package
--- a/src/components/views/avatars/DecoratedRoomAvatar.tsx
+++ b/src/components/views/avatars/DecoratedRoomAvatar.tsx
@ -16,8 +16,9 @@ limitations under the License.

 import React from "react";
 import classNames from "classnames";
-import { Room, RoomEvent, MatrixEvent, User, UserEvent, EventType, JoinRule } from "matrix-js-sdk/src/matrix";
+import { EventType, JoinRule, MatrixEvent, Room, RoomEvent, User, UserEvent } from "matrix-js-sdk/src/matrix";
 import { UnstableValue } from "matrix-js-sdk/src/NamespacedValue";
+import { Tooltip } from "@vector-im/compound-web";

 import RoomAvatar from "./RoomAvatar";
 import NotificationBadge from "../rooms/NotificationBadge";
@ -26,10 +27,8 @@ import { NotificationState } from "../../../stores/notifications/NotificationSta
 import { isPresenceEnabled } from "../../../utils/presence";
 import { MatrixClientPeg } from "../../../MatrixClientPeg";
 import { _t } from "../../../languageHandler";
-import TextWithTooltip from "../elements/TextWithTooltip";
 import DMRoomMap from "../../../utils/DMRoomMap";
 import { IOOBData } from "../../../stores/ThreepidInviteStore";
-import TooltipTarget from "../elements/TooltipTarget";

 interface IProps {
    room: Room;
@ -38,7 +37,9 @@ interface IProps {
    forceCount?: boolean;
    oobData?: IOOBData;
    viewAvatarOnClick?: boolean;
-    tooltipProps?: Omit<React.ComponentProps<typeof TooltipTarget>, "label" | "tooltipClassName" | "className">;
+    tooltipProps?: {
+        tabIndex?: number;
+    };
 }

 interface IState {
@ -94,9 +95,7 @@ export default class DecoratedRoomAvatar extends React.PureComponent<IProps, ISt
    }

    private get isPublicRoom(): boolean {
-        const joinRules = this.props.room.currentState.getStateEvents(EventType.RoomJoinRules, "");
-        const joinRule = joinRules && joinRules.getContent().join_rule;
-        return joinRule === JoinRule.Public;
+        return this.props.room.getJoinRule() === JoinRule.Public;
    }

    private get dmUser(): User | null {
@ -191,10 +190,9 @@ export default class DecoratedRoomAvatar extends React.PureComponent<IProps, ISt
        let icon: JSX.Element | undefined;
        if (this.state.icon !== Icon.None) {
            icon = (
-                <TextWithTooltip
-                    tooltip={tooltipText(this.state.icon)}
-                    tooltipProps={this.props.tooltipProps}
-                    class={`mx_DecoratedRoomAvatar_icon mx_DecoratedRoomAvatar_icon_${this.state.icon.toLowerCase()}`}
+                <div
+                    tabIndex={this.props.tooltipProps?.tabIndex ?? 0}
+                    className={`mx_DecoratedRoomAvatar_icon mx_DecoratedRoomAvatar_icon_${this.state.icon.toLowerCase()}`}
                />
            );
        }
@ -211,7 +209,7 @@ export default class DecoratedRoomAvatar extends React.PureComponent<IProps, ISt
                    oobData={this.props.oobData}
                    viewAvatarOnClick={this.props.viewAvatarOnClick}
                />
-                {icon}
+                {icon && <Tooltip label={tooltipText(this.state.icon)!}>{icon}</Tooltip>}
                {badge}
            </div>
        );
--- a/src/components/views/dialogs/ServerPickerDialog.tsx
+++ b/src/components/views/dialogs/ServerPickerDialog.tsx
@ -182,7 +182,7 @@ export default class ServerPickerDialog extends React.PureComponent<IProps, ISta
        let defaultServerName: React.ReactNode = this.defaultServer.hsName;
        if (this.defaultServer.hsNameIsDifferent) {
            defaultServerName = (
-                <TextWithTooltip class="mx_Login_underlinedServerName" tooltip={this.defaultServer.hsUrl}>
+                <TextWithTooltip className="mx_Login_underlinedServerName" tooltip={this.defaultServer.hsUrl}>
                    {this.defaultServer.hsName}
                </TextWithTooltip>
            );
--- a/src/components/views/elements/AppPermission.tsx
+++ b/src/components/views/elements/AppPermission.tsx
@ -27,9 +27,9 @@ import MemberAvatar from "../avatars/MemberAvatar";
 import BaseAvatar from "../avatars/BaseAvatar";
 import Heading from "../typography/Heading";
 import AccessibleButton from "./AccessibleButton";
-import TextWithTooltip from "./TextWithTooltip";
 import { parseUrl } from "../../../utils/UrlUtils";
 import { Icon as HelpIcon } from "../../../../res/img/feather-customised/help-circle.svg";
+import TooltipTarget from "./TooltipTarget";

 interface IProps {
    url: string;
@ -116,13 +116,14 @@ export default class AppPermission extends React.Component<IProps, IState> {
            </div>
        );
        const warningTooltip = (
-            <TextWithTooltip
-                tooltip={warningTooltipText}
-                tooltipClass="mx_Tooltip--appPermission mx_Tooltip--appPermission--dark"
-                class="mx_TextWithTooltip_target--helpIcon"
+            <TooltipTarget
+                label={warningTooltipText}
+                tooltipClassName="mx_Tooltip--appPermission mx_Tooltip--appPermission--dark"
+                tooltipTargetClassName="mx_TextWithTooltip_target mx_TextWithTooltip_target--helpIcon"
+                className="mx_TextWithTooltip_tooltip"
            >
                <HelpIcon className="mx_Icon mx_Icon_12" />
-            </TextWithTooltip>
+            </TooltipTarget>
        );

        // Due to i18n limitations, we can't dedupe the code for variables in these two messages.
--- a/src/components/views/elements/InfoTooltip.tsx
+++ b/src/components/views/elements/InfoTooltip.tsx
@ -17,47 +17,38 @@ limitations under the License.

 import React, { ReactNode } from "react";
 import classNames from "classnames";
+import { Tooltip } from "@vector-im/compound-web";

-import { Alignment } from "./Tooltip";
 import { _t } from "../../../languageHandler";
-import TooltipTarget from "./TooltipTarget";

 export enum InfoTooltipKind {
    Info = "info",
    Warning = "warning",
 }

-interface ITooltipProps {
-    tooltip?: React.ReactNode;
+interface TooltipProps {
+    tooltip?: string;
    className?: string;
-    tooltipClassName?: string;
    kind?: InfoTooltipKind;
    children?: ReactNode;
+    tabIndex?: number;
 }

-export default class InfoTooltip extends React.PureComponent<ITooltipProps> {
-    public constructor(props: ITooltipProps) {
-        super(props);
-    }
-
+export default class InfoTooltip extends React.PureComponent<TooltipProps> {
    public render(): React.ReactNode {
-        const { tooltip, children, tooltipClassName, className, kind } = this.props;
+        const { tooltip, children, className, kind } = this.props;
        const title = _t("info_tooltip_title");
        const iconClassName =
            kind !== InfoTooltipKind.Warning ? "mx_InfoTooltip_icon_info" : "mx_InfoTooltip_icon_warning";

        // Tooltip are forced on the right for a more natural feel to them on info icons
        return (
-            <TooltipTarget
-                tooltipTargetClassName={classNames("mx_InfoTooltip", className)}
-                className="mx_InfoTooltip_container"
-                tooltipClassName={classNames("mx_InfoTooltip_tooltip", tooltipClassName)}
-                label={tooltip || title}
-                alignment={Alignment.Right}
-            >
-                <span className={classNames("mx_InfoTooltip_icon", iconClassName)} aria-label={title} />
-                {children}
-            </TooltipTarget>
+            <Tooltip label={tooltip || title} side="right">
+                <div className={classNames("mx_InfoTooltip", className)} tabIndex={this.props.tabIndex ?? 0}>
+                    <span className={classNames("mx_InfoTooltip_icon", iconClassName)} aria-label={title} />
+                    {children}
+                </div>
+            </Tooltip>
        );
    }
 }
--- a/src/components/views/elements/ServerPicker.tsx
+++ b/src/components/views/elements/ServerPicker.tsx
@ -77,7 +77,7 @@ const ServerPicker: React.FC<IProps> = ({ title, dialogTitle, serverConfig, onSe
    let serverName: React.ReactNode = serverConfig.isNameResolvable ? serverConfig.hsName : serverConfig.hsUrl;
    if (serverConfig.hsNameIsDifferent) {
        serverName = (
-            <TextWithTooltip class="mx_Login_underlinedServerName" tooltip={serverConfig.hsUrl}>
+            <TextWithTooltip className="mx_Login_underlinedServerName" tooltip={serverConfig.hsUrl}>
                {serverConfig.hsName}
            </TextWithTooltip>
        );
--- a/src/components/views/elements/TextWithTooltip.tsx
+++ b/src/components/views/elements/TextWithTooltip.tsx
@ -15,16 +15,13 @@
 */

 import React, { HTMLAttributes } from "react";
-import classNames from "classnames";
-
-import TooltipTarget from "./TooltipTarget";
+import { Tooltip } from "@vector-im/compound-web";

 interface IProps extends HTMLAttributes<HTMLSpanElement> {
-    class?: string;
-    tooltipClass?: string;
-    tooltip: React.ReactNode;
-    tooltipProps?: Omit<React.ComponentProps<typeof TooltipTarget>, "label" | "tooltipClassName" | "className">;
-    onClick?: (ev: React.MouseEvent) => void;
+    tooltip: string;
+    tooltipProps?: {
+        tabIndex?: number;
+    };
 }

 export default class TextWithTooltip extends React.Component<IProps> {
@ -33,20 +30,14 @@ export default class TextWithTooltip extends React.Component<IProps> {
    }

    public render(): React.ReactNode {
-        const { class: className, children, tooltip, tooltipClass, tooltipProps, ...props } = this.props;
+        const { className, children, tooltip, tooltipProps } = this.props;

        return (
-            <TooltipTarget
-                onClick={this.props.onClick}
-                tooltipTargetClassName={classNames("mx_TextWithTooltip_target", className)}
-                {...tooltipProps}
-                label={tooltip}
-                tooltipClassName={tooltipClass}
-                className="mx_TextWithTooltip_tooltip"
-                {...props}
-            >
-                {children}
-            </TooltipTarget>
+            <Tooltip label={tooltip} side="right">
+                <span className={className} tabIndex={tooltipProps?.tabIndex ?? 0}>
+                    {children}
+                </span>
+            </Tooltip>
        );
    }
 }
--- a/src/components/views/rooms/RoomList.tsx
+++ b/src/components/views/rooms/RoomList.tsx
@ -170,7 +170,6 @@ const DmAuxButton: React.FC<IAuxButtonProps> = ({ tabIndex, dispatcher = default
                    tabIndex={tabIndex}
                    onClick={openMenu}
                    className="mx_RoomSublist_auxButton"
-                    tooltipClassName="mx_RoomSublist_addRoomTooltip"
                    aria-label={_t("action|add_people")}
                    title={_t("action|add_people")}
                    isExpanded={menuDisplayed}
@ -189,7 +188,6 @@ const DmAuxButton: React.FC<IAuxButtonProps> = ({ tabIndex, dispatcher = default
                    PosthogTrackers.trackInteraction("WebRoomListRoomsSublistPlusMenuCreateChatItem", e);
                }}
                className="mx_RoomSublist_auxButton"
-                tooltipClassName="mx_RoomSublist_addRoomTooltip"
                aria-label={_t("action|start_chat")}
                title={_t("action|start_chat")}
            />
@ -355,7 +353,6 @@ const UntaggedAuxButton: React.FC<IAuxButtonProps> = ({ tabIndex }) => {
                    tabIndex={tabIndex}
                    onClick={openMenu}
                    className="mx_RoomSublist_auxButton"
-                    tooltipClassName="mx_RoomSublist_addRoomTooltip"
                    aria-label={_t("room_list|add_room_label")}
                    title={_t("room_list|add_room_label")}
                    isExpanded={menuDisplayed}
--- a/src/toasts/IncomingCallToast.tsx
+++ b/src/toasts/IncomingCallToast.tsx
@ -58,7 +58,7 @@ function JoinCallButtonWithCall({ onClick, call }: JoinCallButtonWithCallProps):
            className="mx_IncomingCallToast_joinButton"
            onClick={onClick}
            disabled={disabledTooltip !== null}
-            tooltip={disabledTooltip}
+            tooltip={disabledTooltip ?? undefined}
            kind="primary"
        >
            {_t("action|join")}
--- a/test/components/structures/snapshots/SpaceHierarchy-test.tsx.snap
+++ b/test/components/structures/snapshots/SpaceHierarchy-test.tsx.snap
@ -320,9 +320,9 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
          >
            Join
          </div>
-          <div
-            aria-describedby="mx_TooltipTarget_EetmBG4y"
-            class="mx_TextWithTooltip_target"
+          <span
+            class=""
+            data-state="closed"
            tabindex="0"
          >
            <span
@ -330,12 +330,12 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
            >
              <input
                disabled=""
-                id="checkbox_VCeEefiPqp"
+                id="checkbox_EetmBG4yVC"
                tabindex="-1"
                type="checkbox"
              />
              <label
-                for="checkbox_VCeEefiPqp"
+                for="checkbox_EetmBG4yVC"
              >
                <div
                  class="mx_Checkbox_background"
@ -346,7 +346,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
                </div>
              </label>
            </span>
-          </div>
+          </span>
        </div>
      </div>
    </li>
--- a/test/components/views/avatars/DecoratedRoomAvatar-test.tsx
+++ b/test/components/views/avatars/DecoratedRoomAvatar-test.tsx
@ -0,0 +1,66 @@
+/*
+Copyright 2023 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 { render, waitFor } from "@testing-library/react";
+import { mocked } from "jest-mock";
+import { JoinRule, MatrixClient, PendingEventOrdering, Room } from "matrix-js-sdk/src/matrix";
+import React from "react";
+import userEvent from "@testing-library/user-event";
+
+import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
+import { stubClient } from "../../../test-utils";
+import DecoratedRoomAvatar from "../../../../src/components/views/avatars/DecoratedRoomAvatar";
+import DMRoomMap from "../../../../src/utils/DMRoomMap";
+
+describe("DecoratedRoomAvatar", () => {
+    const ROOM_ID = "roomId";
+
+    let mockClient: MatrixClient;
+    let room: Room;
+
+    beforeEach(() => {
+        stubClient();
+        mockClient = mocked(MatrixClientPeg.safeGet());
+
+        room = new Room(ROOM_ID, mockClient, mockClient.getUserId() ?? "", {
+            pendingEventOrdering: PendingEventOrdering.Detached,
+        });
+
+        const dmRoomMap = {
+            getUserIdForRoomId: jest.fn(),
+        } as unknown as DMRoomMap;
+        jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap);
+    });
+
+    it("shows an avatar with globe icon and tooltip for public room", async () => {
+        room.getJoinRule = jest.fn().mockReturnValue(JoinRule.Public);
+        const { container, asFragment } = render(<DecoratedRoomAvatar room={room} size="32px" />);
+
+        const globe = container.querySelector(".mx_DecoratedRoomAvatar_icon_globe")!;
+        expect(globe).toBeVisible();
+        await userEvent.hover(globe!);
+
+        // wait for the tooltip to open
+        const tooltip = await waitFor(() => {
+            const tooltip = document.getElementById(globe.getAttribute("aria-describedby")!);
+            expect(tooltip).toBeVisible();
+            return tooltip;
+        });
+        expect(tooltip).toHaveTextContent("This room is public");
+
+        expect(asFragment()).toMatchSnapshot();
+    });
+});
--- a/test/components/views/avatars/snapshots/DecoratedRoomAvatar-test.tsx.snap
+++ b/test/components/views/avatars/snapshots/DecoratedRoomAvatar-test.tsx.snap
@ -0,0 +1,26 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`DecoratedRoomAvatar shows an avatar with globe icon and tooltip for public room 1`] = `
+<DocumentFragment>
+  <div
+    class="mx_DecoratedRoomAvatar mx_DecoratedRoomAvatar_cutout"
+  >
+    <span
+      class="_avatar_1o69u_17 mx_BaseAvatar _avatar-imageless_1o69u_60"
+      data-color="3"
+      data-testid="avatar-img"
+      data-type="round"
+      role="presentation"
+      style="--cpd-avatar-size: 32px;"
+    >
+      r
+    </span>
+    <div
+      aria-describedby="radix-0"
+      class="mx_DecoratedRoomAvatar_icon mx_DecoratedRoomAvatar_icon_globe"
+      data-state="delayed-open"
+      tabindex="0"
+    />
+  </div>
+</DocumentFragment>
+`;
--- a/test/components/views/dialogs/snapshots/ServerPickerDialog-test.tsx.snap
+++ b/test/components/views/dialogs/snapshots/ServerPickerDialog-test.tsx.snap
@ -55,13 +55,13 @@ exports[`<ServerPickerDialog /> should render dialog 1`] = `
        <div
          class="mx_StyledRadioButton_content"
        >
-          <div
-            aria-describedby="mx_TooltipTarget_vY7Q4uEh"
-            class="mx_TextWithTooltip_target mx_Login_underlinedServerName"
+          <span
+            class="mx_Login_underlinedServerName"
+            data-state="closed"
            tabindex="0"
          >
            matrix.org
-          </div>
+          </span>
        </div>
        <div
          class="mx_StyledRadioButton_spacer"
--- a/test/components/views/elements/InfoTooltip-test.tsx
+++ b/test/components/views/elements/InfoTooltip-test.tsx
@ -0,0 +1,41 @@
+/*
+Copyright 2023 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 userEvent from "@testing-library/user-event";
+import { render, waitFor } from "@testing-library/react";
+
+import InfoTooltip from "../../../../src/components/views/elements/InfoTooltip";
+
+describe("InfoTooltip", () => {
+    it("should show tooltip on hover", async () => {
+        const { getByText, asFragment } = render(<InfoTooltip tooltip="Tooltip text">Trigger text</InfoTooltip>);
+
+        const trigger = getByText("Trigger text");
+        expect(trigger).toBeVisible();
+        await userEvent.hover(trigger!);
+
+        // wait for the tooltip to open
+        const tooltip = await waitFor(() => {
+            const tooltip = document.getElementById(trigger.getAttribute("aria-describedby")!);
+            expect(tooltip).toBeVisible();
+            return tooltip;
+        });
+        expect(tooltip).toHaveTextContent("Tooltip text");
+
+        expect(asFragment()).toMatchSnapshot();
+    });
+});
--- a/test/components/views/elements/snapshots/InfoTooltip-test.tsx.snap
+++ b/test/components/views/elements/snapshots/InfoTooltip-test.tsx.snap
@ -0,0 +1,18 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`InfoTooltip should show tooltip on hover 1`] = `
+<DocumentFragment>
+  <div
+    aria-describedby="radix-0"
+    class="mx_InfoTooltip"
+    data-state="delayed-open"
+    tabindex="0"
+  >
+    <span
+      aria-label="Information"
+      class="mx_InfoTooltip_icon mx_InfoTooltip_icon_info"
+    />
+    Trigger text
+  </div>
+</DocumentFragment>
+`;