Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors Tooltip from a prop-based API to a composition-based API (Tooltip.Trigger / Tooltip.Content), introduces TooltipTrigger/TooltipProvider/TooltipContent components, removes legacy TooltipRoot/provider/utils, updates styles, docs, demos, tests, and adapts sidebar usage to the new composition and trackCursorAxis behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Trigger as Tooltip.Trigger
participant Provider as Tooltip.Provider
participant Content as Tooltip.Content
participant Portal as DOM/Portal
User->>Trigger: click / hover
Trigger->>Provider: request open (onOpenChange)
Provider-->>Trigger: manage delay/trackCursorAxis
Provider->>Content: mount/show with side/align/showArrow
Content->>Portal: render content + optional arrow
Portal-->>User: visible tooltip
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/www/src/content/docs/components/tooltip/demo.ts`:
- Around line 104-106: The inline style on the span in the Tooltip demo uses an
invalid fontWeight value ("medium"); locate the <span style={{ fontWeight:
"medium" }}>Custom Tooltip</span> in
apps/www/src/content/docs/components/tooltip/demo.ts and replace "medium" with a
valid CSS font-weight such as "500" or the keyword "normal"/"bold" (e.g.,
fontWeight: 500 or fontWeight: "normal") so the browser applies the weight
correctly.
In `@apps/www/src/content/docs/components/tooltip/index.mdx`:
- Line 7: Remove the unused import noArrowDemo from the import list in
apps/www/src/content/docs/components/tooltip/index.mdx (the current line that
imports playground, sideDemo, alignDemo, customDemo, providerDemo,
trackCursorDemo, noArrowDemo, arrowDemo) or alternatively add a new
documentation section that references noArrowDemo (e.g., a "Without Arrow" demo
contrasting with the "With Arrow" section) and render it in the MDX content so
the import is used.
In `@apps/www/src/content/docs/components/tooltip/props.ts`:
- Around line 47-52: The JSDoc in tooltip-content.tsx incorrectly documents
showArrow as `@default` true while the implementation sets showArrow = false;
update the JSDoc on the showArrow prop in tooltip-content.tsx to `@default` false
so it matches the actual default behavior and the props.ts documentation
(reference: showArrow in tooltip-content.tsx and TooltipContentProps in
props.ts).
In `@packages/raystack/components/tooltip/__tests__/tooltip.test.tsx`:
- Around line 154-165: The test expecting an arrow by default contradicts
TooltipContent's default (showArrow = false); update the test in
tooltip.test.tsx to explicitly opt-in to the arrow: render the Tooltip with
Tooltip.Content set to showArrow={true} (keep Tooltip.Trigger the same) and then
assert the arrow exists, i.e., change the test case that currently asserts
default arrow visibility to test the showArrow={true} behavior on
Tooltip.Content.
In `@packages/raystack/components/tooltip/tooltip-content.tsx`:
- Around line 15-19: Update the JSDoc for the showArrow prop on the
TooltipContent component to match the implementation and docs: change the
`@default` value from true to false so it reflects the default assignment
(showArrow = false) and the "Arrow is opt-in" behavior; ensure the JSDoc above
the showArrow?: boolean declaration and any related prop documentation use
"@default false".
In `@packages/raystack/components/tooltip/tooltip.module.css`:
- Around line 43-57: The align-based animation selectors
(.content[data-side="top"][data-align="start"]:not([data-instant]),
.content[data-side="top"][data-align="end"]:not([data-instant]),
.content[data-side="bottom"][data-align="start"]:not([data-instant]),
.content[data-side="bottom"][data-align="end"]:not([data-instant])) are missing
the [data-open] attribute and should only apply when the tooltip is open; update
each of these selectors to include [data-open] (matching the side-based rules)
so the animations only run for open tooltips.
🧹 Nitpick comments (3)
packages/raystack/components/tooltip/tooltip-misc.tsx (1)
8-13: Consider using a more flexible ref type for the render prop pattern.The
TooltipTriggeris typed withHTMLButtonElementref, but therenderprop allows rendering arbitrary elements (e.g.,<div>insidebar-root.tsx). This could cause TypeScript mismatches when consumers pass refs for non-button elements.♻️ Suggested type adjustment
-export const TooltipTrigger = forwardRef< - HTMLButtonElement, - TooltipPrimitive.Trigger.Props ->((props, ref) => { +export const TooltipTrigger = forwardRef< + HTMLElement, + TooltipPrimitive.Trigger.Props +>((props, ref) => {apps/www/src/content/docs/components/tooltip/demo.ts (2)
5-16: Consider omittingtrackCursorAxiswhen value is'none'.The current implementation outputs
trackCursorAxis="none"when the value is'none', but since'none'is the default, it would be cleaner to omit it entirely from the generated code.♻️ Proposed improvement
export const getCode = (props: any) => { const { children = 'Tooltip message', trackCursorAxis, ...rest } = props; + const showTrackCursor = trackCursorAxis && trackCursorAxis !== 'none'; return ` - <Tooltip ${trackCursorAxis ? `trackCursorAxis="${trackCursorAxis}"` : ''}> + <Tooltip${showTrackCursor ? ` trackCursorAxis="${trackCursorAxis}"` : ''}> <Tooltip.Trigger render={<Button />}> Hover me </Tooltip.Trigger> <Tooltip.Content ${getPropsString(rest)}> ${children} </Tooltip.Content> </Tooltip>`; };
43-47: RedundantinitialValueanddefaultValueforshowArrow.Both
initialValueanddefaultValueare set tofalse. Consider removing one for consistency with other controls that only use one or the other.♻️ Proposed fix
showArrow: { type: 'checkbox', - initialValue: false, defaultValue: false },
| <div> | ||
| <span style={{ fontWeight: "medium" }}>Custom Tooltip</span> | ||
| </div> |
There was a problem hiding this comment.
Invalid CSS fontWeight value.
"medium" is not a valid CSS font-weight value. Valid values are numeric (100-900) or keywords like "normal" (400), "bold" (700). This will be ignored by the browser.
🐛 Proposed fix
- <span style={{ fontWeight: "medium" }}>Custom Tooltip</span>
+ <span style={{ fontWeight: 500 }}>Custom Tooltip</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div> | |
| <span style={{ fontWeight: "medium" }}>Custom Tooltip</span> | |
| </div> | |
| <div> | |
| <span style={{ fontWeight: 500 }}>Custom Tooltip</span> | |
| </div> |
🤖 Prompt for AI Agents
In `@apps/www/src/content/docs/components/tooltip/demo.ts` around lines 104 - 106,
The inline style on the span in the Tooltip demo uses an invalid fontWeight
value ("medium"); locate the <span style={{ fontWeight: "medium" }}>Custom
Tooltip</span> in apps/www/src/content/docs/components/tooltip/demo.ts and
replace "medium" with a valid CSS font-weight such as "500" or the keyword
"normal"/"bold" (e.g., fontWeight: 500 or fontWeight: "normal") so the browser
applies the weight correctly.
| --- | ||
|
|
||
| import { playground, sideDemo, customDemo,followCursorDemo, providerDemo } from "./demo.ts"; | ||
| import { playground, sideDemo, alignDemo, customDemo, providerDemo, trackCursorDemo, noArrowDemo, arrowDemo } from "./demo.ts"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for noArrowDemo usage in the tooltip directory
rg -n 'noArrowDemo' apps/www/src/content/docs/components/tooltip/Repository: raystack/apsara
Length of output: 243
🏁 Script executed:
#!/bin/bash
# Read the entire file to see what demos are actually used in the content
cat -n apps/www/src/content/docs/components/tooltip/index.mdx | head -100Repository: raystack/apsara
Length of output: 2673
🏁 Script executed:
#!/bin/bash
# Check if there's more content in the file after line 100
wc -l apps/www/src/content/docs/components/tooltip/index.mdx
tail -20 apps/www/src/content/docs/components/tooltip/index.mdxRepository: raystack/apsara
Length of output: 563
Remove unused import noArrowDemo.
The noArrowDemo is imported but not used anywhere in the documentation. Either remove the import or add a section demonstrating the no-arrow variant (e.g., contrasting with the "With Arrow" section).
🤖 Prompt for AI Agents
In `@apps/www/src/content/docs/components/tooltip/index.mdx` at line 7, Remove the
unused import noArrowDemo from the import list in
apps/www/src/content/docs/components/tooltip/index.mdx (the current line that
imports playground, sideDemo, alignDemo, customDemo, providerDemo,
trackCursorDemo, noArrowDemo, arrowDemo) or alternatively add a new
documentation section that references noArrowDemo (e.g., a "Without Arrow" demo
contrasting with the "With Arrow" section) and render it in the MDX content so
the import is used.
| export interface TooltipContentProps { | ||
| /** | ||
| * Controls whether to show the arrow | ||
| * @default false | ||
| */ | ||
| onOpenChange?: (open: boolean) => void; | ||
| showArrow?: boolean; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find tooltip-related files
find apps/www/src -name "*tooltip*" -type f | head -20Repository: raystack/apsara
Length of output: 114
🏁 Script executed:
# Read the props.ts file around lines 47-52
cat -n apps/www/src/content/docs/components/tooltip/props.ts | sed -n '40,60p'Repository: raystack/apsara
Length of output: 530
🏁 Script executed:
# Search for tooltip-content files more broadly
find . -name "*tooltip-content*" -type f 2>/dev/nullRepository: raystack/apsara
Length of output: 117
🏁 Script executed:
# Search for showArrow default values in TypeScript files
rg "showArrow" --type ts -B 2 -A 2Repository: raystack/apsara
Length of output: 4552
Fix JSDoc documentation for showArrow in tooltip-content.tsx.
The JSDoc comment in tooltip-content.tsx incorrectly states @default true for showArrow, but the actual implementation defaults to false (line with showArrow = false). This contradicts both the documentation in props.ts which correctly states @default false. Update the JSDoc comment to match the actual implementation.
🤖 Prompt for AI Agents
In `@apps/www/src/content/docs/components/tooltip/props.ts` around lines 47 - 52,
The JSDoc in tooltip-content.tsx incorrectly documents showArrow as `@default`
true while the implementation sets showArrow = false; update the JSDoc on the
showArrow prop in tooltip-content.tsx to `@default` false so it matches the actual
default behavior and the props.ts documentation (reference: showArrow in
tooltip-content.tsx and TooltipContentProps in props.ts).
packages/raystack/components/tooltip/__tests__/tooltip.test.tsx
Outdated
Show resolved
Hide resolved
| /** | ||
| * Controls whether to show the arrow | ||
| * @default true | ||
| */ | ||
| showArrow?: boolean; |
There was a problem hiding this comment.
JSDoc @default value is incorrect.
The JSDoc states @default true, but the implementation defaults to false (line 30: showArrow = false). This contradicts both the PR description ("Arrow is opt-in") and the documentation in apps/www/src/content/docs/components/tooltip/props.ts which correctly states @default false.
📝 Proposed fix
/**
* Controls whether to show the arrow
- * `@default` true
+ * `@default` false
*/
showArrow?: boolean;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Controls whether to show the arrow | |
| * @default true | |
| */ | |
| showArrow?: boolean; | |
| /** | |
| * Controls whether to show the arrow | |
| * `@default` false | |
| */ | |
| showArrow?: boolean; |
🤖 Prompt for AI Agents
In `@packages/raystack/components/tooltip/tooltip-content.tsx` around lines 15 -
19, Update the JSDoc for the showArrow prop on the TooltipContent component to
match the implementation and docs: change the `@default` value from true to false
so it reflects the default assignment (showArrow = false) and the "Arrow is
opt-in" behavior; ensure the JSDoc above the showArrow?: boolean declaration and
any related prop documentation use "@default false".
| .content[data-side="top"][data-align="start"]:not([data-instant]) { | ||
| animation-name: slideDownRightAndFade; | ||
| } | ||
|
|
||
| .content[data-side="top"][data-align="end"]:not([data-instant]) { | ||
| animation-name: slideDownLeftAndFade; | ||
| } | ||
|
|
||
| .content[data-side="bottom"][data-align="start"]:not([data-instant]) { | ||
| animation-name: slideUpRightAndFade; | ||
| } | ||
|
|
||
| .content[data-side="bottom"][data-align="end"]:not([data-instant]) { | ||
| animation-name: slideUpLeftAndFade; | ||
| } |
There was a problem hiding this comment.
Missing [data-open] selector on align-based animation rules.
The animation rules for data-align combinations (lines 43-57) are missing the [data-open] attribute selector that the side-based rules use (lines 28-42). This means these animations will apply regardless of the tooltip's open state, which may cause unexpected animation behavior or override the side-based animations incorrectly.
🐛 Proposed fix to add missing [data-open] selector
-.content[data-side="top"][data-align="start"]:not([data-instant]) {
+.content[data-open][data-side="top"][data-align="start"]:not([data-instant]) {
animation-name: slideDownRightAndFade;
}
-.content[data-side="top"][data-align="end"]:not([data-instant]) {
+.content[data-open][data-side="top"][data-align="end"]:not([data-instant]) {
animation-name: slideDownLeftAndFade;
}
-.content[data-side="bottom"][data-align="start"]:not([data-instant]) {
+.content[data-open][data-side="bottom"][data-align="start"]:not([data-instant]) {
animation-name: slideUpRightAndFade;
}
-.content[data-side="bottom"][data-align="end"]:not([data-instant]) {
+.content[data-open][data-side="bottom"][data-align="end"]:not([data-instant]) {
animation-name: slideUpLeftAndFade;
}🤖 Prompt for AI Agents
In `@packages/raystack/components/tooltip/tooltip.module.css` around lines 43 -
57, The align-based animation selectors
(.content[data-side="top"][data-align="start"]:not([data-instant]),
.content[data-side="top"][data-align="end"]:not([data-instant]),
.content[data-side="bottom"][data-align="start"]:not([data-instant]),
.content[data-side="bottom"][data-align="end"]:not([data-instant])) are missing
the [data-open] attribute and should only apply when the tooltip is open; update
each of these selectors to include [data-open] (matching the side-based rules)
so the animations only run for open tooltips.
1 participant
Description
Breaking Changes
Changes
Docs & Tests
Summary by CodeRabbit