feat: drawer close icon placement #5269
Conversation
mynetfan
requested review from
anncwb,
vince292007 and
a team
as code owners
|
|
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
packages/@core/ui-kit/popup-ui/src/drawer/drawer.tsOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThis pull request enhances the Vben Drawer component by introducing a new Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue (3)
165-182: Consider extracting the close icon into a reusable componentThe close icon implementation with its slot is duplicated in the template. Consider extracting it into a separate component for better maintainability.
Create a new component like this:
<!-- CloseIconButton.vue --> <template> <SheetClose as-child :class="cn('data-[state=open]:bg-secondary ml-[2px] cursor-pointer rounded-full opacity-80 transition-opacity hover:opacity-100 focus:outline-none disabled:pointer-events-none')" > <slot> <VbenIconButton> <X class="size-4" /> </VbenIconButton> </slot> </SheetClose> </template>Then use it in the drawer:
- <SheetClose - v-if="closable && closeIconPlacement === 'left'" - as-child - class="data-[state=open]:bg-secondary ml-[2px] cursor-pointer rounded-full opacity-80 transition-opacity hover:opacity-100 focus:outline-none disabled:pointer-events-none" - > - <slot name="close-icon"> - <VbenIconButton> - <X class="size-4" /> - </VbenIconButton> - </slot> - </SheetClose> + <CloseIconButton v-if="closable && closeIconPlacement === 'left'"> + <slot name="close-icon" /> + </CloseIconButton>
207-215: Remove code duplication in close icon implementationThe close icon implementation is duplicated. After extracting the close icon into a separate component as suggested above, update this section as well.
- <SheetClose - v-if="closable && closeIconPlacement === 'right'" - as-child - class="data-[state=open]:bg-secondary ml-[2px] cursor-pointer rounded-full opacity-80 transition-opacity hover:opacity-100 focus:outline-none disabled:pointer-events-none" - > - <slot name="close-icon"> - <VbenIconButton> - <X class="size-4" /> - </VbenIconButton> - </slot> - </SheetClose> + <CloseIconButton v-if="closable && closeIconPlacement === 'right'"> + <slot name="close-icon" /> + </CloseIconButton>
160-162: Consider using CSS classes for conditional stylingInstead of using inline style conditions, consider using CSS classes for better maintainability.
- 'pl-2': closable && closeIconPlacement === 'left', + 'drawer-header--left-close': closable && closeIconPlacement === 'left',Then define the CSS class:
.drawer-header--left-close { padding-left: 0.5rem; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/src/components/common-ui/vben-drawer.md(2 hunks)packages/@core/ui-kit/popup-ui/src/drawer/drawer.ts(2 hunks)packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue(4 hunks)
🔇 Additional comments (5)
packages/@core/ui-kit/popup-ui/src/drawer/drawer.ts (2)
9-10: LGTM: Clean type definition for close icon placement
The CloseIconPlacement type correctly defines the allowed values for the close icon position.
23-30: LGTM: Well-documented prop definitions
The closable and closeIconPlacement props are properly documented with clear descriptions.
docs/src/components/common-ui/vben-drawer.md (2)
87-87: LGTM: Clear documentation for new prop
The closeIconPlacement prop is well-documented with its type and default value.
133-134: LGTM: Clear documentation for new slots
The new slots close-icon and extra are properly documented with clear descriptions.
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue (1)
37-37: LGTM: Proper default value for new prop
The default value 'right' maintains backward compatibility.
Description
为抽屉添加closeIconPlacement用来控制关闭按钮的位置（左侧或者默认的右侧），并且添加close-icon插槽用来定制关闭图标。
close #5267
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes
New Features
Documentation
These updates provide more flexibility in positioning and customizing the drawer's close icon and header content.