From f54568706793a6952359de1df583cb1ead08d78c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 20 Nov 2023 11:44:08 +1300 Subject: [PATCH 1/6] add isTriggerInteractive prop to Tooltip to set correct delay --- src/components/Tooltip/Tooltip.stories.tsx | 110 +++++++++++++- src/components/Tooltip/Tooltip.test.tsx | 66 ++++++--- src/components/Tooltip/Tooltip.tsx | 12 +- .../__snapshots__/Tooltip.test.tsx.snap | 138 ++++++++++++++++-- 4 files changed, 290 insertions(+), 36 deletions(-) diff --git a/src/components/Tooltip/Tooltip.stories.tsx b/src/components/Tooltip/Tooltip.stories.tsx index 320b20f4..0cd78c4c 100644 --- a/src/components/Tooltip/Tooltip.stories.tsx +++ b/src/components/Tooltip/Tooltip.stories.tsx @@ -21,13 +21,63 @@ import { Tooltip as TooltipComponent } from "./Tooltip"; import { IconButton } from "../IconButton/IconButton"; import UserIcon from "@vector-im/compound-design-tokens/icons/user-profile.svg"; +import { Text } from "../Typography/Text"; export default { title: "Tooltip", component: TooltipComponent, tags: ["autodocs"], - argTypes: {}, - args: {}, + controls: { + include: [ + "side", + "align", + "open", + "label", + "caption", + "isTriggerInteractive", + ], + }, + argTypes: { + side: { + control: "inline-radio", + options: ["left", "right", "top", "bottom"], + }, + align: { + control: "inline-radio", + options: ["center", "start", "end"], + }, + open: { + control: "boolean", + }, + isTriggerInteractive: { + control: "boolean", + }, + label: { + control: "string", + }, + caption: { + control: "string", + }, + }, + args: { + side: "left", + align: "center", + open: undefined, + label: "@bob:example.org", + caption: undefined, + children: ( + + + + ), + }, + decorators: [ + (Story: StoryFn) => ( +
+ +
+ ), + ], } as Meta; const TemplateSide: StoryFn = () => ( @@ -104,3 +154,59 @@ const TemplateAlign: StoryFn = () => ( export const Align = TemplateAlign.bind({}); Align.args = {}; + +export const Default = { + args: { + // unset to test defaults + side: undefined, + align: undefined, + }, +}; + +export const WithCaption = { + args: { + ...Default.args, + label: "Copy", + caption: "⌘ + C", + }, +}; + +export const ForcedOpen = { + args: { + ...Default.args, + open: true, + label: "I'm always open", + }, +}; + +export const ForcedClose = { + args: { + ...Default.args, + open: false, + label: "You can't see me", + children: No tooltip to see here, + }, +}; + +export const InteractiveTrigger = { + args: { + ...Default.args, + isTriggerInteractive: true, + label: "Shown with delay", + }, +}; + +export const NonInteractiveTrigger = { + args: { + ...Default.args, + isTriggerInteractive: false, + label: "Shown without delay", + children: ( + + + + + + ), + }, +}; diff --git a/src/components/Tooltip/Tooltip.test.tsx b/src/components/Tooltip/Tooltip.test.tsx index 8b460644..e9fb6a10 100644 --- a/src/components/Tooltip/Tooltip.test.tsx +++ b/src/components/Tooltip/Tooltip.test.tsx @@ -14,35 +14,63 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { describe, it, expect, beforeAll } from "vitest"; -import { render } from "@testing-library/react"; +import { describe, it, expect, beforeAll, afterEach } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import React from "react"; -import { Tooltip } from "./Tooltip"; -import { IconButton } from "../IconButton/IconButton"; +import * as stories from "./Tooltip.stories"; +import { composeStories } from "@storybook/react"; + +const { Default, WithCaption, ForcedOpen, ForcedClose, InteractiveTrigger } = + composeStories(stories); describe("Tooltip", () => { beforeAll(() => { global.ResizeObserver = require("resize-observer-polyfill"); }); + + afterEach(cleanup); + it("renders open by default", () => { - const { asFragment } = render( - - - - - , - ); + const { asFragment } = render(); + // trigger rendered expect(asFragment()).toMatchSnapshot(); + // tooltip shown + expect(screen.getByRole("tooltip")).toMatchSnapshot(); + }); + + it("renders closed by default", () => { + const { asFragment } = render(); + expect(asFragment()).toMatchSnapshot(); + // no tooltip + expect(screen.queryByRole("tooltip")).not.toBeInTheDocument(); + }); + + it("renders default tooltip", async () => { + render(); + const trigger = screen.getByTestId("testbutton"); + + fireEvent.focus(trigger); + // tooltip shown + expect(await screen.findByRole("tooltip")).toMatchSnapshot(); }); - it("renders", () => { - const { asFragment } = render( - - - - - , - ); + + it("opens tooltip on focus", async () => { + render(); + const trigger = screen.getByTestId("testbutton"); + + fireEvent.focus(trigger); + // tooltip shown + expect(await screen.findByRole("tooltip")).toMatchSnapshot(); + }); + + it("renders with caption", async () => { + const { asFragment } = render(); expect(asFragment()).toMatchSnapshot(); + const trigger = screen.getByTestId("testbutton"); + + fireEvent.focus(trigger); + // tooltip shown + expect(await screen.findByRole("tooltip")).toMatchSnapshot(); }); }); diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx index b6d14a08..80579de3 100644 --- a/src/components/Tooltip/Tooltip.tsx +++ b/src/components/Tooltip/Tooltip.tsx @@ -64,11 +64,20 @@ type TooltipProps = { >["onPointerDownOutside"]; /** * The controlled open state of the tooltip. + * When true, the tooltip is always open. When false, the tooltip is always hidden. + * When undefined, the tooltip will manage its own open state. * You will mostly want to omit this property. Will be used the vast majority * of the time during development. * @default undefined */ open?: boolean; + /** + * Whether the trigger element is interactive. + * When trigger is interactive tooltip will be shown after a 300ms delay. + * When trigger is not interactive tooltip will be shown instantly when pointer enters trigger. + * @default true + */ + isTriggerInteractive?: boolean; }; /** @@ -83,11 +92,12 @@ export const Tooltip = ({ align = "center", onEscapeKeyDown, onPointerDownOutside, + isTriggerInteractive = true, open, }: PropsWithChildren): JSX.Element => { return ( - + {children} renders 1`] = ` +exports[`Tooltip > opens tooltip on focus 1`] = ` + + Shown with delay + +`; + +exports[`Tooltip > renders closed by default 1`] = ` - +

+ No tooltip to see here +

+ `; +exports[`Tooltip > renders default tooltip 1`] = ` + + @bob:example.org + +`; + exports[`Tooltip > renders open by default 1`] = ` - + + +`; + +exports[`Tooltip > renders open by default 2`] = ` + + I'm always open + +`; + +exports[`Tooltip > renders with caption 1`] = ` + +
- - + +
`; + +exports[`Tooltip > renders with caption 2`] = ` + + Copy + + ⌘ + C + + +`; From 2a6c6056a32a42344b46c6fc737e5e6033f541a7 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 20 Nov 2023 14:27:30 +1300 Subject: [PATCH 2/6] fix non-interactive story --- src/components/Tooltip/Tooltip.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Tooltip/Tooltip.stories.tsx b/src/components/Tooltip/Tooltip.stories.tsx index 0cd78c4c..941d225b 100644 --- a/src/components/Tooltip/Tooltip.stories.tsx +++ b/src/components/Tooltip/Tooltip.stories.tsx @@ -203,7 +203,7 @@ export const NonInteractiveTrigger = { label: "Shown without delay", children: ( - + From c536538ea8989c00470ad3e0ef56b53a68417362 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 20 Nov 2023 15:21:11 +1300 Subject: [PATCH 3/6] add focusable wrapper to non-interactive tooltip triggers --- src/components/Tooltip/Tooltip.stories.tsx | 8 +++----- src/components/Tooltip/Tooltip.tsx | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/components/Tooltip/Tooltip.stories.tsx b/src/components/Tooltip/Tooltip.stories.tsx index 941d225b..192cfe0e 100644 --- a/src/components/Tooltip/Tooltip.stories.tsx +++ b/src/components/Tooltip/Tooltip.stories.tsx @@ -202,11 +202,9 @@ export const NonInteractiveTrigger = { isTriggerInteractive: false, label: "Shown without delay", children: ( - - - - - + + + ), }, }; diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx index 80579de3..6f924c83 100644 --- a/src/components/Tooltip/Tooltip.tsx +++ b/src/components/Tooltip/Tooltip.tsx @@ -73,8 +73,11 @@ type TooltipProps = { open?: boolean; /** * Whether the trigger element is interactive. - * When trigger is interactive tooltip will be shown after a 300ms delay. - * When trigger is not interactive tooltip will be shown instantly when pointer enters trigger. + * When trigger is interactive: + * - tooltip will be shown after a 300ms delay. + * When trigger is not interactive: + * - tooltip will be shown instantly when pointer enters trigger. + * - trigger will be wrapped in a focusable span. * @default true */ isTriggerInteractive?: boolean; @@ -98,7 +101,13 @@ export const Tooltip = ({ return ( - {children} + + {isTriggerInteractive ? ( + children + ) : ( + {children} + )} + Date: Wed, 29 Nov 2023 13:28:05 +1300 Subject: [PATCH 4/6] add nonInteractiveTriggerTabIndex to allow overriding tabIndex on non-interactive wrapper --- src/components/Tooltip/Tooltip.test.tsx | 38 +++++++- src/components/Tooltip/Tooltip.tsx | 11 ++- .../__snapshots__/Tooltip.test.tsx.snap | 96 ++++++++++++++++++- 3 files changed, 139 insertions(+), 6 deletions(-) diff --git a/src/components/Tooltip/Tooltip.test.tsx b/src/components/Tooltip/Tooltip.test.tsx index e9fb6a10..462f3d0e 100644 --- a/src/components/Tooltip/Tooltip.test.tsx +++ b/src/components/Tooltip/Tooltip.test.tsx @@ -19,10 +19,16 @@ import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import React from "react"; import * as stories from "./Tooltip.stories"; -import { composeStories } from "@storybook/react"; +import { composeStories, composeStory } from "@storybook/react"; -const { Default, WithCaption, ForcedOpen, ForcedClose, InteractiveTrigger } = - composeStories(stories); +const { + Default, + WithCaption, + ForcedOpen, + ForcedClose, + InteractiveTrigger, + NonInteractiveTrigger, +} = composeStories(stories); describe("Tooltip", () => { beforeAll(() => { @@ -64,6 +70,32 @@ describe("Tooltip", () => { expect(await screen.findByRole("tooltip")).toMatchSnapshot(); }); + it("opens tooltip on focus where trigger is non interactive", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + const trigger = screen.getByTestId("testbutton"); + fireEvent.focus(trigger); + // tooltip shown + expect(await screen.findByRole("tooltip")).toMatchSnapshot(); + }); + + it("overrides default tab index for non interactive triggers", async () => { + const Component = composeStory( + { + ...stories.NonInteractiveTrigger, + args: { + ...stories.NonInteractiveTrigger.args, + nonInteractiveTriggerTabIndex: -1, + }, + }, + stories.default, + ); + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); + it("renders with caption", async () => { const { asFragment } = render(); expect(asFragment()).toMatchSnapshot(); diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx index 6f924c83..be1bea3c 100644 --- a/src/components/Tooltip/Tooltip.tsx +++ b/src/components/Tooltip/Tooltip.tsx @@ -77,10 +77,16 @@ type TooltipProps = { * - tooltip will be shown after a 300ms delay. * When trigger is not interactive: * - tooltip will be shown instantly when pointer enters trigger. - * - trigger will be wrapped in a focusable span. + * - trigger will be wrapped in a span with a tab index from prop nonInteractiveTriggerTabIndex * @default true */ isTriggerInteractive?: boolean; + /** + * Tab index to apply to the span wrapping non interactive tooltip triggers. + * Only used when `isTriggerInteractive` is falsy. + * @default 0 + */ + nonInteractiveTriggerTabIndex?: number; }; /** @@ -96,6 +102,7 @@ export const Tooltip = ({ onEscapeKeyDown, onPointerDownOutside, isTriggerInteractive = true, + nonInteractiveTriggerTabIndex = 0, open, }: PropsWithChildren): JSX.Element => { return ( @@ -105,7 +112,7 @@ export const Tooltip = ({ {isTriggerInteractive ? ( children ) : ( - {children} + {children} )} diff --git a/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap b/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap index a442f0c3..5dc01e24 100644 --- a/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap +++ b/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap @@ -10,6 +10,100 @@ exports[`Tooltip > opens tooltip on focus 1`] = ` `; +exports[`Tooltip > opens tooltip on focus where trigger is non interactive 1`] = ` +
+
+ + + +
+
+`; + +exports[`Tooltip > opens tooltip on focus where trigger is non interactive 2`] = ` + + Shown without delay + +`; + +exports[`Tooltip > overrides default tab index for non interactive triggers 1`] = ` +
+
+ + + +
+
+`; + exports[`Tooltip > renders closed by default 1`] = `
renders with caption 1`] = ` exports[`Tooltip > renders with caption 2`] = ` From 98d63780cba035fdba920e36434f573abe9636ac Mon Sep 17 00:00:00 2001 From: Kerry Date: Mon, 4 Dec 2023 08:30:53 +1300 Subject: [PATCH 5/6] Update src/components/Tooltip/Tooltip.tsx Co-authored-by: Robin --- src/components/Tooltip/Tooltip.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx index be1bea3c..eb65f650 100644 --- a/src/components/Tooltip/Tooltip.tsx +++ b/src/components/Tooltip/Tooltip.tsx @@ -83,7 +83,7 @@ type TooltipProps = { isTriggerInteractive?: boolean; /** * Tab index to apply to the span wrapping non interactive tooltip triggers. - * Only used when `isTriggerInteractive` is falsy. + * Only used when `isTriggerInteractive` is false. * @default 0 */ nonInteractiveTriggerTabIndex?: number; From 51ade3c9260651d6b2ca1e3e0efd78045258f1cc Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 6 Dec 2023 10:33:22 -0500 Subject: [PATCH 6/6] Improve some tests --- src/components/Tooltip/Tooltip.stories.tsx | 3 +-- src/components/Tooltip/Tooltip.test.tsx | 2 ++ src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap | 5 ++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/Tooltip/Tooltip.stories.tsx b/src/components/Tooltip/Tooltip.stories.tsx index 192cfe0e..eb1b6cf8 100644 --- a/src/components/Tooltip/Tooltip.stories.tsx +++ b/src/components/Tooltip/Tooltip.stories.tsx @@ -21,7 +21,6 @@ import { Tooltip as TooltipComponent } from "./Tooltip"; import { IconButton } from "../IconButton/IconButton"; import UserIcon from "@vector-im/compound-design-tokens/icons/user-profile.svg"; -import { Text } from "../Typography/Text"; export default { title: "Tooltip", @@ -184,7 +183,7 @@ export const ForcedClose = { ...Default.args, open: false, label: "You can't see me", - children: No tooltip to see here, + children: No tooltip to see here, }, }; diff --git a/src/components/Tooltip/Tooltip.test.tsx b/src/components/Tooltip/Tooltip.test.tsx index 462f3d0e..3cf55da6 100644 --- a/src/components/Tooltip/Tooltip.test.tsx +++ b/src/components/Tooltip/Tooltip.test.tsx @@ -65,6 +65,7 @@ describe("Tooltip", () => { render(); const trigger = screen.getByTestId("testbutton"); + expect(screen.queryByRole("tooltip")).not.toBeInTheDocument(); fireEvent.focus(trigger); // tooltip shown expect(await screen.findByRole("tooltip")).toMatchSnapshot(); @@ -74,6 +75,7 @@ describe("Tooltip", () => { const { container } = render(); expect(container).toMatchSnapshot(); + expect(screen.queryByRole("tooltip")).not.toBeInTheDocument(); const trigger = screen.getByTestId("testbutton"); fireEvent.focus(trigger); // tooltip shown diff --git a/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap b/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap index 5dc01e24..db0803a2 100644 --- a/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap +++ b/src/components/Tooltip/__snapshots__/Tooltip.test.tsx.snap @@ -109,12 +109,11 @@ exports[`Tooltip > renders closed by default 1`] = `
-

No tooltip to see here -

+
`;