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

fix(Dropdown): close on click item with to #103

Merged
merged 7 commits into from
Oct 26, 2022
Merged

fix(Dropdown): close on click item with to #103

merged 7 commits into from
Oct 26, 2022

Conversation

smarroufin
Copy link
Contributor

@smarroufin smarroufin commented Oct 25, 2022

Fixes #98
Fixes #106

@vercel
Copy link

vercel bot commented Oct 25, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
ui ✅ Ready (Inspect) Visit Preview Oct 26, 2022 at 11:06AM (UTC)

@netlify
Copy link

netlify bot commented Oct 25, 2022

Deploy Preview for nuxthq-ui canceled.

Name Link
🔨 Latest commit e842109
🔍 Latest deploy log https://app.netlify.com/sites/nuxthq-ui/deploys/6359144f59cc1000080c3053

@@ -151,6 +151,8 @@ function onItemClick (e, item: any) {

if (item.click) {
Copy link
Member

Choose a reason for hiding this comment

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

Why does it closes here without doing your change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In any case, except with to, Menu closes on item click.
When to is defined, Menu click handling is disrupted (not sure why tho), that's why we need a manual closing.
ℹ️ If there are no to or click props, the Menu closes on item click.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should try to investigate why the click is disrupted? Maybe NuxtLink?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is NuxtLink for sure. and this is not the first time we encouter issues around this. Preventing default from click of a NuxtLink or stopping the bubbling are still a struggle. Let me find an old PR with something like that.

Copy link
Contributor Author

Still investigating on this to issue. From what I found out yet, it is related to #106

@smarroufin
Copy link
Contributor Author

smarroufin commented Oct 26, 2022

@larbish Since you use UI, from now on you should consider any event passed to a dropdown item click handler as protentially null:

const dropdownItems = [
  [{
    label: 'Edit',
    icon: 'heroicons-solid:pencil',
    click: (e) => {
      e?.preventDefault()
    }
  }]
]

Event is null on keyboard enter hit, and is defined on mouse click.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants