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

Allow manual closing of dropdown for links with preventDefault or possibly asynchronous operations #1630

Closed
agusterodin opened this issue Jun 30, 2022 · 26 comments · Fixed by #1897
Assignees

Comments

@agusterodin
Copy link

agusterodin commented Jun 30, 2022

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.6.6

What browser are you using?

N/A

Reproduction URL

https://github.com/agusterodin/headless-ui-dropdown-not-disappearing-after-click-bug-reproduction

Describe your issue

I am overriding a link's behavior with event.preventDefault() so that if the user clicks the link, logic runs to check if the user has any unsaved changes. Unfortunately, a recent update to Headless UI makes it so that the dropdown no longer disappears when clicking a link with event.preventDefault(). I could just make this a button element instead so I don't need event.preventDefault() but then I no longer get "open in new tab" functionality and other benefits that come with using proper semantic HTML links.

Screen.Recording.2022-06-30.at.2.24.24.PM.mov

I noticed that the popover component has a close render prop for situations where you need to manually close the popover. Could we pretty please get a close render prop for Dropdown.Item as well?

Thanks in advance. I absolutely love what you have built with the Tailwind ecosystem.

@prestonholt
Copy link

I also noticed this happening when an Inertia Link is used inside a MenuItem after upgrading to 1.6.6 (seems to work in 1.6.5), due to the preventDefault()

@sebsobseb
Copy link

I have the same issue with Inertia Link

@ravibpatel
Copy link

I encountered this with Inertia Link too.

@sebsobseb
Copy link

@RobinMalfait you think there is a solution for this? Is it an Inertia issue?

@johanmolen
Copy link

I have the same problem with Inertia Link, somebody already solved it :)?

@ravibpatel
Copy link

@johanmolen It happens due to usage of preventDefault() so as a workaround you can try using normal anchor tag and Inertia's visit function. Make sure you import Inertia before doing this.

import { Inertia } from "@inertiajs/inertia";

Before

<Link href="http://example.com">Example</Link>

After

<a href="#" @click="Inertia.visit('http://example.com', { preserveState: true })"></a>

In case you have the menu at the bottom of the page, and you want to prevent weird scroll issue before redirect due to click without preventDefault, then you can use button instead of anchor tag.

<button @click="Inertia.visit('http://example.com', { preserveState: true })"></button>

@ragulka
Copy link

ragulka commented Sep 5, 2022

The workarounds @ravibpatel provided have one important shortcoming: right-click "open in new tab" will not work anymore.

@ragulka
Copy link

ragulka commented Sep 5, 2022

The issue is that MenuItem will ignore the click if the item has a click handler that calls event.preventDefault(). It happens the render helper will skip any built-in event handlers when event.defaultPrevented is true (this is same for both vue and react versions).

For the Vue package, it looks like this was perhaps an unintentional side-effect in #1651 - the description states that when event.preventDefault() is called on the click handler, HeadlessUI will no longer try to handle that event. This behavior was copied over from the react version, which added this in #1265.

As far as I can tell, React does not support multiple click handler on the same component/element. Vue does not have this limitation, which is why this change was not introduced to the Vue package at the same time as to the React package.

I understand how this is helpful for React users so they can use their own event handler, and I can understand how it can be useful to disable the default event handling both in Vue and React.

Initially, I thought event.stopPropagation() would make more sense in this context, but the I realized that MenuItem is truly renderless and its event handlers will be merged with the component passed into its slot (meaning, the handler would still fire, even if event.stopPropagation()) was called.

It's unlikely that the event.preventDefault() handling will be backtracked (and once I understood how it works and why it was implemented, it probably shouldn't). However, the component API should then at least provide a way to manually close the menu, just as the OP described.

@DamianGlowala
Copy link

DamianGlowala commented Sep 27, 2022

I can confirm that <NuxtLink> inside a <MenuItem> doesn't close the menu either (when clicked).

@RobinMalfait, any chances you could have a look at this issue? 🙏

@RobinMalfait
Copy link
Member

Thanks for the issue and reproductions!

The issue:
Headless UI will run its default behaviour, this means that whenever you click on a Menu.Item it will invoke whatever action is attached (invoke the onClick of a button or go to the link in case of an a) and then close the Menu.

However, we also have an escape hatch built-in where you can opt-out of default behaviour, in this case if you have an onClick={e => e.preventDefault()} then it won't close the menu after it's done invoking the attached action.

This is a typical problem you see with tools like Inertia's Link component, because it intercepts the native browser action by running event.preventDefault() so that your browser doesn't make a full page refresh. Instead, it will intercept that, and use SPA-like routing to navigate you to the new spot.

Due to this event.preventDefault(), it also means that the default behaviour of Headless UI is prevented.

The solution:
What we came up with is that we now expose a close function from the Menu and Menu.Item components' render prop for React, and a close function from the Menu and MenuItem components slot prop for Vue.

This will allow you to imperatively call the close function. For example, for Vue users it would look something like this:

