-
-
Notifications
You must be signed in to change notification settings - Fork 2
Issue 49 #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 49 #52
Conversation
…ll of the slots to render inside of the default slot, but still passes the default slot.
WalkthroughThe changes introduce new CSS class wrappers around slot content in the drawer component, add a static CSS class to the handle container's class list, and update SCSS to apply a flex column layout to the drawer's content area. No public APIs or function signatures are modified. Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/plugin/VResizeDrawer.vue (1)
53-55
: Consider conditional rendering for default slot wrapper.While the
v-navigation-drawer__content
wrapper improves structure and enables the new flex layout styling, consider whether this should be conditional like the prepend/append slots to avoid unnecessary DOM elements when the default slot is empty.If you want to avoid rendering empty wrappers, consider this approach:
- <div class="v-navigation-drawer__content"> - <slot></slot> - </div> + <template v-if="slots.default || $slots.default"> + <div class="v-navigation-drawer__content"> + <slot></slot> + </div> + </template>However, this might break the flex layout styling if the content wrapper is always expected to be present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/plugin/composables/__tests__/__snapshots__/classes.test.ts.snap
is excluded by!**/*.snap
📒 Files selected for processing (3)
src/plugin/VResizeDrawer.vue
(1 hunks)src/plugin/composables/classes.ts
(1 hunks)src/plugin/styles/main.scss
(1 hunks)
🔇 Additional comments (4)
src/plugin/VResizeDrawer.vue (2)
47-49
: Good structural improvement for prepend slot.The wrapper div with
v-navigation-drawer__prepend
class aligns with Vuetify's standard navigation drawer structure, providing better styling consistency and layout control.
59-61
: Good structural improvement for append slot.The wrapper div with
v-navigation-drawer__append
class maintains consistency with the prepend slot structure and Vuetify conventions.src/plugin/composables/classes.ts (1)
39-39
: Good addition of Vuetify-standard CSS class.Adding the
v-navigation-drawer__handle
class provides better CSS targeting consistency and aligns with Vuetify's BEM-style naming conventions for navigation drawer components.src/plugin/styles/main.scss (1)
133-136
: Well-coordinated flex layout styling.The new CSS rule perfectly complements the template changes by applying flex column layout to the
v-navigation-drawer__content
wrapper. The direct child selector (>) ensures it only affects the immediate content wrapper without interfering with nested content.
fixes Issue #49
Summary by CodeRabbit
Style
New Features