Allow tooltips to label non-interactive triggers

The combination of tooltip labels and non-interactive triggers was problematic: The tooltip component would place the label on the span element that it creates, rather than your provided trigger element. If your trigger was an icon or image, then accessibility tooling would flag it as missing a label. You might try putting aria-hidden on the element, but then you would still end up with the label being on an element that lacks a role (which is illegal).

The solution I've implemented here is to move the aria-labelledby and aria-describedby attributes to the trigger.

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;
-}
+};