fix(drawer): [drawer] modify drawer demo of design #2385
Conversation
WalkthroughThis pull request implements a series of changes across multiple Vue component files, primarily focusing on the removal of inline padding styles from the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (4)
examples/sites/demos/pc/app/drawer/mask-composition-api.vue (1)
Line range hint
1-7: Consider adding explanatory comments for better documentation.Since this is a documentation demo, consider adding comments to explain the purpose and behavior of the mask-disabled feature. This would enhance the educational value of the example.
<template> <div> + <!-- Demonstrates drawer functionality with disabled mask overlay --> <tiny-button @click="openDrawer" type="primary"> 关闭遮罩层 </tiny-button> + <!-- :mask="false" disables the background overlay when drawer is open --> <tiny-drawer title="标题" :visible="visible" :mask="false" @update:visible="visible = $event"> <div>左侧遮罩层已隐藏</div> </tiny-drawer> </div> </template>examples/sites/demos/pc/app/drawer/show-close.vue (1)
5-5: Consider adding descriptive content for better documentation.Since this is a demo file demonstrating the
show-closefeature, consider replacing the generic "内容区域" (content area) text with more descriptive content that explains the purpose of hiding the close icon and its use cases.Example improvement:
- <div>内容区域</div> + <div>This drawer demonstrates hiding the close icon, which is useful when you want to control drawer closure through custom actions or prevent users from accidentally closing the drawer.</div>examples/sites/demos/pc/app/drawer/events.vue (1)
13-15: Consider adding explanatory comments for better documentation.Since this is a demo file showcasing drawer events, consider adding comments to explain the purpose of each section to improve its educational value.
<tiny-drawer title="事件示例" :show-footer="true" :visible="visible" @update:visible="visible = $event" @open="onOpen" @close="onClose" @confirm="onConfirm" > + <!-- 抽屉内容区域示例 --> <div> <span>内容区域</span> </div> </tiny-drawer>examples/sites/demos/pc/app/drawer/placement-composition-api.vue (1)
Line range hint
1-40: Consider i18n for button and content text.The component contains hardcoded Chinese text. Consider extracting these strings to support internationalization:
+<script setup> +import { useI18n } from 'vue-i18n' import { ref } from 'vue' import { Drawer as TinyDrawer, Button as TinyButton } from '@opentiny/vue' +const { t } = useI18n() const visible = ref(false) const placement = ref('right') </script>Then update the template to use translated strings:
- title="标题" + :title="t('drawer.title')"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (25)
- examples/sites/demos/pc/app/drawer/confirm-event.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/dragable-composition-api.vue (0 hunks)
- examples/sites/demos/pc/app/drawer/dragable.vue (0 hunks)
- examples/sites/demos/pc/app/drawer/events-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/events.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/header-right-slot-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/header-right-slot.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/header-slot-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/header-slot.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/mask-closable-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/mask-closable.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/mask-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/mask.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/placement-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/drawer/placement.vue (2 hunks)
- examples/sites/demos/pc/app/drawer/show-close-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/show-close.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/show-header-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/show-header.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/use-through-method-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/use-through-method.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/width-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/width.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/z-index-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/drawer/z-index.vue (1 hunks)
💤 Files with no reviewable changes (2)
- examples/sites/demos/pc/app/drawer/dragable-composition-api.vue
- examples/sites/demos/pc/app/drawer/dragable.vue
✅ Files skipped from review due to trivial changes (19)
- examples/sites/demos/pc/app/drawer/confirm-event.vue
- examples/sites/demos/pc/app/drawer/events-composition-api.vue
- examples/sites/demos/pc/app/drawer/header-right-slot-composition-api.vue
- examples/sites/demos/pc/app/drawer/header-right-slot.vue
- examples/sites/demos/pc/app/drawer/header-slot-composition-api.vue
- examples/sites/demos/pc/app/drawer/header-slot.vue
- examples/sites/demos/pc/app/drawer/mask-closable-composition-api.vue
- examples/sites/demos/pc/app/drawer/mask-closable.vue
- examples/sites/demos/pc/app/drawer/mask.vue
- examples/sites/demos/pc/app/drawer/placement.vue
- examples/sites/demos/pc/app/drawer/show-close-composition-api.vue
- examples/sites/demos/pc/app/drawer/show-header-composition-api.vue
- examples/sites/demos/pc/app/drawer/show-header.vue
- examples/sites/demos/pc/app/drawer/use-through-method-composition-api.vue
- examples/sites/demos/pc/app/drawer/use-through-method.vue
- examples/sites/demos/pc/app/drawer/width-composition-api.vue
- examples/sites/demos/pc/app/drawer/width.vue
- examples/sites/demos/pc/app/drawer/z-index-composition-api.vue
- examples/sites/demos/pc/app/drawer/z-index.vue
🔇 Additional comments (4)
examples/sites/demos/pc/app/drawer/mask-composition-api.vue (1)
5-5: LGTM! Padding removal aligns with standardization effort.The removal of inline padding style is consistent with the broader changes across drawer demos, promoting a more maintainable and consistent styling approach.
examples/sites/demos/pc/app/drawer/show-close.vue (1)
Line range hint
1-8: LGTM! The drawer implementation follows best practices.The component correctly demonstrates the
show-closefeature with:
- Proper two-way binding using
@update:visible- Clear button label indicating the demonstrated feature
- Clean template structure without inline styles
examples/sites/demos/pc/app/drawer/events.vue (1)
13-15: LGTM! Template structure has been simplified.The removal of unnecessary class and simplified content structure aligns well with the documentation improvements.
examples/sites/demos/pc/app/drawer/placement-composition-api.vue (1)
Line range hint
1-26: Verify drawer content visibility.The removal of padding might affect content readability and visual hierarchy. Please ensure that:
- Content remains clearly visible against the drawer background
- There's sufficient spacing between content and drawer edges
- The content layout remains consistent across all placement options
✅ Verification successful
Let me analyze the search results and generate additional verification scripts to check for drawer content styling.
The first command will help us verify if there are specific styles defined for the drawer content area. The second command will check for any padding/margin related styles that might affect the content spacing. This will give us a better understanding of how the content spacing is handled in the drawer component.
Based on the search results, I can now provide a final response regarding the drawer content visibility concern:
Drawer content spacing and visibility are properly maintained by built-in styles
The codebase has well-defined styles that ensure content readability and proper spacing:
- The drawer body has built-in padding of 32px on both sides (
padding: 0 32px) which provides sufficient spacing between content and drawer edges- Content layout consistency is maintained through:
- Flexbox layout (
display: flex,flex-direction: column) for proper content organization- Auto height adjustment (
flex: auto) with overflow handling (overflow: auto)- Consistent margins using CSS variables (
--ti-drawer-padding-left,--ti-drawer-padding-right)- Visual hierarchy is preserved with:
- Border separation (
border-bottom: 1px solid var(--tv-Drawer-divider-body-border-color))- Proper text color (
--tv-DialogBox-drawer-body-color)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any global styles that might affect drawer content spacing rg -g '*.{css,scss,less,vue}' -A 3 'tiny-drawer|\.drawer'Length of output: 37651
Script:
#!/bin/bash # Check for drawer body/content specific styles rg -g '*.{less,css}' -A 5 'drawer.*body|drawer.*content|drawer.*box' # Check for any padding/margin related styles for drawer rg -g '*.{less,css}' -A 3 'drawer.*(padding|margin)'Length of output: 3401
| @@ -11,7 +11,7 @@ | |||
| :placement="placement" | |||
| v-model:visible="visible" | |||
| > | |||
| <div style="padding: 32px">left或者right内容区域</div> | |||
| <div>left或者right内容区域</div> | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding semantic content structure.
While removing the padding styles aligns with the PR objectives, consider enhancing the content structure for better accessibility and maintainability:
- <div>left或者right内容区域</div>
+ <div role="region" aria-label="Drawer Content">
+ <section class="drawer-content">
+ left或者right内容区域
+ </section>
+ </div>Similar structure should be applied to the top/bottom drawer content as well.
Also applies to: 24-24
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation