fix: update shadcn input and tooltip to latest versions #236
Conversation
|
@kumarayushkumar is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe Input and Tooltip components were refactored from forwardRef-based wrappers to plain function components. Input removes ref forwarding and adds data-slot attribute with expanded classNames. Tooltip replaces static aliases with explicit functions, adding TooltipProvider composition, data-slot attributes, and adjusting default sideOffset from 4 to 0. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/ui/input.tsx(1 hunks)components/ui/tooltip.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/ui/tooltip.tsx (1)
lib/utils.ts (1)
cn(6-8)
components/ui/input.tsx (1)
lib/utils.ts (1)
cn(6-8)
🔇 Additional comments (6)
components/ui/input.tsx (2)
1-3: LGTM!Imports are correct and appropriate for the component.
5-19: LGTM! Changes align with PR objectives.The Input component correctly implements the latest shadcn/ui patterns:
- Removal of
forwardRefis appropriate for React 19, whererefis now a regular prop.- Dark mode variants are present (
dark:bg-input/30,dark:aria-invalid:ring-destructive/40).- The
data-slot="input"attribute aligns with modern shadcn/ui component architecture.components/ui/tooltip.tsx (4)
1-6: LGTM!The "use client" directive and imports are correct for a Radix UI-based tooltip component.
8-19: LGTM!TooltipProvider correctly wraps the Radix primitive. The
delayDuration={0}default provides immediate tooltip display, which aligns with modern UI expectations.
31-35: LGTM!TooltipTrigger is correctly implemented with the
data-slotattribute.
37-59: LGTM! Changes align with PR objectives.TooltipContent correctly implements the latest patterns:
sideOffset={0}default (changed from previous value) for tighter positioning- Includes
TooltipPrimitive.Arrowfor the tooltip pointer- Animation classes and color tokens provide proper styling
- The implementation supports dark mode through semantic color tokens (
bg-foreground,text-background)
| function Tooltip({ | ||
| ...props | ||
| }: React.ComponentProps<typeof TooltipPrimitive.Root>) { | ||
| return ( | ||
| <TooltipProvider> | ||
| <TooltipPrimitive.Root data-slot="tooltip" {...props} /> | ||
| </TooltipProvider> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify this pattern matches the latest shadcn/ui implementation.
The Tooltip component embeds its own TooltipProvider, creating a new provider per tooltip instance. This differs from the typical Radix UI pattern where a single TooltipProvider wraps the application root and multiple tooltips share it.
Implications:
- Users cannot share provider configuration across multiple tooltips
- Performance overhead with many tooltip instances (each creates a new React context provider)
- Users cannot wrap
TooltipProviderat a higher level to configure all tooltips globally
Action required:
Please verify this matches the latest shadcn/ui tooltip implementation. If this is indeed the new pattern, consider documenting it for users migrating from older versions.
🌐 Web query:
Latest shadcn/ui tooltip component implementation pattern TooltipProvider usage
💡 Result:
Short answer — wrap your Tooltip(s) with TooltipProvider at a high level (usually near App/layout) and use the Tooltip + TooltipTrigger + TooltipContent primitives for each tooltip. Example (official pattern):
import {
Tooltip,
TooltipContent,
TooltipProvider,
TooltipTrigger,
} from "@/components/ui/tooltip"
function App() {
return (
Hover me
Tooltip text
)
}
Notes:
- TooltipProvider gives global defaults (timing/behavior) for nested tooltips; place it near your app root or layout. [1][2]
- Use TooltipTrigger asChild when you want the trigger to be the child element without extra wrapper. [1]
Sources: official shadcn/ui docs. [1] UI docs (latest site) (ui.shadcn.com) [2] v3 docs / examples. (v3.shadcn.com)
Citations:
- 1: https://ui.shadcn.com/docs/components/tooltip?utm_source=openai
- 2: https://v3.shadcn.com/docs/components/tooltip?utm_source=openai
Fix: Remove embedded TooltipProvider and align with official shadcn/ui pattern.
The current implementation wraps each Tooltip instance with its own TooltipProvider, which deviates from the official shadcn/ui pattern where TooltipProvider should be placed near your app root or layout and multiple tooltips share it.
Issues with current pattern:
- Creates unnecessary React context overhead (one provider per tooltip instead of one shared provider)
- Prevents users from configuring provider behavior globally via a single
TooltipProvider - Does not match the official usage pattern where a single
TooltipProviderwraps multipleTooltip+TooltipTrigger+TooltipContentinstances
Remove the embedded TooltipProvider from the Tooltip component and expect users to place TooltipProvider at a high level in their application tree (or document this custom pattern with an explicit reason if this deviation is intentional).
🤖 Prompt for AI Agents
components/ui/tooltip.tsx around lines 21 to 29: the Tooltip component currently
wraps each instance in its own TooltipProvider which creates per-tooltip context
overhead and deviates from the shadcn/ui pattern; remove the embedded
TooltipProvider and have Tooltip simply forward props to TooltipPrimitive.Root
so that a single TooltipProvider can be placed at the app root or layout and
shared by all Tooltip instances; update any docs/comments to instruct consumers
to wrap their app/layout with TooltipProvider if they need global tooltip
configuration.
|
@jnsahaj please let me what need to be fixed in this PR |
1 participant
What
Updates
inputandtooltipcomponents to the latest shadcn/ui versions that includedark:variant styles for improved dark mode support.Changes
inputcomponent with latest dark mode variants (dark:bg-input/30, etc.)tooltipcomponent to latest versionFixes #231
Summary by CodeRabbit
Refactor
Style