Fix TAC opening with keyboard (#12285)

* Fix TAC opening with keyboard

* Use compound tooltip and icon button

* Revert "Fix TAC opening with keyboard"

This reverts commit 5a1e5d0c

* Add missing aria-label

* Update tests

* Add tests

* Fix visual regression

* Fix remaining tooltip

* Fix ref typing

* Fix typing

res/css/structures/_ThreadsActivityCentre.pcss

@@ -17,22 +17,31 @@
  */
 
 .mx_ThreadsActivityCentreButton {
-    color: $secondary-content;
-    height: 32px;
-    min-width: 32px;
-    display: flex;
-    align-items: center;
-    justify-content: center;
     border-radius: 8px;
     margin: 18px auto auto auto;
 
     &.expanded {
         /* align with settings icon */
-        margin-left: 25px;
+        margin-left: 21px;
 
-        & > .mx_ThreadsActivityCentreButton_IndicatorIcon {
+        /**
+         * modify internal css of the compound component
+         * dirty but we need to add the `Threads` label into the indicator icon button
+         **/
+        & > div {
+            display: flex;
+            align-items: center;
+        }
+
+        & .mx_ThreadsActivityCentreButton_Icon {
             /* align with settings label */
             margin-right: 14px;
+            /* required to set the icon width when into a flex container */
+            min-width: 24px;
+        }
+
+        & .mx_ThreadsActivityCentreButton_Text {
+            color: $secondary-content;
         }
     }
 

src/components/views/spaces/threads-activity-centre/ThreadsActivityCentreButton.tsx

@@ -16,17 +16,16 @@
  * /
  */
 
-import React, { forwardRef, HTMLProps } from "react";
+import React, { ComponentProps, forwardRef } from "react";
 import { Icon } from "@vector-im/compound-design-tokens/icons/threads-solid.svg";
 import classNames from "classnames";
-import { IndicatorIcon } from "@vector-im/compound-web";
+import { IconButton, Text, Tooltip } from "@vector-im/compound-web";
 
 import { _t } from "../../../../languageHandler";
-import AccessibleTooltipButton from "../../elements/AccessibleTooltipButton";
 import { NotificationLevel } from "../../../../stores/notifications/NotificationLevel";
 import { notificationLevelToIndicator } from "../../../../utils/notifications";
 
-interface ThreadsActivityCentreButtonProps extends HTMLProps<HTMLDivElement> {
+interface ThreadsActivityCentreButtonProps extends ComponentProps<typeof IconButton> {
     /**
      * Display the `Treads` label next to the icon.
      */
@@ -40,28 +39,36 @@ interface ThreadsActivityCentreButtonProps extends HTMLProps<HTMLDivElement> {
 /**
  * A button to open the thread activity centre.
  */
-export const ThreadsActivityCentreButton = forwardRef<HTMLDivElement, ThreadsActivityCentreButtonProps>(
+export const ThreadsActivityCentreButton = forwardRef<HTMLButtonElement, ThreadsActivityCentreButtonProps>(
     function ThreadsActivityCentreButton({ displayLabel, notificationLevel, ...props }, ref): React.JSX.Element {
+        // Disable tooltip when the label is displayed
+        const openTooltip = displayLabel ? false : undefined;
+
         return (
-            <AccessibleTooltipButton
-                className={classNames("mx_ThreadsActivityCentreButton", { expanded: displayLabel })}
-                title={_t("common|threads")}
-                // @ts-ignore
-                // ref nightmare...
-                ref={ref}
-                forceHide={displayLabel}
-                aria-expanded={displayLabel}
-                {...props}
-            >
-                <IndicatorIcon
-                    className="mx_ThreadsActivityCentreButton_IndicatorIcon"
+            <Tooltip label={_t("common|threads")} side="right" open={openTooltip}>
+                <IconButton
+                    aria-label={_t("common|threads")}
+                    className={classNames("mx_ThreadsActivityCentreButton", { expanded: displayLabel })}
                     indicator={notificationLevelToIndicator(notificationLevel)}
-                    size="24px"
+                    {...props}
+                    ref={ref}
                 >
-                    <Icon className="mx_ThreadsActivityCentreButton_Icon" />
-                </IndicatorIcon>
-                {displayLabel && _t("common|threads")}
-            </AccessibleTooltipButton>
+                    <>
+                        <Icon className="mx_ThreadsActivityCentreButton_Icon" />
+                        {/* This is dirty, but we need to add the label to the indicator icon */}
+                        {displayLabel && (
+                            <Text
+                                className="mx_ThreadsActivityCentreButton_Text"
+                                as="span"
+                                size="md"
+                                title={_t("common|threads")}
+                            >
+                                {_t("common|threads")}
+                            </Text>
+                        )}
+                    </>
+                </IconButton>
+            </Tooltip>
         );
     },
 );

test/components/views/spaces/ThreadsActivityCentre-test.tsx

@@ -16,10 +16,11 @@
  * /
  */
 
-import React from "react";
+import React, { ComponentProps } from "react";
 import { getByText, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { NotificationCountType, PendingEventOrdering, Room } from "matrix-js-sdk/src/matrix";
+import { TooltipProvider } from "@vector-im/compound-web";
 
 import { ThreadsActivityCentre } from "../../../../src/components/views/spaces/threads-activity-centre";
 import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
@@ -37,12 +38,16 @@ describe("ThreadsActivityCentre", () => {
         return screen.getByRole("menu");
     };
 
-    const renderTAC = () => {
+    const getTACDescription = () => {
+        return screen.getByText("Threads");
+    };
+
+    const renderTAC = (props?: ComponentProps<typeof ThreadsActivityCentre>) => {
         render(
             <MatrixClientContext.Provider value={cli}>
-                <ThreadsActivityCentre />
-                );
+                <ThreadsActivityCentre {...props} />
             </MatrixClientContext.Provider>,
+            { wrapper: TooltipProvider },
         );
     };
 
@@ -105,6 +110,12 @@ describe("ThreadsActivityCentre", () => {
         expect(getTACButton()).toBeInTheDocument();
     });
 
+    it("should render the threads activity centre button and the display label", async () => {
+        renderTAC({ displayButtonLabel: true });
+        expect(getTACButton()).toBeInTheDocument();
+        expect(getTACDescription()).toBeInTheDocument();
+    });
+
     it("should render the threads activity centre menu when the button is clicked", async () => {
         renderTAC();
         await userEvent.click(getTACButton());

test/components/views/spaces/__snapshots__/ThreadsActivityCentre-test.tsx.snap

@@ -2,7 +2,7 @@
 
 exports[`ThreadsActivityCentre renders notifications matching the snapshot 1`] = `
 <div
-  aria-labelledby="radix-12"
+  aria-labelledby="radix-20"
   aria-orientation="vertical"
   class="_menu_1x5h1_17"
   data-align="end"
@@ -11,7 +11,7 @@ exports[`ThreadsActivityCentre renders notifications matching the snapshot 1`] =
   data-side="right"
   data-state="open"
   dir="ltr"
-  id="radix-13"
+  id="radix-21"
   role="menu"
   style="outline: none; --radix-dropdown-menu-content-transform-origin: var(--radix-popper-transform-origin); --radix-dropdown-menu-content-available-width: var(--radix-popper-available-width); --radix-dropdown-menu-content-available-height: var(--radix-popper-available-height); --radix-dropdown-menu-trigger-width: var(--radix-popper-anchor-width); --radix-dropdown-menu-trigger-height: var(--radix-popper-anchor-height); pointer-events: auto;"
   tabindex="-1"
@@ -127,7 +127,7 @@ exports[`ThreadsActivityCentre renders notifications matching the snapshot 1`] =
 
 exports[`ThreadsActivityCentre should match snapshot when empty 1`] = `
 <div
-  aria-labelledby="radix-18"
+  aria-labelledby="radix-28"
   aria-orientation="vertical"
   class="_menu_1x5h1_17"
   data-align="end"
@@ -136,7 +136,7 @@ exports[`ThreadsActivityCentre should match snapshot when empty 1`] = `
   data-side="right"
   data-state="open"
   dir="ltr"
-  id="radix-19"
+  id="radix-29"
   role="menu"
   style="outline: none; --radix-dropdown-menu-content-transform-origin: var(--radix-popper-transform-origin); --radix-dropdown-menu-content-available-width: var(--radix-popper-available-width); --radix-dropdown-menu-content-available-height: var(--radix-popper-available-height); --radix-dropdown-menu-trigger-width: var(--radix-popper-anchor-width); --radix-dropdown-menu-trigger-height: var(--radix-popper-anchor-height); pointer-events: auto;"
   tabindex="-1"