feat: Remove radix ui#699
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request migrates various UI components from Radix UI to Base UI, updates Tailwind CSS theme configurations, and adjusts global styles. The review feedback highlights several issues with the migration: in input-group.tsx, className must be explicitly destructured to avoid overwriting variant classes; in navigation-menu.tsx, NavigationMenuIndicator was incorrectly migrated to NavigationMenuPrimitive.Icon instead of using NavigationMenu.Arrow; and in tooltip.tsx, data-slot attributes were invalidly added to logical components (Tooltip.Provider and Tooltip.Root) that do not render DOM elements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| variant = "ghost", | ||
| size = "xs", | ||
| ...props | ||
| }: Omit<React.ComponentProps<typeof Button>, "size"> & | ||
| VariantProps<typeof inputGroupButtonVariants>) { | ||
| }: Omit<React.ComponentProps<typeof Button>, "size" | "type"> & | ||
| VariantProps<typeof inputGroupButtonVariants> & { | ||
| type?: "button" | "reset" | "submit"; | ||
| }) { |
There was a problem hiding this comment.
The className prop is not destructured from the component parameters, which means it remains inside the ...props object. Because {...props} is spread after the custom className on the Button component, any className passed to InputGroupButton will completely overwrite the classes generated by inputGroupButtonVariants({ size }) instead of merging them.
To fix this, destructure className explicitly so it is excluded from props and can be correctly merged using cn.
className,
variant = "ghost",
size = "xs",
...props
}: Omit<React.ComponentProps<typeof Button>, "size" | "type"> &
VariantProps<typeof inputGroupButtonVariants> & {
type?: "button" | "reset" | "submit";
}) {
| function NavigationMenuIndicator({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof NavigationMenuPrimitive.Indicator>) { | ||
| }: React.ComponentProps<typeof NavigationMenuPrimitive.Icon>) { | ||
| return ( | ||
| <NavigationMenuPrimitive.Indicator | ||
| <NavigationMenuPrimitive.Icon | ||
| className={cn( | ||
| "data-[state=hidden]:animate-out data-[state=hidden]:fade-out data-[state=visible]:animate-in data-[state=visible]:fade-in top-full z-1 flex h-1.5 items-end justify-center overflow-hidden", | ||
| "top-full z-1 flex h-1.5 items-end justify-center overflow-hidden", | ||
| className, | ||
| )} | ||
| data-slot="navigation-menu-indicator" | ||
| {...props} | ||
| > | ||
| <div className="bg-border relative top-[60%] h-2 w-2 rotate-45 rounded-ss-sm shadow-md" /> | ||
| </NavigationMenuPrimitive.Indicator> | ||
| </NavigationMenuPrimitive.Icon> | ||
| ); | ||
| } |
There was a problem hiding this comment.
In the migration from Radix UI to Base UI, NavigationMenuIndicator has been updated to render NavigationMenuPrimitive.Icon instead of an indicator primitive.
In Base UI, NavigationMenu.Icon is designed to be placed inside NavigationMenu.Trigger to render visual cues (like carets) and does not possess the layout tracking logic of a floating indicator. If you want to render an arrow pointing to the active trigger, use NavigationMenu.Arrow inside NavigationMenu.Popup instead, or remove NavigationMenuIndicator entirely if it is no longer needed.
| <TooltipPrimitive.Provider | ||
| data-slot="tooltip-provider" | ||
| delay={delay} | ||
| {...props} | ||
| /> |
There was a problem hiding this comment.
| function Tooltip({ ...props }: TooltipPrimitive.Root.Props) { | ||
| return <TooltipPrimitive.Root {...props} />; | ||
| return <TooltipPrimitive.Root data-slot="tooltip" {...props} />; | ||
| } |
There was a problem hiding this comment.
In Base UI, Tooltip.Root is a logical wrapper component that manages state and does not render its own HTML element. Passing data-slot="tooltip" to it is invalid and will be ignored or cause TypeScript compilation errors.
| function Tooltip({ ...props }: TooltipPrimitive.Root.Props) { | |
| return <TooltipPrimitive.Root {...props} />; | |
| return <TooltipPrimitive.Root data-slot="tooltip" {...props} />; | |
| } | |
| function Tooltip({ ...props }: TooltipPrimitive.Root.Props) { | |
| return <TooltipPrimitive.Root {...props} />; | |
| } |
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?