chore: Apply shadcn latest styling changes to components#722
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds AlertAction component and exports; broad Tailwind class refactors across many UI components (alert, avatar, button, dropdown, label, scroll-area, separator, sheet, dialog, tooltip); tightens ButtonLink prop types and removes its Disabled story; adds forceRenderOverlay prop to ConfirmResponsiveDrawer; small presentation tweaks in devtools hints. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 2
🧹 Nitpick comments (5)
src/components/ui/dialog.tsx (2)
140-140: DialogTitle font size may be smaller than before.Removing
text-lgmeans the title will inherit the parent's font size (likelytext-smfrom DialogContent at line 60). Combined with changing fromfont-semiboldtofont-medium, titles will appear noticeably smaller and lighter.If this is intentional per shadcn updates, the change is fine. Otherwise, consider keeping a size class for visual hierarchy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/dialog.tsx` at line 140, DialogTitle lost its explicit size and weight by removing "text-lg" and changing "font-semibold" to "font-medium", causing titles to inherit the parent small size; restore an explicit size (e.g., add "text-lg") and/or the stronger weight (e.g., "font-semibold") in the DialogTitle className (the string passed to cn in the DialogTitle component) so the title keeps its intended visual hierarchy.
33-36: Notable visual change: significantly lighter overlay.The backdrop changes from
bg-black/80(80% opacity) tobg-black/10(10% opacity) with backdrop blur. This is a substantial visual shift that makes the overlay much more transparent.While this aligns with modern design trends favoring lighter overlays with blur effects, verify this works well across different background content (especially colorful or busy backgrounds) where a very light overlay might not provide sufficient visual separation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/dialog.tsx` around lines 33 - 36, The overlay opacity change in the dialog component (the className passed into cn in src/components/ui/dialog.tsx for the dialog backdrop) moved from bg-black/80 to bg-black/10 making the backdrop much lighter; either restore the previous opacity (bg-black/80) or make the overlay opacity configurable (add a prop like overlayOpacity or variant and use it to choose bg-black/10 / bg-black/40 / bg-black/80) so callers can pick appropriate contrast for busy backgrounds; update the class assembly where cn(...) is called (the backdrop className) and adjust any consuming code/tests to pass the new prop or keep the default at the higher-opacity value.src/components/ui/tooltip.tsx (1)
6-6: Consider UX implications of zero delay for tooltips.Reducing delay from 500ms to 0ms means tooltips appear immediately on hover. While this provides a snappier feel, it may cause visual clutter in dense UI layouts (e.g., input groups with info icons, help buttons in forms) where users might trigger tooltips unintentionally while moving their cursor.
If this is intentional to match shadcn's latest defaults, consider documenting this behavior change. Individual components can still override via
<TooltipProvider delay={500}>if needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/tooltip.tsx` at line 6, The default tooltip delay was changed to 0 (in the delay default in src/components/ui/tooltip.tsx), which makes tooltips appear immediately and may cause unwanted visual clutter; either revert the default to 500ms or add a clear comment/docstring near the TooltipProvider/tooltip delay default explaining the intentional immediate-show behavior and how components can override it (e.g., use <TooltipProvider delay={500}>); update docs/README or inline comment to note the behavioral change so consumers know to override per-component if needed.src/components/ui/label.tsx (1)
7-7: Note:items-baseline→items-centeralignment change.Changing from
items-baselinetoitems-centermay affect how labels align with adjacent content (icons, badges, or multi-line text). If labels with icons previously aligned to text baseline, they will now center-align. This is likely intentional for the new styling, but worth noting for visual regression testing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/label.tsx` at line 7, The utility class for the Label component was changed from items-baseline to items-center which will center-align label content (icons/badges/multiline text) instead of aligning to the text baseline; if baseline alignment is required restore the original class in the Label component by replacing 'items-center' with 'items-baseline' in the class string (search for the class string in label.tsx or the Label component), otherwise confirm the intentional change and add/update visual/regression tests for the Label usage (icons, badges, multiline) to validate the new alignment across affected stories/components.src/components/ui/alert.tsx (1)
7-7: Consider making action layout resilient to variable-width content.
AlertActionis absolute while the container reserves fixed space (pr-18). Longer action labels can overlap content in tight widths. A grid-column action layout would be more robust.Optional layout refactor
- "group/alert relative grid w-full gap-0.5 rounded-lg border px-4 py-3 text-left text-sm has-data-[slot=alert-action]:relative has-data-[slot=alert-action]:pr-18 has-[>svg]:grid-cols-[auto_1fr] ..." + "group/alert relative grid w-full gap-0.5 rounded-lg border px-4 py-3 text-left text-sm has-data-[slot=alert-action]:grid-cols-[1fr_auto] has-[>svg]:grid-cols-[auto_1fr] ..." - className={cn('absolute top-2.5 right-3', className)} + className={cn('col-start-2 row-start-1 self-start justify-self-end', className)}Also applies to: 66-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/alert.tsx` at line 7, The container currently reserves fixed right padding (pr-18) while AlertAction is positioned absolute, which causes overlap for long action labels; update the Alert container and AlertAction components to use a grid-column layout instead of absolute positioning: remove the absolute positioning on AlertAction and the fixed padding token (pr-18) from the class string, add a responsive two-column grid (e.g., main content + auto-width action column) via the container's grid template or utility classes so the action occupies its own column, and ensure AlertAction is placed in that second column (update the class list referenced in the string literal and the AlertAction render so it uses grid column/auto sizing); apply the same change for the other similar block noted (lines 66-73).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/card.tsx`:
- Line 28: The Tailwind arbitrary variant selectors in the Card header class
string are targeting ancestors; update the self-targeting selectors to use the &
placeholder: replace '[.border-b]:pb-6' with '[&.border-b]:pb-6' and
'[.border-t]:pt-6' with '[&.border-t]:pt-6' in the class list used by the card
header (look for the 'group/card-header' class string in
src/components/ui/card.tsx) and make the same substitutions for the similar
class string near line 87.
In `@src/components/ui/label.tsx`:
- Line 7: The Tailwind classes 'group-data-[disabled=true]:pointer-events-none'
and 'group-data-[disabled=true]:opacity-50' in the label string in
src/components/ui/label.tsx are dead unless a parent uses both the 'group' class
and a data-disabled="true" attribute; either remove these two classes from the
label class list or, if they're intentional for a planned change, add a short
TODO comment in the FormField component next to the div rendering (the one using
className={cn('flex flex-col gap-1.5', props.className)}) stating that FormField
should add 'group' and toggle data-disabled when disabled so those selectors
will be effective.
---
Nitpick comments:
In `@src/components/ui/alert.tsx`:
- Line 7: The container currently reserves fixed right padding (pr-18) while
AlertAction is positioned absolute, which causes overlap for long action labels;
update the Alert container and AlertAction components to use a grid-column
layout instead of absolute positioning: remove the absolute positioning on
AlertAction and the fixed padding token (pr-18) from the class string, add a
responsive two-column grid (e.g., main content + auto-width action column) via
the container's grid template or utility classes so the action occupies its own
column, and ensure AlertAction is placed in that second column (update the class
list referenced in the string literal and the AlertAction render so it uses grid
column/auto sizing); apply the same change for the other similar block noted
(lines 66-73).
In `@src/components/ui/dialog.tsx`:
- Line 140: DialogTitle lost its explicit size and weight by removing "text-lg"
and changing "font-semibold" to "font-medium", causing titles to inherit the
parent small size; restore an explicit size (e.g., add "text-lg") and/or the
stronger weight (e.g., "font-semibold") in the DialogTitle className (the string
passed to cn in the DialogTitle component) so the title keeps its intended
visual hierarchy.
- Around line 33-36: The overlay opacity change in the dialog component (the
className passed into cn in src/components/ui/dialog.tsx for the dialog
backdrop) moved from bg-black/80 to bg-black/10 making the backdrop much
lighter; either restore the previous opacity (bg-black/80) or make the overlay
opacity configurable (add a prop like overlayOpacity or variant and use it to
choose bg-black/10 / bg-black/40 / bg-black/80) so callers can pick appropriate
contrast for busy backgrounds; update the class assembly where cn(...) is called
(the backdrop className) and adjust any consuming code/tests to pass the new
prop or keep the default at the higher-opacity value.
In `@src/components/ui/label.tsx`:
- Line 7: The utility class for the Label component was changed from
items-baseline to items-center which will center-align label content
(icons/badges/multiline text) instead of aligning to the text baseline; if
baseline alignment is required restore the original class in the Label component
by replacing 'items-center' with 'items-baseline' in the class string (search
for the class string in label.tsx or the Label component), otherwise confirm the
intentional change and add/update visual/regression tests for the Label usage
(icons, badges, multiline) to validate the new alignment across affected
stories/components.
In `@src/components/ui/tooltip.tsx`:
- Line 6: The default tooltip delay was changed to 0 (in the delay default in
src/components/ui/tooltip.tsx), which makes tooltips appear immediately and may
cause unwanted visual clutter; either revert the default to 500ms or add a clear
comment/docstring near the TooltipProvider/tooltip delay default explaining the
intentional immediate-show behavior and how components can override it (e.g.,
use <TooltipProvider delay={500}>); update docs/README or inline comment to note
the behavioral change so consumers know to override per-component if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 33c1850c-42c5-46bc-aa0c-fbc70aa08fe5
📒 Files selected for processing (11)
src/components/ui/alert.tsxsrc/components/ui/avatar.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/label.tsxsrc/components/ui/scroll-area.tsxsrc/components/ui/separator.tsxsrc/components/ui/sheet.tsxsrc/components/ui/tooltip.tsx
| data-slot="card-header" | ||
| className={cn( | ||
| '@container/card-header grid auto-rows-min grid-rows-[auto_auto] items-start gap-1.5 px-4 has-data-[slot=card-action]:grid-cols-[1fr_auto] [.border-b]:pb-4', | ||
| 'group/card-header @container/card-header grid auto-rows-min items-start gap-1 rounded-t-xl px-6 group-data-[size=sm]/card:px-4 has-data-[slot=card-action]:grid-cols-[1fr_auto] has-data-[slot=card-description]:grid-rows-[auto_auto] [.border-b]:pb-6 group-data-[size=sm]/card:[.border-b]:pb-4', |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Tailwind CSS v4.1, for a class variant like [.border-b]:pb-6, does this target the element itself when it has class border-b, or should self-targeting use [&.border-b]:pb-6 instead?
💡 Result:
[.border-b]:pb-6 does not self-target — it generates a rule that applies to an element with the Tailwind class when it is inside an ancestor that matches .border-b (roughly: .border-b .\[\.border-b\]\:pb-6 { padding-bottom: ... }).
For “this same element also has border-b”, you should use the & placeholder: [&.border-b]:pb-6 (Tailwind’s docs describe arbitrary variants as selector format strings, and show self-targeting with & like [&.is-dragging]:...). [1]
Source: Tailwind “Using arbitrary variants” documentation. [1]
Use [&.border-b] and [&.border-t] for self-targeting in arbitrary Tailwind variants.
The selectors [.border-b]:pb-6 and [.border-t]:pt-6 target ancestors with those classes, not the component itself. For conditional spacing when this element has the border class, use the & placeholder: [&.border-b]:pb-6 and [&.border-t]:pt-6.
Required changes
- '... [.border-b]:pb-6 ...'
+ '... [&.border-b]:pb-6 ...'
- '... [.border-t]:pt-6 ...'
+ '... [&.border-t]:pt-6 ...'Also applies to: line 87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/card.tsx` at line 28, The Tailwind arbitrary variant
selectors in the Card header class string are targeting ancestors; update the
self-targeting selectors to use the & placeholder: replace '[.border-b]:pb-6'
with '[&.border-b]:pb-6' and '[.border-t]:pt-6' with '[&.border-t]:pt-6' in the
class list used by the card header (look for the 'group/card-header' class
string in src/components/ui/card.tsx) and make the same substitutions for the
similar class string near line 87.
|
|
||
| const labelVariants = cva( | ||
| 'flex items-baseline gap-1.5 text-sm peer-disabled:cursor-not-allowed peer-disabled:opacity-50' | ||
| 'flex items-center gap-2 text-sm leading-none font-medium select-none group-data-[disabled=true]:pointer-events-none group-data-[disabled=true]:opacity-50 peer-disabled:cursor-not-allowed peer-disabled:opacity-50' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usages of "group" class combined with data-disabled in form-related components
echo "=== Checking FormField for group class ==="
rg -n "group" src/components/form/form-field.tsx
echo ""
echo "=== Checking for data-disabled usage in form components ==="
rg -n "data-disabled" src/components/form/
echo ""
echo "=== Checking Label usage contexts that might have group + data-disabled ==="
rg -B5 -A5 "<Label" --type=tsx | head -100Repository: BearStudio/start-ui-web
Length of output: 292
🏁 Script executed:
#!/bin/bash
echo "=== Checking label.tsx content ==="
cat -n src/components/ui/label.tsx
echo ""
echo "=== Checking form-field.tsx content ==="
cat -n src/components/form/form-field.tsx
echo ""
echo "=== Searching for group-data-[disabled usage ==="
rg "group-data-\[disabled" .
echo ""
echo "=== Searching for data-disabled attribute ==="
rg "data-disabled" .
echo ""
echo "=== Searching for 'group' class in form components ==="
rg "group" src/components/form/ -A2 -B2
echo ""
echo "=== Checking if Label is wrapped by group-enabled components ==="
rg -B10 "Label" src/components/form/ | head -150Repository: BearStudio/start-ui-web
Length of output: 35449
🏁 Script executed:
#!/bin/bash
echo "=== Checking form-field-controller.tsx ==="
cat src/components/form/form-field-controller.tsx | head -80
echo ""
echo "=== Checking if any component adds group class to FormField ==="
rg "FormField" src/components/form/ -A3 | grep -E "(group|data-disabled)" | head -20Repository: BearStudio/start-ui-web
Length of output: 1996
🏁 Script executed:
#!/bin/bash
echo "=== Finding form-field-controller ==="
find src/components/form -name "*field-controller*" -type f
echo ""
echo "=== Checking FormFieldContainer ==="
find src/components/form -name "*form-field-container*" -type f | xargs cat -n
echo ""
echo "=== Search for where data-disabled or group might be added to form fields ==="
rg "disabled" src/components/form/form-field-container.tsx -B2 -A2Repository: BearStudio/start-ui-web
Length of output: 526
🏁 Script executed:
#!/bin/bash
echo "=== Checking if any component wraps FormField with group class ==="
rg -B5 "FormField" src/components/ | grep -E "(className.*group|group.*className)" | head -20
echo ""
echo "=== Checking how Label is actually used in practice (find complete examples) ==="
rg "FormFieldLabel" src/components/form/*/docs.stories.tsx -B3 -A3 | head -60Repository: BearStudio/start-ui-web
Length of output: 6079
Clarify if group-data-[disabled=true] selectors are intentional or dead code.
The new group-data-[disabled=true]:pointer-events-none and group-data-[disabled=true]:opacity-50 classes require a parent element with both the group Tailwind class and data-disabled="true" attribute. However, FormField (the primary container) only renders <div className={cn('flex flex-col gap-1.5', props.className)}> without these attributes. This means these selectors cannot activate in the standard form field context.
The peer-disabled selector will still work when an input element is disabled, but the group-data-[disabled=true] rules appear unused. If this is preparation for a future FormField update, add a TODO comment explaining the intent. Otherwise, remove these unused classes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/label.tsx` at line 7, The Tailwind classes
'group-data-[disabled=true]:pointer-events-none' and
'group-data-[disabled=true]:opacity-50' in the label string in
src/components/ui/label.tsx are dead unless a parent uses both the 'group' class
and a data-disabled="true" attribute; either remove these two classes from the
label class list or, if they're intentional for a planned change, add a short
TODO comment in the FormField component next to the div rendering (the one using
className={cn('flex flex-col gap-1.5', props.className)}) stating that FormField
should add 'group' and toggle data-disabled when disabled so those selectors
will be effective.
|
Style issue on dashboard Before
Now
|
|
Issue in user card Before
Now
|
Before
Now
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/devtools/login-hint.tsx (1)
54-55: Consider addingflex-wrapfor consistency.Line 23 uses
flex flex-wrap gap-x-1while this line uses onlyflex gap-x-1. While the OTP code is typically short enough that wrapping is unlikely, addingflex-wrapwould maintain consistency withLoginEmailHintand handle edge cases on narrow viewports.♻️ Suggested change
- <AlertDescription className="flex gap-x-1 text-sm leading-4"> + <AlertDescription className="flex flex-wrap gap-x-1 text-sm leading-4">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/devtools/login-hint.tsx` around lines 54 - 55, In AlertDescription inside src/features/devtools/login-hint.tsx (the JSX element named AlertDescription), add the tailwind utility "flex-wrap" to the existing className "flex gap-x-1" so it becomes consistent with LoginEmailHint's "flex flex-wrap gap-x-1" and prevents layout issues on narrow viewports; update the className string on the AlertDescription element accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/features/devtools/login-hint.tsx`:
- Around line 54-55: In AlertDescription inside
src/features/devtools/login-hint.tsx (the JSX element named AlertDescription),
add the tailwind utility "flex-wrap" to the existing className "flex gap-x-1" so
it becomes consistent with LoginEmailHint's "flex flex-wrap gap-x-1" and
prevents layout issues on narrow viewports; update the className string on the
AlertDescription element accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c790fda3-7f5b-41d2-a196-51ba7a3f840a
📒 Files selected for processing (1)
src/features/devtools/login-hint.tsx
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A link cannot be semantically disabled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reverts card.tsx to its previous styling (rounded-sm, border, py-4, gap-2,
font-semibold titles) as the new shadcn defaults don't fit the current design.
Also fixes a whitespace collapse bug in login-hint flex containers where
trailing {' '} was being stripped — replaced with gap-x-1.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ntatoud
force-pushed
the
ntatoud/update-shadcn-components
branch
from
db47677 to
61857ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/components/ui/dialog.tsx (1)
140-140: Title loses explicit sizing and visual weight.Removing
text-lgand switching fromfont-semiboldtofont-mediummakes dialog titles smaller and lighter. The title now inherits font size rather than explicitly setting it, which could lead to inconsistent sizing depending on parent context.Consider retaining
text-lg(or equivalent liketext-base) to ensure consistent title sizing across different dialog usage contexts.♻️ Suggested fix to preserve explicit sizing
- className={cn('leading-none font-medium', className)} + className={cn('text-lg leading-none font-medium', className)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/dialog.tsx` at line 140, The dialog title lost explicit sizing and weight; update the DialogTitle className usage (the line rendering className={cn('leading-none font-medium', className)}) to include an explicit size and stronger weight—e.g., add text-lg and font-semibold so it becomes cn('text-lg font-semibold leading-none', className)—to ensure consistent title sizing and visual weight across contexts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/alert.tsx`:
- Line 7: The alert component hardcodes LTR utilities ("text-left" and a
"right-3" positioning) which break RTL layouts; update the main class string in
the group/alert element (the long class that currently contains "text-left") to
use logical-direction utilities (replace "text-left" with "text-start"), and
update any positioning instances that use "right-3" (e.g., the action/icon
wrapper referenced in this file) to their logical equivalents (e.g., "end-3") so
the Alert becomes RTL-safe while keeping the same visual layout in LTR.
In `@src/components/ui/dialog.tsx`:
- Line 34: The overlay opacity was reduced from bg-black/80 to bg-black/10 in
the dialog overlay class string, which makes the backdrop nearly transparent and
may cause visual regressions; update the class in the Dialog overlay (the fixed
inset-0 isolate z-50 ... string in src/components/ui/dialog.tsx) to use a
stronger semi-transparent value (for example bg-black/50 or bg-black/70) and
retain the supports-backdrop-filter:backdrop-blur-xs token so the blur still
applies conditionally—choose the value that matches your design tokens or theme
variables and adjust the class string accordingly to restore appropriate dialog
prominence.
In `@src/components/ui/scroll-area.tsx`:
- Line 43: The scroll-area component's orientation selectors use non-existent
attributes data-horizontal/data-vertical, so in the ScrollArea (the CSS/string
that builds the scrollbar/track classes) replace those selectors with the
attribute form using data-[orientation=horizontal] and
data-[orientation=vertical] (e.g., change data-horizontal:h-2.5 ...
data-vertical:w-2.5 to data-[orientation=horizontal]:h-2.5 ...
data-[orientation=vertical]:w-2.5) so the primitive's emitted data-orientation
attribute actually applies the styles.
In `@src/components/ui/tooltip.tsx`:
- Around line 5-7: The global TooltipProvider default of delay = 0 causes all
tooltips to open instantly and is being reapplied because Tooltip currently
always creates its own provider; to fix, remove the immediate default (change
TooltipProvider signature to not default delay to 0, e.g., leave it undefined)
and update Tooltip so it does not unconditionally wrap its content in a new
TooltipProvider—instead detect an existing provider (or only create a provider
when none exists) and pass through any delay prop; finally, set delay={0}
explicitly where the sidebar needs immediate tooltips so the behavior remains
scoped (refer to TooltipProvider and Tooltip functions to locate changes).
---
Nitpick comments:
In `@src/components/ui/dialog.tsx`:
- Line 140: The dialog title lost explicit sizing and weight; update the
DialogTitle className usage (the line rendering className={cn('leading-none
font-medium', className)}) to include an explicit size and stronger weight—e.g.,
add text-lg and font-semibold so it becomes cn('text-lg font-semibold
leading-none', className)—to ensure consistent title sizing and visual weight
across contexts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4280d34b-ca33-4ba5-a5a2-18294280b8ab
📒 Files selected for processing (13)
src/components/ui/alert.tsxsrc/components/ui/avatar.tsxsrc/components/ui/button-link.stories.tsxsrc/components/ui/button-link.tsxsrc/components/ui/button.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/label.tsxsrc/components/ui/scroll-area.tsxsrc/components/ui/separator.tsxsrc/components/ui/sheet.tsxsrc/components/ui/tooltip.tsxsrc/features/devtools/login-hint.tsx
💤 Files with no reviewable changes (1)
- src/components/ui/button-link.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/ui/label.tsx
- src/features/devtools/login-hint.tsx
- src/components/ui/button.tsx
- src/components/ui/dropdown-menu.tsx
|
|
||
| const alertVariants = cva( | ||
| 'relative grid w-full grid-cols-[0_1fr] items-start gap-y-0.5 rounded-lg border px-4 py-3 text-sm has-[>svg]:grid-cols-[calc(var(--spacing)*4)_1fr] has-[>svg]:gap-x-3 [&>svg]:size-4 [&>svg]:translate-y-0.5 [&>svg]:text-current', | ||
| "group/alert relative grid w-full gap-0.5 rounded-lg border px-4 py-3 text-left text-sm has-data-[slot=alert-action]:relative has-data-[slot=alert-action]:pr-18 has-[>svg]:grid-cols-[auto_1fr] has-[>svg]:gap-x-2.5 *:[svg]:row-span-2 *:[svg]:translate-y-0.5 *:[svg]:text-current *:[svg:not([class*='size-'])]:size-4", |
There was a problem hiding this comment.
Use logical-direction utilities to keep Alert RTL-safe.
Line 7 (text-left) and Line 70 (right-3) hardcode LTR direction. This can misalign alert content/actions in RTL locales.
💡 Proposed fix
- "group/alert relative grid w-full gap-0.5 rounded-lg border px-4 py-3 text-left text-sm has-data-[slot=alert-action]:relative has-data-[slot=alert-action]:pr-18 has-[>svg]:grid-cols-[auto_1fr] has-[>svg]:gap-x-2.5 *:[svg]:row-span-2 *:[svg]:translate-y-0.5 *:[svg]:text-current *:[svg:not([class*='size-'])]:size-4",
+ "group/alert relative grid w-full gap-0.5 rounded-lg border px-4 py-3 text-start text-sm has-data-[slot=alert-action]:relative has-data-[slot=alert-action]:pr-18 has-[>svg]:grid-cols-[auto_1fr] has-[>svg]:gap-x-2.5 *:[svg]:row-span-2 *:[svg]:translate-y-0.5 *:[svg]:text-current *:[svg:not([class*='size-'])]:size-4",- className={cn('absolute top-2.5 right-3', className)}
+ className={cn('absolute top-2.5 end-3', className)}Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/alert.tsx` at line 7, The alert component hardcodes LTR
utilities ("text-left" and a "right-3" positioning) which break RTL layouts;
update the main class string in the group/alert element (the long class that
currently contains "text-left") to use logical-direction utilities (replace
"text-left" with "text-start"), and update any positioning instances that use
"right-3" (e.g., the action/icon wrapper referenced in this file) to their
logical equivalents (e.g., "end-3") so the Alert becomes RTL-safe while keeping
the same visual layout in LTR.
| data-slot="dialog-overlay" | ||
| className={cn( | ||
| 'fixed inset-0 isolate z-50 bg-black/80 backdrop-blur-xs duration-100 data-closed:animate-out data-closed:fade-out-0 data-open:animate-in data-open:fade-in-0', | ||
| 'fixed inset-0 isolate z-50 bg-black/10 duration-100 data-closed:animate-out data-closed:fade-out-0 data-open:animate-in data-open:fade-in-0 supports-backdrop-filter:backdrop-blur-xs', |
There was a problem hiding this comment.
Significant overlay opacity reduction may cause visual regression.
The overlay opacity changed from bg-black/80 (80%) to bg-black/10 (10%) — an 8× reduction. Combined with the conditional supports-backdrop-filter:backdrop-blur-xs, this makes the overlay nearly transparent by default.
Given the visual regressions reported in PR comments, this dramatic change may reduce dialog prominence and affect readability of content behind the overlay. Consider whether this aligns with the project's design intent or if a middle-ground value (e.g., bg-black/50) would better balance the update with existing UX.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/dialog.tsx` at line 34, The overlay opacity was reduced
from bg-black/80 to bg-black/10 in the dialog overlay class string, which makes
the backdrop nearly transparent and may cause visual regressions; update the
class in the Dialog overlay (the fixed inset-0 isolate z-50 ... string in
src/components/ui/dialog.tsx) to use a stronger semi-transparent value (for
example bg-black/50 or bg-black/70) and retain the
supports-backdrop-filter:backdrop-blur-xs token so the blur still applies
conditionally—choose the value that matches your design tokens or theme
variables and adjust the class string accordingly to restore appropriate dialog
prominence.
| 'h-full w-2.5 border-l border-l-transparent', | ||
| orientation === 'horizontal' && | ||
| 'h-2.5 flex-col border-t border-t-transparent', | ||
| 'flex touch-none p-px transition-colors select-none data-horizontal:h-2.5 data-horizontal:flex-col data-horizontal:border-t data-horizontal:border-t-transparent data-vertical:h-full data-vertical:w-2.5 data-vertical:border-l data-vertical:border-l-transparent', |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @base-ui/react/scroll-areav1.2.0, what orientation data attributes are emitted on ScrollArea.Scrollbar? Does it emitdata-horizontal/data-vertical, or only data-orientation="horizontal|vertical"?
💡 Result:
In @base-ui/react/scroll-area@1.2.0, ScrollArea.Scrollbar emits only:
data-orientation="vertical"ordata-orientation="horizontal"(derived from theorientationstate key) [1]
It does not emit data-horizontal / data-vertical. The default state→data-attribute behavior is data-${key.toLowerCase()} (or a custom mapping where provided), which yields data-orientation=..., not data-horizontal/data-vertical [2].
Sources:
[1] Base UI v1.2.0 ScrollAreaScrollbarDataAttributes.ts (orientation = data-orientation) https://github.com/mui/base-ui/blob/v1.2.0/packages/react/src/scroll-area/scrollbar/ScrollAreaScrollbarDataAttributes.ts
[2] Base UI v1.2.0 getStateAttributesProps.ts (default data-${key.toLowerCase()} mapping) https://github.com/mui/base-ui/blob/v1.2.0/packages/react/src/utils/getStateAttributesProps.ts
Replace data-horizontal / data-vertical selectors with data-orientation.
The @base-ui/react/scroll-area@1.2.0 primitive emits only data-orientation="horizontal|vertical", not data-horizontal/data-vertical attributes. The current selectors are no-ops and won't style the scrollbar. Update to:
'flex touch-none p-px transition-colors select-none data-[orientation=horizontal]:h-2.5 data-[orientation=horizontal]:flex-col data-[orientation=horizontal]:border-t data-[orientation=horizontal]:border-t-transparent data-[orientation=vertical]:h-full data-[orientation=vertical]:w-2.5 data-[orientation=vertical]:border-l data-[orientation=vertical]:border-l-transparent',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/scroll-area.tsx` at line 43, The scroll-area component's
orientation selectors use non-existent attributes data-horizontal/data-vertical,
so in the ScrollArea (the CSS/string that builds the scrollbar/track classes)
replace those selectors with the attribute form using
data-[orientation=horizontal] and data-[orientation=vertical] (e.g., change
data-horizontal:h-2.5 ... data-vertical:w-2.5 to
data-[orientation=horizontal]:h-2.5 ... data-[orientation=vertical]:w-2.5) so
the primitive's emitted data-orientation attribute actually applies the styles.
…ay explicitly The z-50 was mistakenly removed along with z-60. Also removes the default forceRenderOverlay=true from DrawerContent/DialogContent and passes it explicitly in sidebar-nested usages (BuildInfoDrawer, ConfirmSignOut) so the backdrop renders correctly on mobile. Co-authored-by: Claude <noreply@anthropic.com>
|
|
I also noticed the backdrop blur of the sign out modal was not applied properly on mobile (floating element inside of the sidebar sheet) I fixed by forcing overlay render in src/features/auth/confirm-signout.tsx |
2 participants
Summary
Compared existing shadcn components with the latest base-vega registry versions and applied meaningful styling improvements while preserving all project-specific customizations (loading states, custom variants, i18n, etc.).
Bug fixes
login-hintVisual changes
High impact
dialog`sheet`
Medium impact
`alert`
`button`
`dropdown-menu`
Subtle
`avatar` — Subtle border overlay using `after:` pseudo-element with `mix-blend-darken/lighten`
`tooltip` — `delay` changed from `500` → `0`, tooltip appears immediately on hover
`label` — `items-center gap-2 font-medium select-none`, group-level disabled state support
Non-visual (refactoring only)
`scroll-area` and `separator` — JS conditionals replaced with CSS `data-horizontal:`/`data-vertical:` selectors (same output, cleaner code)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style
Documentation
Updates
disabledprop.