Merge pull request #246 from element-hq/tooltip-label-child

Allow tooltips to label non-interactive triggers

src/components/Tooltip/Tooltip.stories.tsx

@@ -177,7 +177,7 @@ export const NonInteractiveTrigger = {
   args: {
     isTriggerInteractive: false,
     description: "Shown without delay",
-    children: "Just some text",
+    children: <span>Just some text</span>,
   },
 };
 

src/components/Tooltip/Tooltip.test.tsx

@@ -21,6 +21,9 @@ import React from "react";
 import * as stories from "./Tooltip.stories";
 import { composeStories, composeStory } from "@storybook/react";
 import userEvent from "@testing-library/user-event";
+import { TooltipProvider } from "./TooltipProvider";
+import { Tooltip } from "./Tooltip";
+import { UserIcon } from "@vector-im/compound-design-tokens/assets/web/icons";
 
 const {
   Default,
@@ -79,7 +82,7 @@ describe("Tooltip", () => {
     expect(screen.queryByRole("tooltip")).toBe(null);
     await user.tab();
     // trigger focused, tooltip shown
-    expect(screen.getByText("Just some text")).toHaveFocus();
+    expect(screen.getByText("Just some text").parentElement).toHaveFocus();
     screen.getByRole("tooltip");
   });
 
@@ -125,4 +128,15 @@ describe("Tooltip", () => {
     screen.getByRole("tooltip", { name: "Employer Identification Number" });
     expect(screen.queryByRole("button", { name: "EIN" })).toBe(null);
   });
+
+  it("labels an image", async () => {
+    render(
+      <TooltipProvider>
+        <Tooltip isTriggerInteractive={false} label="User profile">
+          <UserIcon role="image" width={24} height={24} />
+        </Tooltip>
+      </TooltipProvider>,
+    );
+    screen.getByRole("image", { name: "User profile" });
+  });
 });

src/components/Tooltip/Tooltip.tsx

@@ -27,6 +27,9 @@ import React, {
   isValidElement,
   cloneElement,
   useMemo,
+  ReactNode,
+  FC,
+  ReactElement,
 } from "react";
 
 import classNames from "classnames";
@@ -72,12 +75,11 @@ export function Tooltip({
 
   return (
     <TooltipContext.Provider value={context}>
-      <TooltipAnchor>
-        {isTriggerInteractive ? (
-          children
-        ) : (
-          <span tabIndex={nonInteractiveTriggerTabIndex}>{children}</span>
-        )}
+      <TooltipAnchor
+        isTriggerInteractive={isTriggerInteractive}
+        nonInteractiveTriggerTabIndex={nonInteractiveTriggerTabIndex}
+      >
+        {children}
       </TooltipAnchor>
       <TooltipContent>
         <span id={context.labelId}>
@@ -156,31 +158,62 @@ function TooltipContent({
   );
 }
 
+interface TooltipAnchorProps {
+  children: ReactNode;
+  isTriggerInteractive: boolean;
+  nonInteractiveTriggerTabIndex?: number;
+}
+
 /**
  * The anchor of the tooltip
  * @param children
  */
-function TooltipAnchor({ children }: Readonly<PropsWithChildren>): JSX.Element {
+const TooltipAnchor: FC<TooltipAnchorProps> = ({
+  children,
+  isTriggerInteractive,
+  nonInteractiveTriggerTabIndex,
+}) => {
   const context = useTooltipContext();
 
   // The children can have a ref and we don't want to discard it
   // Doing a dirty cast to get the optional ref
   const childrenRef = (children as unknown as { ref?: Ref<HTMLElement> })?.ref;
   const ref = useMergeRefs([context.refs.setReference, childrenRef]);
 
-  // We need to check `isValidElement` to infer the type of `children`
-  const childrenProps = isValidElement(children) && children.props;
-
   const element = useMemo(() => {
     if (!isValidElement(children)) return;
 
-    const props = context.getReferenceProps({ ref, ...childrenProps });
-    return cloneElement(children, props);
-  }, [context, ref, children, childrenProps]);
+    if (isTriggerInteractive) {
+      const props = context.getReferenceProps({ ref, ...children.props });
+      return cloneElement(children, props);
+    } else {
+      // For a non-interactive trigger, we want most of the props to go on the
+      // span element that we provide, since that's what receives focus, but it
+      // should still be the trigger that receives the label/description. It
+      // would be wrong to label the span element, as it lacks a role.
+      const props = context.getReferenceProps({
+        ref,
+        tabIndex: nonInteractiveTriggerTabIndex,
+      });
+      const {
+        "aria-labelledby": labelId,
+        "aria-describedby": descriptionId,
+        ...spanProps
+      } = props;
+      return (
+        <span tabIndex={nonInteractiveTriggerTabIndex} {...spanProps}>
+          {cloneElement(children as ReactElement, {
+            "aria-labelledby": labelId,
+            "aria-describedby": descriptionId,
+          })}
+        </span>
+      );
+    }
+  }, [context, ref, children]);
 
   if (!element) {
     throw new Error("Tooltip anchor must be a single valid React element");
   }
 
   return element;
-}
+};