Add Tooltip to AccessibleButton (#12443)

* Deprecate AccessibleTooltipButton Signed-off-by: Michael Telatynski <[email protected]> * Fix tests Signed-off-by: Michael Telatynski <[email protected]> * Fix tests Signed-off-by: Michael Telatynski <[email protected]> * Deprecate AccessibleTooltipButton Signed-off-by: Michael Telatynski <[email protected]> * Fix tests Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Fix `UserInfo-test.tsx` * Update `LoginWithQRFlow-test.tsx` snapshot * Remove tooltip provider from test * Fix `AccessibleButton` * Update snapshots * Revert to original import * Use title to populate aria-label * Rollback AccessibleButton or Tooltip changes. Will come in another PR * Rollback en.json change * Update snapshots * Fix `UserInfo` * Update snapshots * Use label instead of title in test * Use label instead of title in TAC test * Use label instead of title in read-receipt test * Remove tooltip for ContextMenu * Add extra information for caption field --------- Signed-off-by: Michael Telatynski <[email protected]> Co-authored-by: Michael Telatynski <[email protected]>
matrix-org · Apr 24, 2024 · 700b395 · 700b395
1 parent 644bf78
commit 700b395
Show file tree

Hide file tree

Showing 24 changed files with 147 additions and 76 deletions.
diff --git a/playwright/e2e/read-receipts/index.ts b/playwright/e2e/read-receipts/index.ts
@@ -403,15 +403,15 @@ class Helpers {
      * tests we only open the threads panel.)
      */
     async closeThreadsPanel() {
-        await this.page.locator(".mx_RightPanel").getByTitle("Close").click();
+        await this.page.locator(".mx_RightPanel").getByLabel("Close").click();
         await expect(this.page.locator(".mx_RightPanel")).not.toBeVisible();
     }
 
     /**
      * Return to the list of threads, given we are viewing a single thread.
      */
     async backToThreadsList() {
-        await this.page.locator(".mx_RightPanel").getByTitle("Threads").click();
+        await this.page.locator(".mx_RightPanel").getByLabel("Threads").click();
     }
 
     /**
@@ -539,7 +539,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[title="Threads"]');
+            const $button = $panel.querySelector<HTMLElement>('.mx_BaseCard_back[aria-label="Threads"]');
             // If the Threads back button is present then click it - the
             // threads button can open either threads list or thread panel
             if ($button) {

diff --git a/playwright/e2e/spaces/threads-activity-centre/index.ts b/playwright/e2e/spaces/threads-activity-centre/index.ts
@@ -341,7 +341,7 @@ export class Helpers {
      */
     assertThreadPanelFocused() {
         return expect(
-            this.page.locator(".mx_ThreadPanel").locator(".mx_BaseCard_header").getByTitle("Close"),
+            this.page.locator(".mx_ThreadPanel").locator(".mx_BaseCard_header").getByLabel("Close"),
         ).toBeFocused();
     }
 

diff --git a/src/accessibility/context_menu/ContextMenuButton.tsx b/src/accessibility/context_menu/ContextMenuButton.tsx
@@ -36,7 +36,6 @@ export const ContextMenuButton = forwardRef(function <T extends keyof JSX.Intrin
             {...props}
             onClick={onClick}
             onContextMenu={onContextMenu ?? onClick ?? undefined}
-            title={label}
             aria-label={label}
             aria-haspopup={true}
             aria-expanded={isExpanded}

diff --git a/src/components/views/elements/AccessibleButton.tsx b/src/components/views/elements/AccessibleButton.tsx
@@ -16,6 +16,7 @@
 
 import React, { forwardRef, FunctionComponent, HTMLAttributes, InputHTMLAttributes, Ref } from "react";
 import classnames from "classnames";
+import { Tooltip } from "@vector-im/compound-web";
 
 import { getKeyBindingsManager } from "../../../KeyBindingsManager";
 import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts";
@@ -86,6 +87,15 @@ type Props<T extends keyof JSX.IntrinsicElements> = DynamicHtmlElementProps<T> &
      * Event handler for button activation. Should be implemented exactly like a normal `onClick` handler.
      */
     onClick: ((e: ButtonEvent) => void | Promise<void>) | null;
+    /**
+     * The tooltip to show on hover or focus.
+     */
+    title?: string;
+    /**
+     * The caption is a secondary text displayed under the `title` of the tooltip.
+     * Only valid when used in conjunction with `title`.
+     */
+    caption?: string;
 };
 
 /**
@@ -116,11 +126,14 @@ const AccessibleButton = forwardRef(function <T extends keyof JSX.IntrinsicEleme
         onKeyDown,
         onKeyUp,
         triggerOnMouseDown,
+        title,
+        caption,
         ...restProps
     }: Props<T>,
     ref: Ref<HTMLElement>,
 ): JSX.Element {
     const newProps: RenderedElementProps = restProps;
+    newProps["aria-label"] = newProps["aria-label"] ?? title;
     if (disabled) {
         newProps["aria-disabled"] = true;
         newProps["disabled"] = true;
@@ -182,7 +195,16 @@ const AccessibleButton = forwardRef(function <T extends keyof JSX.IntrinsicEleme
     });
 
     // React.createElement expects InputHTMLAttributes
-    return React.createElement(element, newProps, children);
+    const button = React.createElement(element, newProps, children);
+
+    if (title) {
+        return (
+            <Tooltip label={title} caption={caption} isTriggerInteractive={!disabled}>
+                {button}
+            </Tooltip>
+        );
+    }
+    return button;
 });
 
 // Type assertion required due to forwardRef type workaround in react.d.ts

diff --git a/src/components/views/elements/AccessibleTooltipButton.tsx b/src/components/views/elements/AccessibleTooltipButton.tsx
@@ -60,6 +60,9 @@ type Props<T extends keyof JSX.IntrinsicElements> = ComponentProps<typeof Access
     onHideTooltip?(ev: SyntheticEvent): void;
 };
 
+/**
+ * @deprecated use AccessibleButton with `title` and `caption` instead.
+ */
 const AccessibleTooltipButton = forwardRef(function <T extends keyof JSX.IntrinsicElements>(
     { title, tooltip, children, forceHide, alignment, onHideTooltip, tooltipClassName, ...props }: Props<T>,
     ref: Ref<HTMLElement>,

diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx
@@ -237,7 +237,12 @@ export function DeviceItem({
         );
     } else {
         return (
-            <AccessibleButton className={classes} title={device.deviceId} onClick={onDeviceClick}>
+            <AccessibleButton
+                className={classes}
+                title={device.deviceId}
+                aria-label={deviceName}
+                onClick={onDeviceClick}
+            >
                 <div className={iconClasses} />
                 <div className="mx_UserInfo_device_name">{deviceName}</div>
                 <div className="mx_UserInfo_device_trusted">{trustedLabel}</div>

diff --git a/test/components/structures/__snapshots__/UserMenu-test.tsx.snap b/test/components/structures/__snapshots__/UserMenu-test.tsx.snap
@@ -12,7 +12,6 @@ exports[`<UserMenu> <UserMenu> when video broadcast when rendered should render
       class="mx_AccessibleButton mx_UserMenu_contextMenuButton"
       role="button"
       tabindex="0"
-      title="User menu"
     >
       <div
         class="mx_UserMenu_userAvatar"

diff --git a/test/components/views/beacon/__snapshots__/DialogSidebar-test.tsx.snap b/test/components/views/beacon/__snapshots__/DialogSidebar-test.tsx.snap
@@ -14,11 +14,12 @@ exports[`<DialogSidebar /> renders sidebar correctly with beacons 1`] = `
         View list
       </h4>
       <div
+        aria-label="Close sidebar"
         class="mx_AccessibleButton mx_DialogSidebar_closeButton"
+        data-state="closed"
         data-testid="dialog-sidebar-close"
         role="button"
         tabindex="0"
-        title="Close sidebar"
       >
         <div
           class="mx_DialogSidebar_closeButtonIcon"
@@ -113,11 +114,12 @@ exports[`<DialogSidebar /> renders sidebar correctly without beacons 1`] = `
         View list
       </h4>
       <div
+        aria-label="Close sidebar"
         class="mx_AccessibleButton mx_DialogSidebar_closeButton"
+        data-state="closed"
         data-testid="dialog-sidebar-close"
         role="button"
         tabindex="0"
-        title="Close sidebar"
       >
         <div
           class="mx_DialogSidebar_closeButtonIcon"

diff --git a/test/components/views/beacon/__snapshots__/LeftPanelLiveShareWarning-test.tsx.snap b/test/components/views/beacon/__snapshots__/LeftPanelLiveShareWarning-test.tsx.snap
@@ -3,10 +3,11 @@
 exports[`<LeftPanelLiveShareWarning /> when user has live location monitor renders correctly when minimized 1`] = `
 <DocumentFragment>
   <div
+    aria-label="You are sharing your live location"
     class="mx_AccessibleButton mx_LeftPanelLiveShareWarning mx_LeftPanelLiveShareWarning__minimized"
+    data-state="closed"
     role="button"
     tabindex="0"
-    title="You are sharing your live location"
   >
     <div
       height="10"

diff --git a/test/components/views/beacon/__snapshots__/RoomLiveShareWarning-test.tsx.snap b/test/components/views/beacon/__snapshots__/RoomLiveShareWarning-test.tsx.snap
@@ -109,11 +109,12 @@ exports[`<RoomLiveShareWarning /> when user has live beacons and geolocation is
       Retry
     </button>
     <button
+      aria-label="Stop and close"
       class="mx_AccessibleButton mx_RoomLiveShareWarning_closeButton"
+      data-state="closed"
       data-testid="room-live-share-wire-error-close-button"
       role="button"
       tabindex="0"
-      title="Stop and close"
     >
       <div
         class="mx_RoomLiveShareWarning_closeButtonIcon"

diff --git a/test/components/views/elements/AppTile-test.tsx b/test/components/views/elements/AppTile-test.tsx
@@ -377,12 +377,12 @@ describe("AppTile", () => {
         });
 
         it("clicking 'minimise' should send the widget to the right", async () => {
-            await userEvent.click(renderResult.getByTitle("Minimise"));
+            await userEvent.click(renderResult.getByLabelText("Minimise"));
             expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Right);
         });
 
         it("clicking 'maximise' should send the widget to the center", async () => {
-            await userEvent.click(renderResult.getByTitle("Maximise"));
+            await userEvent.click(renderResult.getByLabelText("Maximise"));
             expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Center);
         });
 
@@ -435,7 +435,7 @@ describe("AppTile", () => {
             });
 
             it("clicking 'un-maximise' should send the widget to the top", async () => {
-                await userEvent.click(renderResult.getByTitle("Un-maximise"));
+                await userEvent.click(renderResult.getByLabelText("Un-maximise"));
                 expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Top);
             });
         });
@@ -461,7 +461,7 @@ describe("AppTile", () => {
             });
 
             it("should display the »Popout widget« button", () => {
-                expect(renderResult.getByTitle("Popout widget")).toBeInTheDocument();
+                expect(renderResult.getByLabelText("Popout widget")).toBeInTheDocument();
             });
         });
     });

diff --git a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap
@@ -13,11 +13,12 @@ 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"
+          data-state="closed"
           data-testid="base-card-close-button"
           role="button"
           tabindex="0"
-          title="Close"
         />
         <div
           class="mx_BaseCard_headerProp"
@@ -37,7 +38,6 @@ exports[`AppTile destroys non-persisted right panel widget on room change 1`] =
               class="mx_AccessibleButton mx_BaseCard_header_title_button--option"
               role="button"
               tabindex="0"
-              title="Options"
             />
           </div>
         </div>
@@ -136,20 +136,22 @@ exports[`AppTile for a pinned widget should render 1`] = `
         class="mx_AppTileMenuBar_widgets"
       >
         <div
+          aria-label="Un-maximise"
           class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
+          data-state="closed"
           role="button"
           tabindex="0"
-          title="Un-maximise"
         >
           <div
             class="mx_Icon mx_Icon_12"
           />
         </div>
         <div
+          aria-label="Minimise"
           class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
+          data-state="closed"
           role="button"
           tabindex="0"
-          title="Minimise"
         >
           <div
             class="mx_Icon mx_Icon_12"
@@ -162,7 +164,6 @@ exports[`AppTile for a pinned widget should render 1`] = `
           class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
           role="button"
           tabindex="0"
-          title="Options"
         >
           <div
             class="mx_Icon mx_Icon_12"
@@ -223,20 +224,22 @@ exports[`AppTile for a pinned widget should render permission request 1`] = `
         class="mx_AppTileMenuBar_widgets"
       >
         <div
+          aria-label="Un-maximise"
           class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
+          data-state="closed"
           role="button"
           tabindex="0"
-          title="Un-maximise"
         >
           <div
             class="mx_Icon mx_Icon_12"
           />
         </div>
         <div
+          aria-label="Minimise"
           class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
+          data-state="closed"
           role="button"
           tabindex="0"
-          title="Minimise"
         >
           <div
             class="mx_Icon mx_Icon_12"
@@ -249,7 +252,6 @@ exports[`AppTile for a pinned widget should render permission request 1`] = `
           class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
           role="button"
           tabindex="0"
-          title="Options"
         >
           <div
             class="mx_Icon mx_Icon_12"
@@ -377,20 +379,22 @@ exports[`AppTile preserves non-persisted widget on container move 1`] = `
               class="mx_AppTileMenuBar_widgets"
             >
               <div
+                aria-label="Maximise"
                 class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
+                data-state="closed"
                 role="button"
                 tabindex="0"
-                title="Maximise"
               >
                 <div
                   class="mx_Icon mx_Icon_12"
                 />
               </div>
               <div
+                aria-label="Minimise"
                 class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
+                data-state="closed"
                 role="button"
                 tabindex="0"
-                title="Minimise"
               >
                 <div
                   class="mx_Icon mx_Icon_12"
@@ -403,7 +407,6 @@ exports[`AppTile preserves non-persisted widget on container move 1`] = `
                 class="mx_AccessibleButton mx_AppTileMenuBar_widgets_button"
                 role="button"
                 tabindex="0"
-                title="Options"
               >
                 <div
                   class="mx_Icon mx_Icon_12"

diff --git a/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap b/test/components/views/location/__snapshots__/LocationViewDialog-test.tsx.snap
@@ -23,22 +23,24 @@ exports[`<LocationViewDialog /> renders map correctly 1`] = `
     class="mx_ZoomButtons"
   >
     <div
+      aria-label="Zoom in"
       class="mx_AccessibleButton mx_ZoomButtons_button"
+      data-state="closed"
       data-testid="map-zoom-in-button"
       role="button"
       tabindex="0"
-      title="Zoom in"
     >
       <div
         class="mx_ZoomButtons_icon"
       />
     </div>
     <div
+      aria-label="Zoom out"
       class="mx_AccessibleButton mx_ZoomButtons_button"
+      data-state="closed"
       data-testid="map-zoom-out-button"
       role="button"
       tabindex="0"
-      title="Zoom out"
     >
       <div
         class="mx_ZoomButtons_icon"

diff --git a/test/components/views/location/__snapshots__/ZoomButtons-test.tsx.snap b/test/components/views/location/__snapshots__/ZoomButtons-test.tsx.snap
@@ -6,22 +6,24 @@ exports[`<ZoomButtons /> renders buttons 1`] = `
     class="mx_ZoomButtons"
   >
     <div
+      aria-label="Zoom in"
       class="mx_AccessibleButton mx_ZoomButtons_button"
+      data-state="closed"
       data-testid="map-zoom-in-button"
       role="button"
       tabindex="0"
-      title="Zoom in"
     >
       <div
         class="mx_ZoomButtons_icon"
       />
     </div>
     <div
+      aria-label="Zoom out"
       class="mx_AccessibleButton mx_ZoomButtons_button"
+      data-state="closed"
       data-testid="map-zoom-out-button"
       role="button"
       tabindex="0"
-      title="Zoom out"
     >
       <div
         class="mx_ZoomButtons_icon"