-
Notifications
You must be signed in to change notification settings - Fork 333
feat(vue-renderless/drawer): add close-on-press-escape api #3900
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
feat(vue-renderless/drawer): add close-on-press-escape api #3900
Conversation
WalkthroughAdds a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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: 1
🧹 Nitpick comments (1)
examples/sites/demos/pc/app/drawer/webdoc/drawer.js (1)
19-30: Missing English translations.The
en-USvalues fornameanddescare empty. Consider adding translations for internationalization completeness.demoId: 'close-on-press-escape', name: { 'zh-CN': '按下 ESC 关闭抽屉', - 'en-US': '' + 'en-US': 'Close Drawer on ESC' }, desc: { 'zh-CN': '<p>添加 <code>close-on-press-escape</code> 属性可以控制是否可以通过 ESC 关闭抽屉。</p>', - 'en-US': '' + 'en-US': '<p>Use <code>close-on-press-escape</code> to control whether the drawer can be closed by pressing ESC.</p>' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/sites/demos/apis/drawer.js(1 hunks)examples/sites/demos/pc/app/drawer/close-on-press-escape-composition-api.vue(1 hunks)examples/sites/demos/pc/app/drawer/close-on-press-escape.spec.ts(1 hunks)examples/sites/demos/pc/app/drawer/close-on-press-escape.vue(1 hunks)examples/sites/demos/pc/app/drawer/webdoc/drawer.js(1 hunks)packages/renderless/src/drawer/index.ts(1 hunks)packages/renderless/src/drawer/vue.ts(3 hunks)packages/vue/src/drawer/src/pc.vue(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/renderless/src/drawer/index.ts (1)
packages/renderless/src/drawer/vue.ts (1)
api(28-28)
packages/renderless/src/drawer/vue.ts (1)
packages/renderless/src/drawer/index.ts (5)
keydown(84-98)addKeydownEvent(100-104)removeKeydownEvent(106-110)addDragEvent(173-185)removeDragEvent(187-199)
🔇 Additional comments (8)
packages/renderless/src/drawer/index.ts (2)
83-98: LGTM! Well-structured ESC key handling.The implementation correctly:
- Guards with early returns for invisible state and disabled prop
- Handles both 'Escape' and 'Esc' keys for browser compatibility
- Forces close with
api.handleClose('esc', true)to bypassbeforeCloseinterceptionOne consideration: if multiple drawers are open, all will respond to the same ESC key. This is typically expected behavior (topmost closes first), but worth noting.
100-111: LGTM!Clean implementation following the established drag event pattern. Using
api.keydownas the handler reference ensures proper cleanup.examples/sites/demos/pc/app/drawer/close-on-press-escape.spec.ts (1)
1-19: LGTM! Good basic test coverage.The test correctly validates the ESC-close functionality. Consider adding a complementary test case to verify that a drawer without
close-on-press-escapedoes NOT close on ESC key press, to ensure the prop is properly gated.packages/vue/src/drawer/src/pc.vue (1)
164-166: LGTM!Clean prop addition. The renderless layer handles all keydown logic, so no template changes are needed.
examples/sites/demos/pc/app/drawer/close-on-press-escape-composition-api.vue (1)
1-29: LGTM!Clean composition API demo that correctly demonstrates the
close-on-press-escapefeature.examples/sites/demos/pc/app/drawer/close-on-press-escape.vue (1)
1-38: LGTM!Clean options API demo that correctly demonstrates the
close-on-press-escapefeature, providing a good counterpart to the composition API version.packages/renderless/src/drawer/vue.ts (2)
56-60: LGTM!Clean integration of keydown handling. The methods are correctly wired to use
apiobject references, ensuring proper add/remove pairing for event listeners.
68-80: LGTM!The keydown listener is added unconditionally with cleanup in
onBeforeUnmount. Since thekeydownhandler itself guards withprops.closeOnPressEscape, this approach keeps the lifecycle simple while the guard handles the feature toggle.
| { | ||
| name: 'close-on-press-escape', | ||
| type: 'boolean', | ||
| defaultValue: 'false', | ||
| desc: { | ||
| 'zh-CN': 'ESC 键关闭抽屉', | ||
| 'en-US': 'ESC key to close drawer' | ||
| }, | ||
| mode: ['pc'], | ||
| pcDemo: 'closeOnPressEscape', | ||
| meta: { | ||
| stable: '3.28.0' | ||
| } | ||
| } |
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.
Inconsistent pcDemo value format.
The pcDemo value uses camelCase (closeOnPressEscape) while other props use kebab-case matching their demoId (e.g., before-close, mask-closable). This may cause the demo link to not resolve correctly.
- pcDemo: 'closeOnPressEscape',
+ pcDemo: 'close-on-press-escape',📝 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.
| { | |
| name: 'close-on-press-escape', | |
| type: 'boolean', | |
| defaultValue: 'false', | |
| desc: { | |
| 'zh-CN': 'ESC 键关闭抽屉', | |
| 'en-US': 'ESC key to close drawer' | |
| }, | |
| mode: ['pc'], | |
| pcDemo: 'closeOnPressEscape', | |
| meta: { | |
| stable: '3.28.0' | |
| } | |
| } | |
| { | |
| name: 'close-on-press-escape', | |
| type: 'boolean', | |
| defaultValue: 'false', | |
| desc: { | |
| 'zh-CN': 'ESC 键关闭抽屉', | |
| 'en-US': 'ESC key to close drawer' | |
| }, | |
| mode: ['pc'], | |
| pcDemo: 'close-on-press-escape', | |
| meta: { | |
| stable: '3.28.0' | |
| } | |
| } |
🤖 Prompt for AI Agents
In examples/sites/demos/apis/drawer.js around lines 211 to 224, the pcDemo value
uses camelCase ("closeOnPressEscape") while the rest use kebab-case matching
demoIds; update the pcDemo string to the kebab-case demo id
("close-on-press-escape") so the demo link resolves consistently with other
props.
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
close-on-press-escapeprop that enables automatic drawer closure when the ESC key is pressed. This property is disabled by default and can be individually configured per drawer instance. Users can now leverage this keyboard-based interaction pattern as a familiar way to close drawers.✏️ Tip: You can customize this high-level summary in your review settings.