Skip to content
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(components): [el-menu] allow user to hide menu when clicking outside #14742

Merged
merged 43 commits into from
Dec 15, 2023

Conversation

cuongle-hdwebsoft
Copy link
Contributor

@cuongle-hdwebsoft cuongle-hdwebsoft commented Nov 4, 2023

when user sets menu-trigger to click and then clicking outside, it should be close menu

closed #14738

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow contributing guide English | (中文 | Español | Français).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer to relative issues for your PR.

Description

🤖 Generated by Copilot at fb1c4cf

This pull request adds a onClickOutside feature to the sub-menu component and disables auto-formatting in VSCode. It improves the user experience of the menu and avoids potential code style issues.

Related Issue

Fixes #14738

Explanation of Changes

🤖 Generated by Copilot at fb1c4cf

  • Import onClickOutside function from @vueuse/core library to detect clicks outside of sub-menu elements (link)
  • Add mouseInChild reactive property to sub-menu component to track whether the mouse is inside the sub-menu items or not (link, link)
  • Use onClickOutside function in sub-menu component's setup function, passing the popper element and a callback that closes the sub-menu if the mouse is outside of the sub-menu trigger and items (link)

when user sets `menu-trigger` to `click` and then clicking outside, it should be close menu

closed element-plus#14738
Copy link

👋 @cuongle-hdwebsoft, thank you for contributing element-plus.

  • You can comment with /label Components:[component_name] to add a label for which component you are working on.
  • You may join our Discord for staying tuned.

Copy link

github-actions bot commented Nov 4, 2023

Hello @cuongle-hdwebsoft, thank you for contributing to element-plus, please see our guideline to see how to make contribution

Copy link

github-actions bot commented Nov 4, 2023

@github-actions github-actions bot added the CommitMessage::Qualified Qualified commit message label Nov 4, 2023
Copy link

github-actions bot commented Nov 4, 2023

🧪 Playground Preview: https://element-plus.run/?pr=14742
Please comment the example via this playground if needed.

@cuongle-hdwebsoft
Copy link
Contributor Author

Before: link

After fix: link

Steps:

  1. Open submenu
  2. Click outside
  3. The menu should be hidden
CleanShot.2023-11-04.at.13.49.03.mp4

@cuongle-hdwebsoft cuongle-hdwebsoft marked this pull request as ready for review November 4, 2023 06:49
@Dmaziyo
Copy link

Dmaziyo commented Nov 7, 2023

I was just wondering if we can provide a prop for the menu, such as "collapse-on-click-outside," to allow the user to decide whether to hide it when clicking outside.

@cuongle-hdwebsoft
Copy link
Contributor Author

I was just wondering if we can provide a prop for the menu, such as "collapse-on-click-outside," to allow the user to decide whether to hide it when clicking outside.

Thank you for your suggestion @Dmaziyo. I think that's a good spot and I will add new props collapse-on-click-outside on the next commit. 🍻

@cuongle-hdwebsoft cuongle-hdwebsoft marked this pull request as draft November 8, 2023 02:16
@cuongle-hdwebsoft
Copy link
Contributor Author

Hi @Dmaziyo you can checkout new update here

@cuongle-hdwebsoft cuongle-hdwebsoft marked this pull request as ready for review November 9, 2023 02:06
Copy link
Member

@ryuhangyeong ryuhangyeong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!

I think it's a good feature.

@ryuhangyeong
Copy link
Member

However, this is more of a feature than a bug fix.

@cuongle-hdwebsoft cuongle-hdwebsoft changed the title fix(components): allow user to hide menu when clicking outside feat(components): [el-menu] allow user to hide menu when clicking outside Nov 10, 2023
@kooriookami
Copy link
Member

Please change tag version to 2.5.0 for next upgrade. :D

@cuongle-hdwebsoft
Copy link
Contributor Author

Please change tag version to 2.5.0 for next upgrade. :D

Yes I've done @kooriookami

@kooriookami
Copy link
Member

I have a question, when I clicked the submenu, the menu hide without timeout. And when I clicked the outside, it has timeout.

QQ2023127-94835-HD.mp4

May be can refer to Dropdown,(only works when trigger is hover):
https://element-plus.org/en-US/component/dropdown.html#dropdown-attributes

image

What do you think? @btea @ryuhangyeong @chenxch

@chenxch
Copy link
Member

chenxch commented Dec 7, 2023

There are already attributes related to hide-timeout in the attributes of submenu. What kind of relationship does the hide-timeout in el-menu have?

@cuongle-hdwebsoft
Copy link
Contributor Author

cuongle-hdwebsoft commented Dec 8, 2023

There are already attributes related to hide-timeout in the attributes of submenu. What kind of relationship does the hide-timeout in el-menu have?

When clicking outside, that hide-timeout in the menu will emit/close all the opened sub-menus. @chenxch

@cuongle-hdwebsoft
Copy link
Contributor Author

cuongle-hdwebsoft commented Dec 13, 2023

I hope I can receive new feedback. Did I address your question correctly? Otherwise Could you please make it clear to me? @chenxch

@kooriookami
Copy link
Member

@cuongle-hdwebsoft I suggest that hide-timeout only works in hover mode. I refered to other framework like antd vue.

docs/en-US/component/menu.md Outdated Show resolved Hide resolved
packages/components/menu/src/menu.ts Outdated Show resolved Hide resolved
packages/components/menu/src/menu.ts Outdated Show resolved Hide resolved
packages/components/menu/src/menu.ts Outdated Show resolved Hide resolved
Copy link
Member

@FrontEndDog FrontEndDog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank your contribute! It's a great feature.

Copy link
Member

@chenxch chenxch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cuongle-hdwebsoft Sorry, I missed the message. Judging from the current results, I have no doubts. Thank you for everything.

.vscode/settings.json Outdated Show resolved Hide resolved
@kooriookami kooriookami merged commit 7f687ae into element-plus:dev Dec 15, 2023
8 checks passed
@element-bot element-bot mentioned this pull request Dec 15, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants