feat(Sidebar): add transition prop#6484
Conversation
📝 WalkthroughWalkthroughThe PR introduces conditional transition styling to the Sidebar component. A new optional Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/runtime/components/Sidebar.vueParsing error: Unexpected token ) src/theme/sidebar.tsParsing error: Unexpected token { 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/theme/sidebar.ts`:
- Around line 20-26: The transition: true variant in src/theme/sidebar.ts
currently unconditionally adds animation classes; update the variant generation
to respect the global options.theme.transitions toggle by returning no
transition classes (or an empty/disabled variant) when options.theme.transitions
is false. Locate the transition: { true: { ... } } entry and change its
implementation to check options.theme.transitions (the global toggle) and only
include gap/container/rail classes when that flag is true, otherwise omit or
return empty strings so animations are effectively disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5647cd1a-f5d5-4d39-9152-5e19bc6abbb1
⛔ Files ignored due to path filters (2)
test/components/__snapshots__/Sidebar-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/Sidebar.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/runtime/components/Sidebar.vuesrc/theme/sidebar.ts
| transition: { | ||
| true: { | ||
| gap: 'transition-[width] duration-200 ease-out', | ||
| container: 'transition-[left,right,width] duration-200 ease-out', | ||
| rail: 'transition-all ease-out' | ||
| } | ||
| }, |
There was a problem hiding this comment.
Honor the global options.theme.transitions toggle in the new variant.
transition: true currently adds animation classes even when options.theme.transitions is false, so global motion disabling is bypassed.
Suggested fix
variants: {
transition: {
- true: {
- gap: 'transition-[width] duration-200 ease-out',
- container: 'transition-[left,right,width] duration-200 ease-out',
- rail: 'transition-all ease-out'
- }
+ true: options.theme.transitions ? {
+ gap: 'transition-[width] duration-200 ease-out',
+ container: 'transition-[left,right,width] duration-200 ease-out',
+ rail: 'transition-all ease-out'
+ } : {}
},📝 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.
| transition: { | |
| true: { | |
| gap: 'transition-[width] duration-200 ease-out', | |
| container: 'transition-[left,right,width] duration-200 ease-out', | |
| rail: 'transition-all ease-out' | |
| } | |
| }, | |
| transition: { | |
| true: options.theme.transitions ? { | |
| gap: 'transition-[width] duration-200 ease-out', | |
| container: 'transition-[left,right,width] duration-200 ease-out', | |
| rail: 'transition-all ease-out' | |
| } : {} | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/theme/sidebar.ts` around lines 20 - 26, The transition: true variant in
src/theme/sidebar.ts currently unconditionally adds animation classes; update
the variant generation to respect the global options.theme.transitions toggle by
returning no transition classes (or an empty/disabled variant) when
options.theme.transitions is false. Locate the transition: { true: { ... } }
entry and change its implementation to check options.theme.transitions (the
global toggle) and only include gap/container/rail classes when that flag is
true, otherwise omit or return empty strings so animations are effectively
disabled.
There was a problem hiding this comment.
the options.theme.transitions is not for transform animations but for color hovers mostly
There was a problem hiding this comment.
@benjamincanac, thanks for the clarification — that makes perfect sense. options.theme.transitions governs color hover animations (like after:transition-colors on the rail), while the new transition prop is an independent, component-level control for the collapse/expand layout animations (width, left, right). The two are orthogonal, so there's no need to gate the variant behind the global toggle. I was wrong to flag this — apologies for the noise!
(╯°□°)╯ 🐇 ← catching the carrot I threw too early
✏️ Learnings added
Learnt from: benjamincanac
Repo: nuxt/ui PR: 6484
File: src/theme/sidebar.ts:20-26
Timestamp: 2026-05-22T10:49:36.271Z
Learning: In `src/theme/sidebar.ts` (and the Nuxt UI theme system generally), `options.theme.transitions` controls **color hover transitions** (e.g. `after:transition-colors`), NOT layout/transform animations such as `transition-[width]` or `transition-[left,right,width]`. Component-level animation props (e.g. the `transition` prop on `USidebar`) are independent of `options.theme.transitions` and should not be gated behind it.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: benjamincanac
Repo: nuxt/ui PR: 6406
File: src/theme/breadcrumb.ts:45-57
Timestamp: 2026-05-05T14:15:14.954Z
Learning: In the Nuxt UI theme code (e.g., `src/theme/breadcrumb.ts` and other theme files), `neutral` should not be expected to appear in `options.theme.colors`. If a change iterates over `options.theme.colors` to generate color variants, do not flag a “duplicate neutral” concern as long as `neutral` is defined only via the explicit variant/color objects (and `neutral` is not part of `options.theme.colors`). As part of the review, verify that `options.theme.colors` does not include `neutral` before suppressing/ignoring any potential duplicate warning.
commit: |
🔗 Linked issue
❓ Type of change
📚 Description
Add a
transitionprop toUSidebar(defaults totrue) thate collapse/expand animation. Follows the same pattern asUModal'stransitionprop.transition: truevariantease-lineartoease-outfor a more natural feel📝 Checklist