Use Compound tooltips instead of homegrown in TextWithTooltip & InfoT…

…ooltip (#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>
matrix-org · Dec 19, 2023 · 2212fba · 2212fba
1 parent 5f92dad
commit 2212fba
Show file tree

Hide file tree

Showing 15 changed files with 209 additions and 75 deletions.
diff --git 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

diff --git 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>
         );

diff --git 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>
             );

diff --git 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.

diff --git 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>
         );
     }
 }
diff --git 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>
         );

diff --git 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>
         );
     }
 }
diff --git 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}

diff --git 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")}

diff --git a/test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap b/test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap
@@ -320,22 +320,22 @@ 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
               class="mx_Checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid"
             >
               <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>

diff --git 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();
+    });
+});