<template>
  <Menu>
    <MenuButton>More</MenuButton>
    <MenuItems>
      <MenuItem v-slot="{ active, close }">
        <Link href="/" preserve-state @click="close">Inertia Link {{ active }}</Link>
      </MenuItem>
    </MenuItems>
  </Menu>
</template>

<script>
import { Link } from '@inertiajs/inertia-vue3'
import { Menu, MenuButton, MenuItems, MenuItem } from '@headlessui/vue'

export default {
  components: {
    Link,
    Menu,
    MenuButton,
    MenuItems,
    MenuItem,
  },
}
</script>

For React it is similar, but it is exposed from the render prop instead.

import { Menu } from '@headlessui/react'
import { MyCustomLink } from './MyCustomLink'

function MyMenu() {
  return (
    <Menu>
      <Menu.Button>Terms</Menu.Button>

      <Menu.Items>
        <Menu.Item>
          {({ close }) => (
            <MyCustomLink href="/" onClick={close}>
              Read and accept
            </MyCustomLink>
          )}
        </Menu.Item>
      </Menu.Items>
    </Menu>
  )
}

This should be fixed by #1897, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

@cronin4392
Copy link

@RobinMalfait would it be possible to expose this for Listbox as well? I have a use case where I want to manually trigger a close of the Listbox and it does not appear that I can trigger this myself.

@SpiritusDeos
Copy link

SpiritusDeos commented Nov 23, 2022

I think it could be interesting to add it in the documentation also @RobinMalfait, I don't see any references to the close slot prop in the VueJS documentation

@RobinMalfait
Copy link
Member

Hey! This should now be documented, sorry about that!

@hymair
Copy link

hymair commented Dec 30, 2022

@RobinMalfait can this be added to Combobox as well? My use-case is I am appending an element to the ComboboxOptions and I'd like to close the dropdown when I click that element.

@mariegodon
Copy link

Hi @RobinMalfait, any updates on allowing manual close for Listbox as well?

@claide
Copy link

claide commented Mar 29, 2023

Using version "1.7.12" for vue. NuxtLink won't close the menu.

<MenuItem v-slot="{ close }">
  <nuxtLink
    @click="close"
    class="group flex w-full items-center rounded-md px-2 py-2 text-sm dark:text-white text-dark"
    to="/admin/account-settings"
  >
    Settings
  </nuxtLink>
</MenuItem>

@ian-travers
Copy link

@claide version "1.7.13" for vue. I have the same with Inertia Link. version "1.7.7" has no that issue. So you can use it for a while.

@ravibpatel
Copy link

@ian-travers I also encountered this issue with Inertia Link in newer versions.

@Youhan
Copy link

Youhan commented Jun 24, 2023

This does not close the menu on the latest version. I used v1.7.7 for it to work.

<MenuItem v-slot="{ close }">
  <NuxtLink
    @click.native="close"
    to="/admin/account-settings"
  >
    Settings
  </NuxtLink>
</MenuItem>

@idahotallpaul
Copy link

@RobinMalfait can this be added to Combobox as well? My use-case is I am appending an element to the ComboboxOptions and I'd like to close the dropdown when I click that element.

Same situation here. Did you find a workaround?

@hymair
Copy link

hymair commented Jul 8, 2023

@RobinMalfait can this be added to Combobox as well? My use-case is I am appending an element to the ComboboxOptions and I'd like to close the dropdown when I click that element.

Same situation here. Did you find a workaround?

I think at that time I used something like

<ComboboxOption
    :value="false"
    @click.prevent="yourHandler"
>
    Click me
</ComboboxOption>

@Quanttelium
Copy link

Thanks for the issue and reproductions!

The issue: Headless UI will run its default behaviour, this means that whenever you click on a Menu.Item it will invoke whatever action is attached (invoke the onClick of a button or go to the link in case of an a) and then close the Menu.

However, we also have an escape hatch built-in where you can opt-out of default behaviour, in this case if you have an onClick={e => e.preventDefault()} then it won't close the menu after it's done invoking the attached action.

This is a typical problem you see with tools like Inertia's Link component, because it intercepts the native browser action by running event.preventDefault() so that your browser doesn't make a full page refresh. Instead, it will intercept that, and use SPA-like routing to navigate you to the new spot.

Due to this event.preventDefault(), it also means that the default behaviour of Headless UI is prevented.

The solution: What we came up with is that we now expose a close function from the Menu and Menu.Item components' render prop for React, and a close function from the Menu and MenuItem components slot prop for Vue.

This will allow you to imperatively call the close function. For example, for Vue users it would look something like this:

<template>
  <Menu>
    <MenuButton>More</MenuButton>
    <MenuItems>
      <MenuItem v-slot="{ active, close }">
        <Link href="/" preserve-state @click="close">Inertia Link {{ active }}</Link>
      </MenuItem>
    </MenuItems>
  </Menu>
</template>

<script>
import { Link } from '@inertiajs/inertia-vue3'
import { Menu, MenuButton, MenuItems, MenuItem } from '@headlessui/vue'

export default {
  components: {
    Link,
    Menu,
    MenuButton,
    MenuItems,
    MenuItem,
  },
}
</script>

