-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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: user-dropdown support hover
trigger
#5143
Conversation
|
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/effects/hooks/src/use-hover-toggle.tsOops! Something went wrong! :( ESLint: 9.16.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 introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant Dropdown
participant HoverToggle
User->>Dropdown: Hover/Click
Dropdown->>HoverToggle: Check hover state
HoverToggle-->>Dropdown: Update visibility
Dropdown->>User: Show/Hide menu
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
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: 3
🧹 Nitpick comments (3)
packages/effects/hooks/src/use-hover-toggle.ts (2)
10-19
: Consider adding English documentation.While the current documentation is clear, adding English documentation would improve maintainability and accessibility for a broader developer audience.
/** - * 监测鼠标是否在元素内部,如果在元素内部则返回 true,否则返回 false - * @param refElement 所有需要检测的元素。如果提供了一个数组,那么鼠标在任何一个元素内部都会返回 true - * @param delay 延迟更新状态的时间 + * Monitors if the mouse is inside the specified elements. Returns true if the mouse is inside any of the elements. + * @param refElement Elements to monitor. If an array is provided, returns true if mouse is inside any element + * @param delay Time to delay state updates (in milliseconds) * @returns 返回一个数组,第一个元素是一个 ref,表示鼠标是否在元素内部,第二个元素是一个控制器,可以通过 enable 和 disable 方法来控制监听器的启用和禁用 */
41-59
: Consider using explicit return type instead of type assertion.The current implementation uses type assertion for the return value. Consider using an explicit return type for better type safety and readability.
+type HoverToggleReturn = [Ref<boolean>, { enable: () => void; disable: () => void }]; export function useHoverToggle( refElement: Arrayable<MaybeElementRef>, delay: (() => number) | number = 500, -) { +): HoverToggleReturn { // ... existing implementation ... - return [value, controller] as [typeof value, typeof controller]; + return [value, controller]; }packages/effects/layouts/src/widgets/user-dropdown/user-dropdown.vue (1)
185-186
: Consider improving hover behavior between trigger and contentThe current implementation might cause the dropdown to close unexpectedly when the user moves the cursor from the trigger to the content area, as there could be a gap between these elements.
Consider adding a small delay before closing or ensuring there's no gap between trigger and content areas. You could:
- Add a small padding area to catch the mouse movement
- Increase the close delay slightly higher than the open delay
- Use a more sophisticated hover detection that accounts for cursor velocity and direction
Example implementation for option 2:
const [openPopover, hoverWatcher] = useHoverToggle( [refTrigger, refContent], - () => props.hoverDelay, + () => ({ + open: props.hoverDelay, + close: props.hoverDelay + 100, + }), );Also applies to: 194-194
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/effects/hooks/package.json
(1 hunks)packages/effects/hooks/src/index.ts
(1 hunks)packages/effects/hooks/src/use-hover-toggle.ts
(1 hunks)packages/effects/layouts/src/widgets/user-dropdown/user-dropdown.vue
(5 hunks)playground/src/layouts/basic.vue
(1 hunks)
🔇 Additional comments (5)
packages/effects/hooks/src/use-hover-toggle.ts (1)
1-8
: LGTM! Imports are well-organized and complete.
All necessary utilities and types are properly imported.
packages/effects/hooks/src/index.ts (1)
4-4
: LGTM! Export is properly integrated.
The new hook export follows the established pattern and is well-organized within the file.
playground/src/layouts/basic.vue (1)
135-135
: LGTM! Verify prop documentation
The trigger="both"
prop is correctly added to enable both click and hover triggers for the dropdown.
Let's verify the prop documentation:
✅ Verification successful
Prop is well-documented and type-safe
The trigger
prop is properly documented in the UserDropdown component with correct TypeScript types supporting all three modes: 'both', 'click', and 'hover'.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for documentation of the trigger prop
rg -A 5 "trigger.*'both'.*'click'.*'hover'"
Length of output: 585
packages/effects/layouts/src/widgets/user-dropdown/user-dropdown.vue (2)
57-60
: LGTM! Well-documented props with type safety
The new props are well-defined with:
- Clear JSDoc comments
- Type-safe trigger options
- Reasonable default values
93-112
: LGTM! Clean hover implementation with proper cleanup
The hover functionality is well-implemented using:
- Template refs for precise element targeting
- Proper watch setup with immediate execution
- Clean enable/disable logic based on trigger prop
Description
顶部的
UserDropdown
支持hover激活弹窗,也可配置hoverDelay
属性设置hover延迟打开或关闭的时间。Issue #4465
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
useHoverToggle
hook to manage hover states for specified elements.UserDropdown
component to accept atrigger
prop for customizable interaction methods.Bug Fixes
Chores