pnpm dlx shadcn@latest add --all#546
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
softmarshmallow
changed the title
pnpm dlx shadcn@latest add --all
WalkthroughUpdates 30+ UI components in editor/components/ui directory with icon imports from lucide-react, adds size/orientation/spacing props, introduces new public components (Combobox, DirectionProvider, Popover subcomponents), refactors Form components from forwardRef to plain functions, migrates Radix Slot imports, rewrites NavigationMenu, and updates dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83b943320c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const SIDEBAR_WIDTH_MOBILE = "18rem"; | ||
| const SIDEBAR_WIDTH_ICON = "3rem"; | ||
| const SIDEBAR_KEYBOARD_SHORTCUT = "\\"; | ||
| const SIDEBAR_KEYBOARD_SHORTCUT = "b"; |
There was a problem hiding this comment.
Setting SIDEBAR_KEYBOARD_SHORTCUT to "b" causes the global keydown listener in SidebarProvider to intercept Cmd/Ctrl+B and call preventDefault() whenever the provider is mounted, which is broad in this repo (workbench/workspace layouts). That steals a standard shortcut users rely on (bold in editable content and browser bookmark toggle) and unexpectedly toggles the sidebar instead, so this should use a non-conflicting key (as before) or at least be disabled while typing in editable elements.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
editor/components/ui/direction.tsx (1)
6-18: Redundantdirectionalias —dirremains required even whendirectionis provided.Since
diris required inReact.ComponentProps<typeof Direction.DirectionProvider>, callers must always supplydireven if they intend to use thedirectionalias. This creates an API wherediris silently overridden bydirection, which is confusing:// Caller must pass dir even though direction wins — dir serves no purpose here <DirectionProvider dir="ltr" direction="rtl">…</DirectionProvider>If the alias exists for backward compatibility with a previous internal API, consider making
diroptional in the combined type:♻️ Suggested API cleanup
function DirectionProvider({ dir, direction, children, -}: React.ComponentProps<typeof Direction.DirectionProvider> & { +}: Omit<React.ComponentProps<typeof Direction.DirectionProvider>, "dir"> & { + dir?: React.ComponentProps<typeof Direction.DirectionProvider>["dir"]; direction?: React.ComponentProps<typeof Direction.DirectionProvider>["dir"]; }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/direction.tsx` around lines 6 - 18, The DirectionProvider wrapper currently requires dir because of React.ComponentProps<typeof Direction.DirectionProvider>, but also accepts a direction alias that overrides dir, causing callers to be forced to pass an unused prop; update the prop typing so dir is optional and direction remains supported: change the combined prop type for the DirectionProvider function (the intersection using React.ComponentProps<typeof Direction.DirectionProvider>) so that the dir property is optional (for example by Omit-ing dir from the component props and re-adding dir?: ..., or by making dir?: in the inline extension) and keep direction?: React.ComponentProps<typeof Direction.DirectionProvider>["dir"]; then keep the implementation using dir ?? direction to preserve behavior.editor/components/ui/sidebar.tsx (1)
186-204:[&>button]:hiddenworkaround can be replaced with the newshowCloseButtonprop.Now that
SheetContentexposesshowCloseButton, the CSS hack on Line 190 is no longer needed:♻️ Suggested cleanup
<SheetContent data-sidebar="sidebar" data-slot="sidebar" data-mobile="true" - className="bg-sidebar text-sidebar-foreground w-(--sidebar-width) p-0 [&>button]:hidden" + className="bg-sidebar text-sidebar-foreground w-(--sidebar-width) p-0" + showCloseButton={false} style={{ "--sidebar-width": SIDEBAR_WIDTH_MOBILE } as React.CSSProperties} side={side} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/sidebar.tsx` around lines 186 - 204, Remove the CSS workaround that hides the close button by deleting the "[&>button]:hidden" part of the className on the SheetContent component and instead pass the new showCloseButton={false} prop to SheetContent; update the SheetContent JSX (referencing SheetContent and its className and props) so the mobile sidebar no longer relies on the CSS hack and uses the showCloseButton prop to suppress the close button.editor/components/ui/item.tsx (1)
43-43: Nit: trailing space in size variant string.
"p-4 gap-4 "has a trailing space. Harmless, but inconsistent with other variant strings.Proposed fix
- default: "p-4 gap-4 ", + default: "p-4 gap-4",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/item.tsx` at line 43, Remove the trailing space in the size variant default string to match other variant strings; locate the variant definition where the key default is set to "p-4 gap-4 " (in editor/components/ui/item.tsx) and change it to "p-4 gap-4" so the variant values are consistent.editor/components/ui/chart.tsx (1)
182-247: Minor:key={item.dataKey}may collide when multiple series share the samedataKey.If the chart has multiple payload entries with the same
dataKey(e.g., stacked series), React will warn about duplicate keys. Consider incorporatingindexas a fallback.Proposed fix
- key={item.dataKey} + key={`${item.dataKey}-${index}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/chart.tsx` around lines 182 - 247, The map over payload uses key={item.dataKey} which can collide for duplicate dataKey values; update the key generation in the payload.map callback (where key is currently derived) to include a stable fallback such as the index or the already-computed key variable (e.g., use `${item.dataKey}-${index}` or `${key}-${index}`) so each returned <div> in that map has a unique React key and avoids duplicate key warnings.editor/components/ui/dialog.tsx (1)
94-118: Close button renders afterchildrenin source order — verify visual placement inflex-col-reverse.With
flex-col-reverseon mobile, theClosebutton (source-last) will appear visually above thechildren. Onsm:flex-rowit appears to the right. If the intent is for the close button to sit on the leading (left) side of the footer row, this is correct. Otherwise, swap the render order.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/dialog.tsx` around lines 94 - 118, The DialogFooter currently renders {children} before the optional Close button (rendered via DialogPrimitive.Close and Button) while its container uses "flex flex-col-reverse gap-2 sm:flex-row sm:justify-end", which causes the Close button to appear above children on small screens; if the intended visual placement is to have the Close button appear after children on mobile, swap the render order so the Close button is rendered before {children}, otherwise leave as-is—update the JSX in DialogFooter accordingly and verify behavior of showCloseButton, children, and the container className.editor/components/ui/combobox.tsx (2)
92-127:data-chips={!!anchor}is an implicit heuristic.Line 117 ties the
data-chipsattribute to the presence ofanchor, assuming that providing an anchor means chip mode. This coupling may surprise consumers who useanchorfor positioning without chips. Consider making this an explicit prop or documenting the behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/combobox.tsx` around lines 92 - 127, ComboboxContent currently infers chips mode by setting data-chips={!!anchor}, which couples positioning (anchor) with presentation; change the API to accept an explicit chips?: boolean prop (in the ComboboxContent parameter list and its props type) and use that prop for data-chips (falling back to false if undefined) instead of deriving from anchor so consumers can pass anchor without enabling chips, and update any call sites/docs to pass the new chips prop where chip behavior is desired; locate the change around the ComboboxContent function and the data-chips attribute in the JSX.
227-242: Redundant type intersection onComboboxChipsprops.
React.ComponentPropsWithRef<typeof ComboboxPrimitive.Chips>andComboboxPrimitive.Chips.Propsshould resolve to the same type. The intersection is harmless but unnecessary.Suggested simplification
function ComboboxChips({ className, ...props -}: React.ComponentPropsWithRef<typeof ComboboxPrimitive.Chips> & - ComboboxPrimitive.Chips.Props) { +}: ComboboxPrimitive.Chips.Props) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/components/ui/combobox.tsx` around lines 227 - 242, The props type for ComboboxChips is redundant: remove the intersection and use a single props type (either React.ComponentPropsWithRef<typeof ComboboxPrimitive.Chips> or ComboboxPrimitive.Chips.Props) for the function signature of ComboboxChips; update the parameter typing in the ComboboxChips declaration so it only references one of those types (keep the rest/spread of props and className handling unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@editor/components/ui/avatar.tsx`:
- Around line 57-71: AvatarBadge's ring (ring-2 ring-background) is getting
clipped because its parent Avatar uses overflow-hidden rounded-full; to fix,
move the positioning container so the badge is placed in a sibling
absolutely-positioned wrapper that does not have overflow-hidden (keep
overflow-hidden only on the image container), or switch the visual separator
from a ring (box-shadow) to an outline on AvatarBadge (use outline styles
instead of ring) so it isn't clipped; update the Avatar component structure and
the AvatarBadge usage accordingly (look for Avatar and AvatarBadge symbols and
the overflow-hidden / rounded-full wrapper and adjust the container or style).
In `@editor/components/ui/chart.tsx`:
- Around line 237-241: The tooltip hides zero values because the JSX uses a
falsy check `{item.value && (...)}`; change this to an explicit null/undefined
check (e.g., `item.value != null` or `item.value !== null && item.value !==
undefined`) in the component rendering the value so that 0 is rendered but
null/undefined still hides it; update the conditional around the span in
chart.tsx (the block using `item.value.toLocaleString()`) accordingly to
preserve formatting while allowing zero to display.
In `@editor/components/ui/form.tsx`:
- Around line 28-30: The context default value makes the guard in useFormField
ineffective: update FormFieldContext/useFormField so the runtime check actually
detects misuse of useFormField outside a <FormField>. Either revert
FormFieldContext to default null (change the createContext call for
FormFieldContext and adjust types so useFormField narrows with if
(!fieldContext) throw) or keep the non-null default but change the guard in
useFormField to check a specific required property (e.g., if
(!fieldContext?.name) throw new Error(...)); ensure the thrown error references
useFormField/FormFieldContext/FormField so callers get a clear message.
In `@editor/components/ui/popover.tsx`:
- Around line 58-66: PopoverTitle currently types its props as
React.ComponentProps<"h2"> but renders a <div>, causing a type/element mismatch;
fix by either (A) rendering an <h2> instead of the <div> so the DOM element
matches the typed props (update the JSX root element in PopoverTitle to h2) or
(B) change the props type to React.ComponentProps<"div"> if you intend to keep a
div; update the signature of PopoverTitle and keep className, data-slot and
spread props consistent with the chosen element.
In `@editor/components/ui/sidebar.tsx`:
- Line 33: The change of SIDEBAR_KEYBOARD_SHORTCUT from "\" to "b" is a breaking
UX change; update toggleSidebar and related keyboard handling to preserve
backward compatibility by supporting both shortcuts (keep
SIDEBAR_KEYBOARD_SHORTCUT as "b" but accept "\" as an alternate trigger), emit a
one-time in-app notification or console.info when the new shortcut is used to
inform users of the change, and ensure keyboard handlers check for focused
editable elements (contentEditable/textarea) so toggleSidebar does not intercept
Ctrl/Cmd+B used for bold formatting; also add a short audit list (or comments)
near the keyboard binding code to check other editor surfaces where Ctrl/Cmd+B
may be handled.
In `@editor/components/ui/slider.tsx`:
- Line 56: The thumb's className currently hardcodes bg-white which breaks dark
mode and custom themes; update the className on the slider thumb (the element
with className containing "border-primary ring-ring/50 ... bg-white ...") to use
the semantic token bg-background instead of bg-white, or if the white thumb is
intentional add a brief comment documenting the design choice and why it should
not follow --background. Ensure no other instances of bg-white remain on this
component so the thumb follows theme tokens like ring-ring and border-primary.
In `@editor/components/ui/sonner.tsx`:
- Around line 3-11: The file uses a React namespace type assertion (} as
React.CSSProperties) but lacks the React namespace import, causing a compile
error; add the namespace import (import * as React from "react") at the top of
editor/components/ui/sonner.tsx so React.CSSProperties resolves, or
alternatively import the specific type (import type { CSSProperties } from
"react") and change the assertion to use CSSProperties; update the file where
the assertion appears (related to the Sonner/Toaster usage) so the type
reference matches the chosen import.
In `@editor/components/ui/toggle-group.tsx`:
- Around line 36-39: The data-[spacing=default] selector is incorrect because
spacing is a number; update the ToggleGroup className to use numeric data
selector matching the prop (e.g., replace
data-[spacing=default]:data-[variant=outline]:shadow-xs with
data-[spacing=0]:data-[variant=outline]:shadow-xs) so the shadow-xs is applied
when spacing is 0, consistent with ToggleGroupItem's data-[spacing=0] usage;
ensure you adjust any other similar selectors that expect "default" to the
correct numeric value or make the spacing prop a string if you intend to keep
the "default" token.
In `@editor/components/ui/tooltip.tsx`:
- Around line 24-25: Tooltip now renders TooltipPrimitive.Root directly and no
longer provides a TooltipProvider ancestor, so update call sites to ensure an
ancestor TooltipProvider exists; wrap top-level routes/layouts (where global
tooltips live) with TooltipProvider and add TooltipProvider around localized
usages (e.g., in ai-elements/message.tsx and any component rendering Tooltip) so
delayDuration/skipDelayDuration remain coordinated, or alternatively add a
TooltipProvider wrapper in each component that previously relied on the old
auto-wrap behavior.
---
Nitpick comments:
In `@editor/components/ui/chart.tsx`:
- Around line 182-247: The map over payload uses key={item.dataKey} which can
collide for duplicate dataKey values; update the key generation in the
payload.map callback (where key is currently derived) to include a stable
fallback such as the index or the already-computed key variable (e.g., use
`${item.dataKey}-${index}` or `${key}-${index}`) so each returned <div> in that
map has a unique React key and avoids duplicate key warnings.
In `@editor/components/ui/combobox.tsx`:
- Around line 92-127: ComboboxContent currently infers chips mode by setting
data-chips={!!anchor}, which couples positioning (anchor) with presentation;
change the API to accept an explicit chips?: boolean prop (in the
ComboboxContent parameter list and its props type) and use that prop for
data-chips (falling back to false if undefined) instead of deriving from anchor
so consumers can pass anchor without enabling chips, and update any call
sites/docs to pass the new chips prop where chip behavior is desired; locate the
change around the ComboboxContent function and the data-chips attribute in the
JSX.
- Around line 227-242: The props type for ComboboxChips is redundant: remove the
intersection and use a single props type (either
React.ComponentPropsWithRef<typeof ComboboxPrimitive.Chips> or
ComboboxPrimitive.Chips.Props) for the function signature of ComboboxChips;
update the parameter typing in the ComboboxChips declaration so it only
references one of those types (keep the rest/spread of props and className
handling unchanged).
In `@editor/components/ui/dialog.tsx`:
- Around line 94-118: The DialogFooter currently renders {children} before the
optional Close button (rendered via DialogPrimitive.Close and Button) while its
container uses "flex flex-col-reverse gap-2 sm:flex-row sm:justify-end", which
causes the Close button to appear above children on small screens; if the
intended visual placement is to have the Close button appear after children on
mobile, swap the render order so the Close button is rendered before {children},
otherwise leave as-is—update the JSX in DialogFooter accordingly and verify
behavior of showCloseButton, children, and the container className.
In `@editor/components/ui/direction.tsx`:
- Around line 6-18: The DirectionProvider wrapper currently requires dir because
of React.ComponentProps<typeof Direction.DirectionProvider>, but also accepts a
direction alias that overrides dir, causing callers to be forced to pass an
unused prop; update the prop typing so dir is optional and direction remains
supported: change the combined prop type for the DirectionProvider function (the
intersection using React.ComponentProps<typeof Direction.DirectionProvider>) so
that the dir property is optional (for example by Omit-ing dir from the
component props and re-adding dir?: ..., or by making dir?: in the inline
extension) and keep direction?: React.ComponentProps<typeof
Direction.DirectionProvider>["dir"]; then keep the implementation using dir ??
direction to preserve behavior.
In `@editor/components/ui/item.tsx`:
- Line 43: Remove the trailing space in the size variant default string to match
other variant strings; locate the variant definition where the key default is
set to "p-4 gap-4 " (in editor/components/ui/item.tsx) and change it to "p-4
gap-4" so the variant values are consistent.
In `@editor/components/ui/sidebar.tsx`:
- Around line 186-204: Remove the CSS workaround that hides the close button by
deleting the "[&>button]:hidden" part of the className on the SheetContent
component and instead pass the new showCloseButton={false} prop to SheetContent;
update the SheetContent JSX (referencing SheetContent and its className and
props) so the mobile sidebar no longer relies on the CSS hack and uses the
showCloseButton prop to suppress the close button.
| function AvatarBadge({ className, ...props }: React.ComponentProps<"span">) { | ||
| return ( | ||
| <span | ||
| data-slot="avatar-badge" | ||
| className={cn( | ||
| "bg-primary text-primary-foreground ring-background absolute right-0 bottom-0 z-10 inline-flex items-center justify-center rounded-full ring-2 select-none", | ||
| "group-data-[size=sm]/avatar:size-2 group-data-[size=sm]/avatar:[&>svg]:hidden", | ||
| "group-data-[size=default]/avatar:size-2.5 group-data-[size=default]/avatar:[&>svg]:size-2", | ||
| "group-data-[size=lg]/avatar:size-3 group-data-[size=lg]/avatar:[&>svg]:size-2", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
AvatarBadge ring (ring-2) will be clipped by the parent Avatar's overflow-hidden.
AvatarBadge relies on ring-2 ring-background (rendered as box-shadow) to visually separate the badge from the avatar image. However, its parent Avatar has overflow-hidden rounded-full, and CSS overflow: hidden clips the box-shadow of child elements that extend beyond the clipping boundary. The ring at the circular edge will be clipped.
If the intention is a visible ring (separation between badge and avatar image), consider one of:
- Wrapping Avatar + Badge in a separate positioning container (no
overflow-hidden) while keepingoverflow-hiddenon the image container only. - Using
outlineinstead ofring(outlines are not clipped byoverflow: hidden). - Accepting the clip as intentional for this design (badge blends cleanly into the avatar circle).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/avatar.tsx` around lines 57 - 71, AvatarBadge's ring
(ring-2 ring-background) is getting clipped because its parent Avatar uses
overflow-hidden rounded-full; to fix, move the positioning container so the
badge is placed in a sibling absolutely-positioned wrapper that does not have
overflow-hidden (keep overflow-hidden only on the image container), or switch
the visual separator from a ring (box-shadow) to an outline on AvatarBadge (use
outline styles instead of ring) so it isn't clipped; update the Avatar component
structure and the AvatarBadge usage accordingly (look for Avatar and AvatarBadge
symbols and the overflow-hidden / rounded-full wrapper and adjust the container
or style).
| {item.value && ( | ||
| <span className="text-foreground font-mono font-medium tabular-nums"> | ||
| {item.value.toLocaleString()} | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
Bug: tooltip value of 0 is hidden due to falsy check.
{item.value && (…)} evaluates to false when item.value is 0, which is a perfectly valid chart data point. The value column will silently disappear for zero-valued items.
Proposed fix
- {item.value && (
+ {item.value != null && (
<span className="text-foreground font-mono font-medium tabular-nums">
{item.value.toLocaleString()}
</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.
| {item.value && ( | |
| <span className="text-foreground font-mono font-medium tabular-nums"> | |
| {item.value.toLocaleString()} | |
| </span> | |
| )} | |
| {item.value != null && ( | |
| <span className="text-foreground font-mono font-medium tabular-nums"> | |
| {item.value.toLocaleString()} | |
| </span> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/chart.tsx` around lines 237 - 241, The tooltip hides
zero values because the JSX uses a falsy check `{item.value && (...)}`; change
this to an explicit null/undefined check (e.g., `item.value != null` or
`item.value !== null && item.value !== undefined`) in the component rendering
the value so that 0 is rendered but null/undefined still hides it; update the
conditional around the span in chart.tsx (the block using
`item.value.toLocaleString()`) accordingly to preserve formatting while allowing
zero to display.
| const FormFieldContext = React.createContext<FormFieldContextValue>( | ||
| {} as FormFieldContextValue | ||
| ); |
There was a problem hiding this comment.
Dead guard: if (!fieldContext) never throws after the default-value change.
FormFieldContext now defaults to {} as FormFieldContextValue (line 28-30), which is truthy. The check on line 52 (if (!fieldContext)) was previously effective when the default was null, but now the empty object always passes the check. Using useFormField outside a <FormField> will silently proceed with fieldContext.name being undefined instead of throwing the intended error.
Suggested fix — restore the guard
Option A: Check the property rather than the object:
- if (!fieldContext) {
+ if (!fieldContext.name) {
throw new Error("useFormField should be used within <FormField>");
}Option B: Revert to null default and add a type-narrowing check:
-const FormFieldContext = React.createContext<FormFieldContextValue>(
- {} as FormFieldContextValue
-);
+const FormFieldContext = React.createContext<FormFieldContextValue | null>(
+ null
+);
const useFormField = () => {
const fieldContext = React.useContext(FormFieldContext);
+ if (!fieldContext) {
+ throw new Error("useFormField should be used within <FormField>");
+ }
const itemContext = React.useContext(FormItemContext);
const { getFieldState } = useFormContext();
const formState = useFormState({ name: fieldContext.name });
const fieldState = getFieldState(fieldContext.name, formState);
-
- if (!fieldContext) {
- throw new Error("useFormField should be used within <FormField>");
- }Also applies to: 45-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/form.tsx` around lines 28 - 30, The context default
value makes the guard in useFormField ineffective: update
FormFieldContext/useFormField so the runtime check actually detects misuse of
useFormField outside a <FormField>. Either revert FormFieldContext to default
null (change the createContext call for FormFieldContext and adjust types so
useFormField narrows with if (!fieldContext) throw) or keep the non-null default
but change the guard in useFormField to check a specific required property
(e.g., if (!fieldContext?.name) throw new Error(...)); ensure the thrown error
references useFormField/FormFieldContext/FormField so callers get a clear
message.
| function PopoverTitle({ className, ...props }: React.ComponentProps<"h2">) { | ||
| return ( | ||
| <div | ||
| data-slot="popover-title" | ||
| className={cn("font-medium", className)} | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Type/element mismatch: props typed as <h2> but renders a <div>.
PopoverTitle accepts React.ComponentProps<"h2"> yet renders a <div>. This misleads consumers and TypeScript — h2-specific attributes would type-check but never reach an actual <h2>. Either render an <h2> or change the props type to "div".
Option A: render an h2
function PopoverTitle({ className, ...props }: React.ComponentProps<"h2">) {
return (
- <div
+ <h2
data-slot="popover-title"
className={cn("font-medium", className)}
{...props}
- />
+ />
);
}Option B: type as div
-function PopoverTitle({ className, ...props }: React.ComponentProps<"h2">) {
+function PopoverTitle({ className, ...props }: React.ComponentProps<"div">) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/popover.tsx` around lines 58 - 66, PopoverTitle
currently types its props as React.ComponentProps<"h2"> but renders a <div>,
causing a type/element mismatch; fix by either (A) rendering an <h2> instead of
the <div> so the DOM element matches the typed props (update the JSX root
element in PopoverTitle to h2) or (B) change the props type to
React.ComponentProps<"div"> if you intend to keep a div; update the signature of
PopoverTitle and keep className, data-slot and spread props consistent with the
chosen element.
| const SIDEBAR_WIDTH_MOBILE = "18rem"; | ||
| const SIDEBAR_WIDTH_ICON = "3rem"; | ||
| const SIDEBAR_KEYBOARD_SHORTCUT = "\\"; | ||
| const SIDEBAR_KEYBOARD_SHORTCUT = "b"; |
There was a problem hiding this comment.
SIDEBAR_KEYBOARD_SHORTCUT change from \ to b is a breaking UX change for existing users.
The new shortcut Ctrl+B / Cmd+B is a well-known convention (VS Code's sidebar toggle), which is a reasonable motivation for the change. However:
- Existing users who have muscle-memory for
Ctrl+\will be silently broken with no in-product hint. Ctrl+B/Cmd+Bis also the bold shortcut in most text editors and rich-text inputs. If the sidebar is visible while a contentEditable/textarea is focused, this will conflict.
Consider logging a console note or adding a toast/notification on first use, and audit any editor surfaces where Ctrl+B for bold may be intercepted by toggleSidebar first.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/sidebar.tsx` at line 33, The change of
SIDEBAR_KEYBOARD_SHORTCUT from "\" to "b" is a breaking UX change; update
toggleSidebar and related keyboard handling to preserve backward compatibility
by supporting both shortcuts (keep SIDEBAR_KEYBOARD_SHORTCUT as "b" but accept
"\" as an alternate trigger), emit a one-time in-app notification or
console.info when the new shortcut is used to inform users of the change, and
ensure keyboard handlers check for focused editable elements
(contentEditable/textarea) so toggleSidebar does not intercept Ctrl/Cmd+B used
for bold formatting; also add a short audit list (or comments) near the keyboard
binding code to check other editor surfaces where Ctrl/Cmd+B may be handled.
| data-slot="slider-thumb" | ||
| key={index} | ||
| className="border-primary bg-background ring-ring/50 block size-4 shrink-0 rounded-full border shadow-sm transition-[color,box-shadow] hover:ring-4 focus-visible:ring-4 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50" | ||
| className="border-primary ring-ring/50 block size-4 shrink-0 rounded-full border bg-white shadow-sm transition-[color,box-shadow] hover:ring-4 focus-visible:ring-4 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50" |
There was a problem hiding this comment.
bg-white hardcodes the thumb color — dark mode / custom themes are not respected.
Every other color on this component (border-primary, ring-ring/50) uses semantic CSS tokens that adapt to the active theme. bg-white will render a white thumb in dark mode as well, regardless of --background.
If this is intentional for the new-york aesthetic (white thumb against any background), it should be documented. Otherwise, revert to bg-background:
🎨 Proposed fix
- className="border-primary ring-ring/50 block size-4 shrink-0 rounded-full border bg-white shadow-sm transition-[color,box-shadow] hover:ring-4 focus-visible:ring-4 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50"
+ className="border-primary ring-ring/50 block size-4 shrink-0 rounded-full border bg-background shadow-sm transition-[color,box-shadow] hover:ring-4 focus-visible:ring-4 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50"📝 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.
| className="border-primary ring-ring/50 block size-4 shrink-0 rounded-full border bg-white shadow-sm transition-[color,box-shadow] hover:ring-4 focus-visible:ring-4 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50" | |
| className="border-primary ring-ring/50 block size-4 shrink-0 rounded-full border bg-background shadow-sm transition-[color,box-shadow] hover:ring-4 focus-visible:ring-4 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/slider.tsx` at line 56, The thumb's className currently
hardcodes bg-white which breaks dark mode and custom themes; update the
className on the slider thumb (the element with className containing
"border-primary ring-ring/50 ... bg-white ...") to use the semantic token
bg-background instead of bg-white, or if the white thumb is intentional add a
brief comment documenting the design choice and why it should not follow
--background. Ensure no other instances of bg-white remain on this component so
the thumb follows theme tokens like ring-ring and border-primary.
| import { | ||
| CircleCheckIcon, | ||
| InfoIcon, | ||
| Loader2Icon, | ||
| OctagonXIcon, | ||
| TriangleAlertIcon, | ||
| } from "lucide-react"; | ||
| import { useTheme } from "next-themes"; | ||
| import { Toaster as Sonner, ToasterProps } from "sonner"; | ||
| import { Toaster as Sonner, type ToasterProps } from "sonner"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm React namespace usage without import in sonner.tsx
echo "=== React imports in sonner.tsx ==="
rg -n "import.*React" editor/components/ui/sonner.tsx || echo "(none found)"
echo ""
echo "=== React.CSSProperties or React.* usages ==="
rg -n "React\." editor/components/ui/sonner.tsx
echo ""
echo "=== Full file content to verify ==="
cat -n editor/components/ui/sonner.tsxRepository: gridaco/grida
Length of output: 1547
Missing React import — React.CSSProperties on line 33 will fail to compile.
The file uses } as React.CSSProperties (line 33) but has no import * as React from "react". The new JSX transform removes the need for React in scope for JSX expressions, but type assertions referencing React.* still require the namespace import. Every other file changed in this PR includes this import; its absence here is almost certainly a shadcn@latest codegen regression.
🐛 Proposed fix
Option A — restore the namespace import (minimal change):
"use client";
+import * as React from "react";
+
import {
CircleCheckIcon,Option B — import only the needed type (more precise):
import { Toaster as Sonner, type ToasterProps } from "sonner";
+import type { CSSProperties } from "react";- } as React.CSSProperties
+ } as CSSProperties📝 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.
| import { | |
| CircleCheckIcon, | |
| InfoIcon, | |
| Loader2Icon, | |
| OctagonXIcon, | |
| TriangleAlertIcon, | |
| } from "lucide-react"; | |
| import { useTheme } from "next-themes"; | |
| import { Toaster as Sonner, ToasterProps } from "sonner"; | |
| import { Toaster as Sonner, type ToasterProps } from "sonner"; | |
| import * as React from "react"; | |
| import { | |
| CircleCheckIcon, | |
| InfoIcon, | |
| Loader2Icon, | |
| OctagonXIcon, | |
| TriangleAlertIcon, | |
| } from "lucide-react"; | |
| import { useTheme } from "next-themes"; | |
| import { Toaster as Sonner, type ToasterProps } from "sonner"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/sonner.tsx` around lines 3 - 11, The file uses a React
namespace type assertion (} as React.CSSProperties) but lacks the React
namespace import, causing a compile error; add the namespace import (import * as
React from "react") at the top of editor/components/ui/sonner.tsx so
React.CSSProperties resolves, or alternatively import the specific type (import
type { CSSProperties } from "react") and change the assertion to use
CSSProperties; update the file where the assertion appears (related to the
Sonner/Toaster usage) so the type reference matches the chosen import.
| data-spacing={spacing} | ||
| style={{ "--gap": spacing } as React.CSSProperties} | ||
| className={cn( | ||
| "group/toggle-group flex w-fit items-center rounded-md data-[variant=outline]:shadow-xs", | ||
| "group/toggle-group flex w-fit items-center gap-[--spacing(var(--gap))] rounded-md data-[spacing=default]:data-[variant=outline]:shadow-xs", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n editor/components/ui/toggle-group.tsx | head -50Repository: gridaco/grida
Length of output: 1693
🏁 Script executed:
rg -n "spacing" editor/components/ui/toggle-group.tsx -A 2 -B 2Repository: gridaco/grida
Length of output: 1588
🏁 Script executed:
rg -n "data-\[spacing=" --type tsx --type tsRepository: gridaco/grida
Length of output: 84
data-[spacing=default] selector will never match — spacing is typed as number.
The spacing prop is number (default 0), so data-spacing receives numeric values (e.g., "0", "1"). The selector data-[spacing=default] on line 39 will never match, preventing the shadow-xs from applying to the outline variant when items are grouped (spacing = 0). This is inconsistent with line 73 in ToggleGroupItem, which correctly uses data-[spacing=0] for numeric selectors.
Fix
className={cn(
- "group/toggle-group flex w-fit items-center gap-[--spacing(var(--gap))] rounded-md data-[spacing=default]:data-[variant=outline]:shadow-xs",
+ "group/toggle-group flex w-fit items-center gap-[--spacing(var(--gap))] rounded-md data-[spacing=0]:data-[variant=outline]:shadow-xs",
className
)}📝 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.
| data-spacing={spacing} | |
| style={{ "--gap": spacing } as React.CSSProperties} | |
| className={cn( | |
| "group/toggle-group flex w-fit items-center rounded-md data-[variant=outline]:shadow-xs", | |
| "group/toggle-group flex w-fit items-center gap-[--spacing(var(--gap))] rounded-md data-[spacing=default]:data-[variant=outline]:shadow-xs", | |
| data-spacing={spacing} | |
| style={{ "--gap": spacing } as React.CSSProperties} | |
| className={cn( | |
| "group/toggle-group flex w-fit items-center gap-[--spacing(var(--gap))] rounded-md data-[spacing=0]:data-[variant=outline]:shadow-xs", | |
| className | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/toggle-group.tsx` around lines 36 - 39, The
data-[spacing=default] selector is incorrect because spacing is a number; update
the ToggleGroup className to use numeric data selector matching the prop (e.g.,
replace data-[spacing=default]:data-[variant=outline]:shadow-xs with
data-[spacing=0]:data-[variant=outline]:shadow-xs) so the shadow-xs is applied
when spacing is 0, consistent with ToggleGroupItem's data-[spacing=0] usage;
ensure you adjust any other similar selectors that expect "default" to the
correct numeric value or make the spacing prop a string if you intend to keep
the "default" token.
| return <TooltipPrimitive.Root data-slot="tooltip" {...props} />; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the tooltip.tsx file
cat -n editor/components/ui/tooltip.tsx | head -60Repository: gridaco/grida
Length of output: 2311
🏁 Script executed:
# Check if SidebarProvider uses TooltipProvider
cat -n editor/components/ui/sidebar.tsx | grep -A 5 -B 5 "TooltipProvider"Repository: gridaco/grida
Length of output: 1065
🏁 Script executed:
# Search for Tooltip/TooltipContent usages in the codebase
rg -n "TooltipContent|<Tooltip" --type tsx --type ts editor/ | head -50Repository: gridaco/grida
Length of output: 84
🏁 Script executed:
# Search for Tooltip usages with correct syntax
rg "TooltipContent|TooltipTrigger" --type-list | grep -i tsxRepository: gridaco/grida
Length of output: 446
🏁 Script executed:
# Use proper file extension matching
rg -t ts -t tsx "TooltipContent|TooltipTrigger" editor/ | head -60Repository: gridaco/grida
Length of output: 84
🏁 Script executed:
# Also check the imports to understand usage patterns
rg "from.*tooltip" -i --type ts --type tsx editor/ | head -40Repository: gridaco/grida
Length of output: 84
🏁 Script executed:
# Search for TooltipContent and TooltipTrigger usage without type flags
rg "TooltipContent|TooltipTrigger" editor/ --glob "*.tsx" --glob "*.ts" | head -80Repository: gridaco/grida
Length of output: 8982
🏁 Script executed:
# Also check if there are imports from tooltip in app directories
rg "from.*tooltip" editor/ --glob "*.tsx" --glob "*.ts" | head -40Repository: gridaco/grida
Length of output: 3857
🏁 Script executed:
# Check if TooltipProvider is imported and used outside sidebar
rg "TooltipProvider" editor/ --glob "*.tsx" --glob "*.ts" -B 2 -A 2 | head -100Repository: gridaco/grida
Length of output: 5811
Tooltip no longer auto-wraps with TooltipProvider — ensure ancestor provider exists in call sites.
Previously, the Tooltip component auto-wrapped itself with TooltipProvider. Now it renders TooltipPrimitive.Root directly. TooltipProvider controls shared delayDuration and skipDelayDuration behavior — without an ancestor provider, tooltips lose coordinated delay (rapid mouse movement between tooltips will re-trigger the delay each time instead of skipping it).
Most app routes already wrap with <TooltipProvider> at the layout level, and components like ai-elements/message.tsx explicitly wrap their tooltip usage. However, this is now an implicit contract: any <Tooltip> component must have an ancestor <TooltipProvider> or it will render but lose the shared delay coordination. This is not enforced at the component level and could silently break for tooltips added in unexpected contexts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/components/ui/tooltip.tsx` around lines 24 - 25, Tooltip now renders
TooltipPrimitive.Root directly and no longer provides a TooltipProvider
ancestor, so update call sites to ensure an ancestor TooltipProvider exists;
wrap top-level routes/layouts (where global tooltips live) with TooltipProvider
and add TooltipProvider around localized usages (e.g., in
ai-elements/message.tsx and any component rendering Tooltip) so
delayDuration/skipDelayDuration remain coordinated, or alternatively add a
TooltipProvider wrapper in each component that previously relied on the old
auto-wrap behavior.
1 participant
regular new-york theme update
Summary by CodeRabbit
Release Notes
New Features
UI Improvements
Updates