Add Tooltip stories for Links and Rich Text Content#2995
Add Tooltip stories for Links and Rich Text Content#2995marcysutton wants to merge 5 commits intomainfrom
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🦋 Changeset detectedLatest commit: 0fccf04 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: NOT Published🤕 Oh noes!! We couldn't find any changesets in this PR (98cf24c). As a result, we did not publish an npm snapshot for you. |
|
Size Change: 0 B Total Size: 121 kB ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-doeeyvhjpy.chromatic.com/ Chromatic results:
|
|
@claude review once |
beaesguerra
left a comment
beaesguerra
left a comment
There was a problem hiding this comment.
Thanks for updating the docs, great idea :)
I left some questions! One thing I wanted to verify is if TooltipContent should be in the example!
| /** | ||
| * To render rich text in tooltip content, pass a React element as `children` | ||
| * instead of a plain string. When a string is passed, it is wrapped in | ||
| * `LabelMedium` and rendered as plain text — HTML tags in a string will appear |
There was a problem hiding this comment.
The comment can be updated!
| * `LabelMedium` and rendered as plain text — HTML tags in a string will appear | |
| * `BodyText` and rendered as plain text — HTML tags in a string will appear |
|
|
||
| /** | ||
| * Tooltips can be used with links as anchors. | ||
| * When a `Link` is the anchor element, set `forceAnchorFocusivity={false}` |
There was a problem hiding this comment.
TIL about forceAnchorFocusivity!
I noticed that the docs for the prop isn't showing up in the SB docs! Could you update that as well?
There was a problem hiding this comment.
Focusivity isn't exactly a word but it works! (I thought it would be "forceAnchorFocusability" when trying to write from memory)
Updated the argtype.
There was a problem hiding this comment.
Yeah, I invented that term a long time ago. Focusability would probably have been a better choice - I can't tell you where my brain was, naming things is hard.
| render: function Render() { | ||
| return ( | ||
| <Tooltip | ||
| content={ |
There was a problem hiding this comment.
I'm wondering if content needs to be wrapped with TooltipContent when passing in jsx! I see the type definition for the prop includes TooltipContent:
It wasn't immediately clear to me in the docs how TooltipContent should be used, so I am glad we're adding more examples :)
cc: @jandrade in case you have context on this!
There was a problem hiding this comment.
I did it this way to mimic the production code, so we could validate it works with the setup in frontend. It seems to work, so maybe the type definition isn't quite right?
There was a problem hiding this comment.
ahh hmmm I think this is another case where the Flow to TS migration couldn't add full type safety support for these cases with TooltipContent. I mean, probably we were expecting to wrap components with TooltipContent, but TS is not able to fully enforce that.
| <View style={styles.scrollbox}> | ||
| <View style={styles.hostbox}> | ||
| <Body> | ||
| <BodyText> |
There was a problem hiding this comment.
Thanks for updating typography components! :)
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * Tooltips can be used with links as anchors. | ||
| * When a `Link` is the anchor element, set `forceAnchorFocusivity={false}` | ||
| * since the link is already keyboard focusable. The tooltip will appear on | ||
| * hover or focus and the `aria-describedby` attribute is automatically applied | ||
| * to the `Link` element. | ||
| */ | ||
| export const WithLinkAnchor: StoryComponentType = { | ||
| render: function Render() { | ||
| return ( | ||
| <Tooltip | ||
| content="This link navigates to the Khan Academy homepage." | ||
| placement="top" | ||
| forceAnchorFocusivity={false} | ||
| > | ||
| <Link href="https://www.khanacademy.org">Khan Academy</Link> | ||
| </Tooltip> | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🟡 The WithLinkAnchor story (lines 169-190) will capture only the link anchor in Chromatic snapshots — the tooltip bubble is never visible because there is no opened={true}, no play function, and no chromatic: { disableSnapshot: true }. This wastes a monthly snapshot and does not demonstrate the tooltip behavior the story is meant to showcase. Fix by adding opened={true} (like WithRichTextContent does in this same PR), a play function to trigger hover (like Default does), or parameters: { chromatic: { disableSnapshot: true } } if visual regression is not needed.
Extended reasoning...
What the bug is and how it manifests
The WithLinkAnchor story renders a Tooltip around a Link component with no mechanism to make the tooltip visible at snapshot time. Chromatic captures a static screenshot of the story in its initial state. Since tooltips only appear on hover or focus, and neither is simulated here, Chromatic will record only the plain "Khan Academy" link — not the tooltip bubble.
The specific code path that triggers it
In tooltip.stories.tsx, the WithLinkAnchor story (around line 169) renders:
<Tooltip
content="This link navigates to the Khan Academy homepage."
placement="top"
forceAnchorFocusivity={false}
>
<Link href="https://www.khanacademy.org">Khan Academy</Link>
</Tooltip>There is no opened={true} prop, no play function, and no parameters.chromatic.disableSnapshot. Chromatic captures this story in its default resting state.
Why existing code doesn't prevent it
Every other story in this file that needs the tooltip bubble visible uses one of three strategies: (1) opened={true} — used by WithRichTextContent, WithStyle, Controlled, and AutoUpdate; (2) a play function that simulates hover — used by Default and ComplexAnchorAndTitle; or (3) chromatic: { disableSnapshot: true } — used by AnchorInScrollableParent, SideBySide, TooltipInModal, TooltipOnButtons, InTopCorner, and InCorners. WithLinkAnchor falls into none of these categories. Critically, WithRichTextContent — the companion story added in this very same PR — correctly uses opened={true}, making the inconsistency visible within the diff itself.
Impact
CLAUDE.md (line 244) explicitly states: "Disable Chromatic for stories that don't need visual regression tests (limited monthly snapshots)." A snapshot that captures only a bare link element with no tooltip visible provides no visual regression value and burns a monthly snapshot. The story's own JSDoc says it demonstrates "Tooltips can be used with links as anchors", but the captured snapshot shows no tooltip at all.
Step-by-step proof
- Chromatic loads the
WithLinkAnchorstory in a headless browser. - The story renders: a
Tooltip(hidden by default) wrapping aLink("Khan Academy"). - No hover or focus event is fired; no
openedprop forces the tooltip open. - Chromatic captures the screenshot — only the "Khan Academy" link is visible.
- The tooltip bubble with "This link navigates to the Khan Academy homepage." never appears in the snapshot.
- The snapshot is stored, consuming a monthly quota, with no useful tooltip regression coverage.
How to fix it
The simplest fix matching this PR's own pattern is to add opened={true} to WithLinkAnchor, mirroring WithRichTextContent. Alternatively, add a play function that hovers the link element (mirroring Default), or add parameters: { chromatic: { disableSnapshot: true } } if tooltip appearance on links does not need visual regression coverage.
There was a problem hiding this comment.
Cool! It caught something I noticed from checking the snapshots visually https://www.chromatic.com/test?appId=5e1bf4b385e3fb0020b7073c&id=69cea98f30ca01a1a71c9dd9
The type for the `Tooltip` `content` prop was a little misleading in that it suggested it could only be used with `TooltipContent`. In reality, any React element could be used--and that's what we saw in `frontend`. This PR widens the type to match runtime behavior.
5 participants
Summary:
We got a question in Slack about whether the Tooltip component supports HTML content, which it does. I noticed there wasn't a story showing this working, so I added one. The apparent consumer code also had incorrect structuring with a Link, and I noticed there wasn't a Link example either. https://khanacademy.slack.com/archives/C8Z9DSKC7/p1775096784762589
Issue: WB-XXXX
Test plan:
/?path=/story/packages-tooltip-tooltip--with-link-anchor/?path=/story/packages-tooltip-tooltip--with-rich-text-content/?path=/story/packages-tooltip-tooltipcontent--rich-text-content