For React it is similar, but it is exposed from the render prop instead.

import { Menu } from '@headlessui/react'
import { MyCustomLink } from './MyCustomLink'

function MyMenu() {
  return (
    <Menu>
      <Menu.Button>Terms</Menu.Button>

      <Menu.Items>
        <Menu.Item>
          {({ close }) => (
            <MyCustomLink href="/" onClick={close}>
              Read and accept
            </MyCustomLink>
          )}
        </Menu.Item>
      </Menu.Items>
    </Menu>
  )
}

This should be fixed by #1897, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

@RobinMalfait I am on the latest version of Headless UI and of Inertia JS which is available till today's date, still the close function as per you described is not working. Please have a look at my implementation below.

    <Menu as="div" class="relative">
        <MenuButton class="-mx-1.5 flex items-center p-1.5">
            <span class="sr-only">Open user menu</span>
            <img
                class="h-6 w-6 rounded-full bg-gray-50"
                :src="$page.props.auth.user.profile_photo_url"
                alt=""
            />
            <span class="hidden lg:flex lg:items-center">
                <span
                    class="ml-4 text-standard font-medium leading-6 text-gray-200"
                    aria-hidden="true"
                >
                    {{ $page.props.auth.user.name }}
                </span>
                <ChevronDownIcon
                    class="ml-2 h-3.5 w-3.5 stroke-2 text-gray-400"
                    aria-hidden="true"
                />
            </span>
        </MenuButton>
        <transition
            enter-active-class="transition ease-out duration-100"
            enter-from-class="transform opacity-0 scale-95"
            enter-to-class="transform opacity-100 scale-100"
            leave-active-class="transition ease-in duration-75"
            leave-from-class="transform opacity-100 scale-100"
            leave-to-class="transform opacity-0 scale-95"
        >
            <MenuItems
                class="absolute right-0 z-10 mt-4 w-32 origin-top-right rounded-md bg-white py-2 shadow ring-1 ring-gray-900/5 focus:outline-none"
            >
                <MenuItem
                    v-for="item in userNavigation"
                    :key="item.name"
                    v-slot="{ active, close }"
                >
                    <Link
                        :as="item.as"
                        :href="item.href"
                        :class="[
                            active
                                ? 'bg-gray-50 text-gray-600'
                                : 'text-gray-500',
                            'block w-full px-3 py-1 text-left text-standard leading-6',
                        ]"
                        :method="item.method"
                        preserve-state
                        @click="close"
                    >
                        {{ item.name }}
                    </Link>
                </MenuItem>
            </MenuItems>
        </transition>
    </Menu>

@joshuadempsey
Copy link

Is there any updates to this issue? Running into problems with Inertia/VueJS. Not able to force the closure of the menu when using Link unfortunately.

@creazy231

This comment was marked as spam.

@siarheikryutsou
Copy link

siarheikryutsou commented Nov 9, 2023

Hi @RobinMalfait, any updates on allowing manual close for Listbox as well?

<template>
  <Listbox as="div">
    <ListboxButton> My button </ListboxButton>
    <ListboxOptions>
      <ListboxOption v-for="i in 5" :key="i" :value="i">
        <li>
          <div class="flex gap-2.5 items-center cursor-pointer" @click="onOptionClick(i)">
            <div class="">{{ i }}</div>
          </div>
        </li>
      </ListboxOption>
    </ListboxOptions>
  </Listbox>
</template>

<script setup lang="ts">
import { Listbox, ListboxButton, ListboxOption, ListboxOptions } from "@headlessui/vue";

function onOptionClick(i): void {
  navigateTo(`/${i}`);
}
</script>

@talaxasy
Copy link

[Vue] @cronin4392, @hymair, @mariegodon
Hey guys, I will show you my approach to close Listbox or Combobox manually via "ref".

For Listbox:

<template>
  <Listbox v-model="...">
    <ListboxButton ref="listboxBtnRef">Trigger button</ListboxButton>
    <ListboxOptions>
      <ListboxOption
        v-for="..."
      >
        <NuxtLink
           to="/hello"
           @click="listboxBtnRef.$el.click()"
         >
             Link text
         </NuxtLink>
      </ListboxOption>
    </ListboxOptions>
  </Listbox>
</template>

<script setup>
const listboxBtnRef = ref(null);
</script>

I'm just programmatically triggering a click on a ListboxButton

For ComboBox:

<template>
  <Combobox v-model="...">
    <ComboboxInput ref="comboboxInputRef" />
    <ComboboxOptions>
      <ComboboxOption
        v-for="..."
      >
        <NuxtLink
           to="/hello"
           @click="comboboxInputRef.$el.blur()"
         >
             Link text
         </NuxtLink>
      </ComboboxOption>
    </ComboboxOptions>
  </Combobox>
</template>

<script setup>
const comboboxInputRef = ref(null);
</script>

I haven't tested it, but I believe I can trigger the blur() action on the ComboboxInput

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

Successfully merging a pull request may close this issue.