diff --git a/src/components/Tooltip/Tooltip.stories.tsx b/src/components/Tooltip/Tooltip.stories.tsx index ece8232a..a90bff88 100644 --- a/src/components/Tooltip/Tooltip.stories.tsx +++ b/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: Just some text, }, }; diff --git a/src/components/Tooltip/Tooltip.test.tsx b/src/components/Tooltip/Tooltip.test.tsx index 291cca79..b935b925 100644 --- a/src/components/Tooltip/Tooltip.test.tsx +++ b/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( + + + + + , + ); + screen.getByRole("image", { name: "User profile" }); + }); }); diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx index f0acec9f..28bf65c6 100644 --- a/src/components/Tooltip/Tooltip.tsx +++ b/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 ( - - {isTriggerInteractive ? ( - children - ) : ( - {children} - )} + + {children} @@ -156,11 +158,21 @@ function TooltipContent({ ); } +interface TooltipAnchorProps { + children: ReactNode; + isTriggerInteractive: boolean; + nonInteractiveTriggerTabIndex?: number; +} + /** * The anchor of the tooltip * @param children */ -function TooltipAnchor({ children }: Readonly): JSX.Element { +const TooltipAnchor: FC = ({ + children, + isTriggerInteractive, + nonInteractiveTriggerTabIndex, +}) => { const context = useTooltipContext(); // The children can have a ref and we don't want to discard it @@ -168,19 +180,40 @@ function TooltipAnchor({ children }: Readonly): JSX.Element { const childrenRef = (children as unknown as { ref?: Ref })?.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 ( + + {cloneElement(children as ReactElement, { + "aria-labelledby": labelId, + "aria-describedby": descriptionId, + })} + + ); + } + }, [context, ref, children]); if (!element) { throw new Error("Tooltip anchor must be a single valid React element"); } return element; -} +};