fix(components): improve arrow styling with stroke-default and fill-bg#6095
fix(components): improve arrow styling with stroke-default and fill-bg#6095
stroke-default and fill-bg#6095Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis pull request changes arrow styling across theme files: in src/theme/dropdown-menu.ts, src/theme/input-menu.ts, src/theme/popover.ts, src/theme/select.ts, and src/theme/tooltip.ts the arrow slot class value was changed from Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
benjamincanac
left a comment
There was a problem hiding this comment.
Thanks for the PR it looks way better indeed, that does actually solve #2337 issue I created 1.5 years ago I completely forgot about 😅
I'm wondering if it would make sense to register fill-bg (and stroke-bg to be consistent) classes 🤔 This can be done in https://github.com/nuxt/ui/blob/v4/src/templates.ts#L266
stroke-default and fill-bg
606caab to
8d2cf6d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/theme/popover.ts (1)
4-4: LGTM — thefill-(--ui-bg) stroke-defaultpattern is consistent and semantically correct.
fill-(--ui-bg)is valid Tailwind v4 CSS variable shorthand syntax, equivalent tofill-[var(--ui-bg)]. Filling the arrow with the component background token and stroking it withstroke-default(matching thering ring-defaulton thecontentslot) correctly resolves the visual inconsistency described in issue#2337. The pattern is applied uniformly across all five affected theme files.The stroke-width concern raised on
tooltip.tsapplies equally here — confirm the arrow component emits a non-zerostroke-widthbefore shipping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/theme/popover.ts` at line 4, The theme sets arrow: 'fill-(--ui-bg) stroke-default' which is fine; ensure the arrow SVG actually renders a visible stroke by adding a non-zero stroke width wherever the arrow is rendered (confirm the component that renders the arrow uses a stroke-width attribute or CSS rule); specifically verify and, if missing, add a stroke-width on the arrow-rendering code (the component that consumes the popover theme's arrow class / the popover arrow SVG element) so the 'stroke-default' token is visible.
🤖 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/theme/tooltip.ts`:
- Line 4: The arrow's stroke color (stroke-default) won't be visible because SVG
paths default to stroke-width:0; update the arrow styling in
src/theme/tooltip.ts (the arrow property) to include a non-zero stroke width
(e.g., add a class or inline style that sets stroke-width) or alternatively
modify the Reka UI Arrow component to set strokeWidth on the rendered <path>;
ensure the change targets the arrow configuration string 'arrow' so the SVG path
receives both stroke-default and a non-zero stroke-width so the border renders.
---
Duplicate comments:
In `@src/theme/popover.ts`:
- Line 4: The theme sets arrow: 'fill-(--ui-bg) stroke-default' which is fine;
ensure the arrow SVG actually renders a visible stroke by adding a non-zero
stroke width wherever the arrow is rendered (confirm the component that renders
the arrow uses a stroke-width attribute or CSS rule); specifically verify and,
if missing, add a stroke-width on the arrow-rendering code (the component that
consumes the popover theme's arrow class / the popover arrow SVG element) so the
'stroke-default' token is visible.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/theme/dropdown-menu.tssrc/theme/input-menu.tssrc/theme/popover.tssrc/theme/select.tssrc/theme/tooltip.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/theme/select.ts
- src/theme/dropdown-menu.ts
src/theme/tooltip.ts
Outdated
| slots: { | ||
| content: 'flex items-center gap-1 bg-default text-highlighted shadow-sm rounded-sm ring ring-default h-6 px-2.5 py-1 text-xs select-none data-[state=delayed-open]:animate-[scale-in_100ms_ease-out] data-[state=closed]:animate-[scale-out_100ms_ease-in] origin-(--reka-tooltip-content-transform-origin) pointer-events-auto', | ||
| arrow: 'fill-default', | ||
| arrow: 'fill-(--ui-bg) stroke-default', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for Arrow component usage and definition
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.vue" \) | head -20Repository: nuxt/ui
Length of output: 839
🏁 Script executed:
# Find arrow-related code in the codebase
rg -n "arrow" --type ts --type tsx --type vue -i | head -30Repository: nuxt/ui
Length of output: 78
🏁 Script executed:
# Look for the tooltip.ts file and surrounding context
cat -n src/theme/tooltip.tsRepository: nuxt/ui
Length of output: 694
🏁 Script executed:
# Search for Arrow component from UI libraries or custom implementation
rg -n "Arrow" --type ts --type tsx --type vue -A 3 -B 1 | head -40Repository: nuxt/ui
Length of output: 78
🏁 Script executed:
# Search for Arrow component definition and SVG rendering
rg -n "stroke-width" --type ts --type vue | head -20Repository: nuxt/ui
Length of output: 78
🏁 Script executed:
# Search for Arrow usage in components, especially in tooltip/popover related files
rg -n "Arrow" --type ts --type vue -A 2 -B 1 | head -50Repository: nuxt/ui
Length of output: 78
🏁 Script executed:
# Look for the components directory structure
fd -type f -name "*.vue" | grep -i "tooltip\|popover\|dropdown" | head -20Repository: nuxt/ui
Length of output: 221
🏁 Script executed:
# Search for SVG arrow implementation or rendering
rg -n "<arrow\|<svg\|viewBox" --type vue -A 3 | grep -i arrow -A 3 | head -30Repository: nuxt/ui
Length of output: 78
🏁 Script executed:
# Check Tailwind config for stroke-default definition
fd -type f -name "tailwind.config.*" -o -name "*.config.ts"Repository: nuxt/ui
Length of output: 221
🏁 Script executed:
# Search for stroke-width usage
rg -n "stroke-width" --include="*.ts" --include="*.js" | head -20Repository: nuxt/ui
Length of output: 492
🏁 Script executed:
# Search for Arrow component references
rg -n "Arrow" --include="*.ts" --include="*.js" -A 2 -B 1 | head -50Repository: nuxt/ui
Length of output: 492
🏁 Script executed:
# Find tooltip and related component files
fd -e "ts" -e "js" | grep -i "tooltip\|popover\|dropdown" | head -20Repository: nuxt/ui
Length of output: 220
🏁 Script executed:
# Look for SVG or arrow rendering patterns
rg -n "svg\|<arrow\|stroke-width" --include="*.ts" --include="*.js" -i | head -30Repository: nuxt/ui
Length of output: 492
🏁 Script executed:
# Check the components directory structure
ls -la src/components/ 2>/dev/null | head -20Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Search for theme files to understand how stroke-default is used
fd -e ts src/theme/ -x cat {} \;Repository: nuxt/ui
Length of output: 396
🏁 Script executed:
# Search for stroke-width in all files
rg "stroke-width"Repository: nuxt/ui
Length of output: 7563
🏁 Script executed:
# Search for Arrow references
rg "Arrow" -A 2 -B 1 | head -50Repository: nuxt/ui
Length of output: 3125
🏁 Script executed:
# View all theme files to understand styling patterns
fd . src/theme/Repository: nuxt/ui
Length of output: 4054
🏁 Script executed:
# Check for Reka UI imports and arrow usage
rg "from.*reka\|import.*Arrow\|<Arrow"Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Look at the components directory
ls -la src/components/Repository: nuxt/ui
Length of output: 113
🏁 Script executed:
# Search for where the arrow slot class is actually used
rg "fill-.*--ui-bg\|stroke-default"Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Search for Tooltip, Popover, DropdownMenu component implementations
fd . src/runtime/components/ -type f 2>/dev/null | head -30Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Look for Tooltip component specifically
fd Tooltip src/runtime/ 2>/dev/nullRepository: nuxt/ui
Length of output: 85
🏁 Script executed:
# Search for arrow slot usage
rg "arrow" -A 3 -B 1 | head -60Repository: nuxt/ui
Length of output: 6629
🏁 Script executed:
# Check if there's a Reka UI component file or wrapper
rg "from.*reka-ui\|import.*reka" -A 2 -B 1 | head -40Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Look for the Select component to see how Arrow is used
cat src/runtime/components/Select.vue | head -100Repository: nuxt/ui
Length of output: 3770
🏁 Script executed:
# View the Tooltip component implementation
cat src/runtime/components/Tooltip.vueRepository: nuxt/ui
Length of output: 4166
🏁 Script executed:
# Search for Reka UI Arrow usage patterns
rg "Reka.*Arrow\|reka-ui.*Arrow" -A 5 -B 2Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Check the popover and dropdown-menu theme files too
cat src/theme/popover.tsRepository: nuxt/ui
Length of output: 387
🏁 Script executed:
# Check dropdown-menu theme
cat src/theme/dropdown-menu.tsRepository: nuxt/ui
Length of output: 4250
🏁 Script executed:
# Look for NavigationMenu to understand arrow styling consistency
cat src/theme/navigation-menu.tsRepository: nuxt/ui
Length of output: 9494
🏁 Script executed:
# Check test snapshots for Tooltip with arrow to see if stroke is rendered
grep -r "data-slot=\"arrow\"" --include="*.snap"Repository: nuxt/ui
Length of output: 7222
🏁 Script executed:
# Search for Reka UI configuration or stroke-width defaults
rg "stroke-width" -C 3Repository: nuxt/ui
Length of output: 10963
🏁 Script executed:
# Check if there's a Tailwind config that might set stroke-width globally
fd tailwind -type fRepository: nuxt/ui
Length of output: 221
🏁 Script executed:
# Look at the package.json to see Reka UI version
cat package.json | grep -A 2 -B 2 "reka-ui"Repository: nuxt/ui
Length of output: 176
🏁 Script executed:
# Check the git history for tooltip.ts to see recent changes
git log --oneline -20 src/theme/tooltip.tsRepository: nuxt/ui
Length of output: 117
🏁 Script executed:
# Check if test snapshots match the current theme
grep -A 2 "data-slot=\"arrow\"" test/components/__snapshots__/Tooltip.spec.ts.snapRepository: nuxt/ui
Length of output: 679
🏁 Script executed:
# Look at recent commits to tooltip to understand the stroke-default change
git log -p --max-count=3 src/theme/tooltip.tsRepository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Check if tests are passing or if snapshots need updating
grep -l "stroke-default" test/components/__snapshots__/*.snapRepository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Show the full diff of that recent commit
git show 8d2cf6dRepository: nuxt/ui
Length of output: 50368
🏁 Script executed:
# Check if Tailwind config has any global stroke-width settings
find . -name "tailwind.config.*" -o -name "tailwind.ts" -o -name "ui.config.*" | xargs cat 2>/dev/null | head -100Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Check if there's a base Tailwind config or preset
rg "stroke-width|@layer|@apply" | head -20Repository: nuxt/ui
Length of output: 8524
🏁 Script executed:
# Check if tests need snapshot updates
git status | grep -i snapshotRepository: nuxt/ui
Length of output: 33
🏁 Script executed:
# Verify Reka UI Arrow component does not set stroke-width
grep -r "stroke-width" node_modules/reka-ui 2>/dev/null | grep -i arrow | head -5Repository: nuxt/ui
Length of output: 33
Arrow stroke will not render—Reka UI's SVG path element needs stroke-width to display the stroke-default color.
The stroke-default class sets the stroke color, but SVG <path> elements have stroke-width: 0 by default, making any stroke invisible. The Reka UI <Arrow> component renders an SVG path without setting a non-zero stroke-width, so the stroke border will not appear regardless of the color class applied.
Either Reka UI's Arrow component must set stroke-width internally, or the theme must include stroke-width (e.g., via an inline style or a custom wrapper). Verify with Reka UI v2.8.2 documentation or test the arrow visually to confirm the stroke is actually rendering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/theme/tooltip.ts` at line 4, The arrow's stroke color (stroke-default)
won't be visible because SVG paths default to stroke-width:0; update the arrow
styling in src/theme/tooltip.ts (the arrow property) to include a non-zero
stroke width (e.g., add a class or inline style that sets stroke-width) or
alternatively modify the Reka UI Arrow component to set strokeWidth on the
rendered <path>; ensure the change targets the arrow configuration string
'arrow' so the SVG path receives both stroke-default and a non-zero stroke-width
so the border renders.
🔗 Linked issue
Resolves #2337
❓ Type of change
📚 Description
Before:
After:
📝 Checklist