feat(dropdown): add contextmenu trigger support #3861
Conversation
WalkthroughAdds support for a right-click (contextmenu) trigger for the dropdown component across demos, tests, docs, and renderless core logic; includes event binding, timing adjustments, demo scenarios, and test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/sites/demos/pc/app/dropdown/trigger-composition-api.vue(1 hunks)examples/sites/demos/pc/app/dropdown/trigger.spec.ts(2 hunks)examples/sites/demos/pc/app/dropdown/trigger.vue(1 hunks)packages/renderless/src/dropdown/index.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (5)
examples/sites/demos/pc/app/dropdown/trigger-composition-api.vue (1)
28-40: LGTM!The contextmenu trigger scenario is correctly implemented and follows the same pattern as the existing scenarios.
examples/sites/demos/pc/app/dropdown/trigger.spec.ts (1)
12-35: LGTM!The test correctly verifies contextmenu trigger behavior: hover should not show the menu, but right-click should.
packages/renderless/src/dropdown/index.ts (3)
54-59: LGTM!Correctly treats
contextmenutrigger the same asclickwith zero delay for immediate show behavior.
81-86: LGTM!Correctly treats
contextmenutrigger the same asclickwith zero delay for immediate hide behavior, consistent with theshow()function.
130-130: LGTM!Good type safety improvement by explicitly casting
event.targettoHTMLElementfor correct typing in downstream operations.
| <tiny-dropdown-item :icon="tinyIconPlus">黄金糕</tiny-dropdown-item> | ||
| <tiny-dropdown-item :icon="tinyIconPlusCircle">狮子头</tiny-dropdown-item> | ||
| <tiny-dropdown-item :icon="tinyIconPlusSquare">螺蛳粉</tiny-dropdown-item> | ||
| <tiny-dropdown-item :icon="tinyIconCheckedLinear">双皮奶</tiny-dropdown-item> | ||
| <tiny-dropdown-item :icon="tinyIconCheckedSur">蚵仔煎</tiny-dropdown-item> |
There was a problem hiding this comment.
Fix incorrect variable references.
The :icon bindings reference tinyIconPlus, tinyIconPlusCircle, etc., but these variables don't exist in the data properties (lines 60-66). This will cause runtime errors. Scenario 2 (lines 20-24) correctly uses iconPlus, iconPlusCircle, etc.
Apply this diff to fix the variable references:
- <tiny-dropdown-item :icon="tinyIconPlus">黄金糕</tiny-dropdown-item>
- <tiny-dropdown-item :icon="tinyIconPlusCircle">狮子头</tiny-dropdown-item>
- <tiny-dropdown-item :icon="tinyIconPlusSquare">螺蛳粉</tiny-dropdown-item>
- <tiny-dropdown-item :icon="tinyIconCheckedLinear">双皮奶</tiny-dropdown-item>
- <tiny-dropdown-item :icon="tinyIconCheckedSur">蚵仔煎</tiny-dropdown-item>
+ <tiny-dropdown-item :icon="iconPlus">黄金糕</tiny-dropdown-item>
+ <tiny-dropdown-item :icon="iconPlusCircle">狮子头</tiny-dropdown-item>
+ <tiny-dropdown-item :icon="iconPlusSquare">螺蛳粉</tiny-dropdown-item>
+ <tiny-dropdown-item :icon="iconCheckedLinear">双皮奶</tiny-dropdown-item>
+ <tiny-dropdown-item :icon="iconCheckedSur">蚵仔煎</tiny-dropdown-item>📝 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.
| <tiny-dropdown-item :icon="tinyIconPlus">黄金糕</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="tinyIconPlusCircle">狮子头</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="tinyIconPlusSquare">螺蛳粉</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="tinyIconCheckedLinear">双皮奶</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="tinyIconCheckedSur">蚵仔煎</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="iconPlus">黄金糕</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="iconPlusCircle">狮子头</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="iconPlusSquare">螺蛳粉</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="iconCheckedLinear">双皮奶</tiny-dropdown-item> | |
| <tiny-dropdown-item :icon="iconCheckedSur">蚵仔煎</tiny-dropdown-item> |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/dropdown/trigger.vue around lines 33 to 37, the
template uses non-existent variables tinyIconPlus, tinyIconPlusCircle,
tinyIconPlusSquare, tinyIconCheckedLinear, and tinyIconCheckedSur causing
runtime errors; change those :icon bindings to the existing data property names
iconPlus, iconPlusCircle, iconPlusSquare, iconCheckedLinear, and iconCheckedSur
respectively so the template references the defined variables.
| /** --------------------------- | ||
| * 新增：右键触发 contextmenu | ||
| * --------------------------- */ | ||
| if (state.trigger === 'contextmenu') { | ||
| on(state.triggerElm, 'contextmenu', (e: MouseEvent) => { | ||
| e.preventDefault() | ||
| api.handleClick() | ||
| }) | ||
| } else if (state.trigger === 'hover') { |
There was a problem hiding this comment.
Memory leak: inline handler can't be removed in cleanup.
The inline arrow function (e: MouseEvent) => { e.preventDefault(); api.handleClick() } is bound to the contextmenu event, but beforeDistory (line 313) attempts to remove it using api.handleClick as the reference. Since off() requires the exact same function reference to remove a listener, the contextmenu handler will never be cleaned up, causing a memory leak.
Recommended solution: Store the handler function to enable proper cleanup:
+export const handleContextMenu =
+ ({ api }: Pick<IDropdownRenderlessParams, 'api'>) =>
+ (e: MouseEvent) => {
+ e.preventDefault()
+ api.handleClick()
+ }Then in the API object (where handlers are composed), add:
const api = {
// ... other handlers
handleContextMenu: handleContextMenu({ api }),
// ...
}Update initEvent:
if (state.trigger === 'contextmenu') {
- on(state.triggerElm, 'contextmenu', (e: MouseEvent) => {
- e.preventDefault()
- api.handleClick()
- })
+ on(state.triggerElm, 'contextmenu', api.handleContextMenu)
} else if (state.trigger === 'hover') {Update beforeDistory:
- off(state.triggerElm, 'contextmenu', api.handleClick) /** 右键清理 */
+ off(state.triggerElm, 'contextmenu', api.handleContextMenu)🤖 Prompt for AI Agents
In packages/renderless/src/dropdown/index.ts around lines 218-226 (and refer to
beforeDistory at line 313), the contextmenu listener is added with an inline
arrow function so off() later cannot remove it; replace the inline handler with
a named function stored on the instance/state and expose it via the API (e.g.,
api.handleContextMenu) so you can both add and remove the exact same function
reference. Concretely: create a handler factory or method that accepts the
event, calls preventDefault and api.handleClick, assign it to state or api
(handleContextMenu) during initEvent, use that reference when calling
on(state.triggerElm, 'contextmenu', handler) and remove it in beforeDistory with
off(state.triggerElm, 'contextmenu', handler); also add the new
handleContextMenu entry to the api object where other handlers are composed.
| /** --------------------------- | ||
| * 新增：右键触发 contextmenu | ||
| * --------------------------- */ | ||
| if (state.trigger === 'contextmenu') { |
There was a problem hiding this comment.
你好，这个应该属于新特性了，需要在对应的api文件中添加描述和版本信息哈
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/sites/demos/apis/dropdown.js(1 hunks)examples/sites/demos/pc/app/dropdown/webdoc/dropdown.js(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (3)
examples/sites/demos/pc/app/dropdown/webdoc/dropdown.js (2)
96-98: LGTM! Documentation properly describes the contextmenu trigger.The trigger demo descriptions correctly document the new contextmenu (right-click) trigger option with appropriate version information (3.28.0) in both Chinese and English.
310-310: LGTM! Features list properly updated.The trigger feature description correctly includes contextmenu (right-click) support with appropriate version information (3.28.0).
examples/sites/demos/apis/dropdown.js (1)
198-209: LGTM! API definition correctly extended with contextmenu support.The trigger prop's type definition properly includes the new 'contextmenu' option, and the meta field appropriately marks version 3.28.0 as when this feature became stable. The changes are consistent with the documentation updates.
| 'zh-CN': '<p>通过 <code>visible</code> 属性手动控制下拉菜单显隐，优先级高于trigger。</p>\n', | ||
| 'en-US': | ||
| '<p>Manually control the visibility of the dropdown menu through the<code>visible</code>attribute, with priority over trigger.</p>\n' | ||
| '<p>Manually control the visibility of the dropdown menu through the <code>visible</code>attribute, with priority over trigger.</p>\n' |
There was a problem hiding this comment.
Add missing space in the English description.
There's a missing space between </code> and attribute in the text.
Apply this diff:
- '<p>Manually control the visibility of the dropdown menu through the <code>visible</code>attribute, with priority over trigger.</p>\n'
+ '<p>Manually control the visibility of the dropdown menu through the <code>visible</code> attribute, with priority over trigger.</p>\n'📝 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.
| '<p>Manually control the visibility of the dropdown menu through the <code>visible</code>attribute, with priority over trigger.</p>\n' | |
| '<p>Manually control the visibility of the dropdown menu through the <code>visible</code> attribute, with priority over trigger.</p>\n' |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/dropdown/webdoc/dropdown.js around line 111, the
English description string is missing a space between the closing code tag and
the word "attribute"; update the string to insert a single space after "</code>"
so it reads "</code> attribute" (adjust surrounding escaping/quotes if needed)
to correct the grammar.
2 participants
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
Documentation
Bug Fixes / Behavior
Tests
✏️ Tip: You can customize this high-level summary in your review settings.