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

[NavigationMenu] Option to open navigation menu on click instead of pointer enter #1630

Open
hannahcancode opened this issue Aug 26, 2022 · 31 comments
Labels
Package: react/navigation-menu Type: Enhancement Small enhancement to existing primitive/feature

Comments

@hannahcancode
Copy link

Feature request

Overview

The ability to optionally have the navigation menu trigger on click instead of on hover.

Who does this impact? Who is this for?

Would love for this to be a beginner friendly boolean prop, that turns on/off click in preference to hover.

For users building more complicated navigation menus, the ability to click to open the menu and have the menu stay open can provide a better UX, as they can move their mouse into the menu more easily, without the possibility of the menu closing accidentally. In our testing it also seems that some users will automatically go to click on the trigger and close the menu without realising.

As the button primitive already has an onClick prop being passed through it could be relatively straightforward to implement with a conditional.

Additional context

For example, vercel.com (logged out) mega menus open on click.
Smashing magazine article on click vs hover: https://www.smashingmagazine.com/2021/05/frustrating-design-patterns-mega-dropdown-hover-menus/#designing-a-better-mega-dropdown-tap-click-menu

@hannahcancode hannahcancode changed the title Option to open navigation menu on click instead of pointer enter [Dropdown Menu] Option to open navigation menu on click instead of pointer enter Aug 26, 2022
@hannahcancode hannahcancode changed the title [Dropdown Menu] Option to open navigation menu on click instead of pointer enter [DropdownMenu] Option to open navigation menu on click instead of pointer enter Aug 26, 2022
@hannahcancode hannahcancode changed the title [DropdownMenu] Option to open navigation menu on click instead of pointer enter [NavigationMenu] Option to open navigation menu on click instead of pointer enter Aug 26, 2022
@andy-hook
Copy link
Collaborator

Thanks for raising @hannahcancode, this is possible by preventing the relevant pointer events on trigger and content

https://codesandbox.io/s/navigation-menu-on-click-duwvgn?file=/App.js

however, this is not perfect as it relies on understanding the implementation details (which may change in the future).

I'll mark this as an improvement to explore.

@andy-hook andy-hook added Type: Enhancement Small enhancement to existing primitive/feature Package: react/navigation-menu labels Sep 5, 2022
@hannahcancode
Copy link
Author

Oh that's great information, thanks @andy-hook. I did dive a little into whether I could "hack" it in such a way but as you say, I didn't have the knowledge of the implementation and failed. I'll see if that helps! Would love to see it as an enhancement 😁

@elbotho
Copy link

elbotho commented Sep 8, 2022

@andy-hook thank you so much for that code sample.

My use case:
I have a horizontal menu that basically folds into an vertical accordion on smaller screens.
The hover events really messed up the navigation on devices with small screens/window sizes that use an input device with hover.

Solved with something like:

export const preventHover = (event: any) => {
  const e = event as Event
  if (window.innerWidth < 1024) e.preventDefault()
}

<NavigationMenu.Trigger
  onPointerMove={preventHover}
  onPointerLeave={preventHover}
> 

<NavigationMenu.Content
  onPointerEnter={preventHover}
  onPointerLeave={preventHover}
>

@longzheng
Copy link

@andy-hook thank you so much for that code sample.

My use case: I have a horizontal menu that basically folds into an vertical accordion on smaller screens. The hover events really messed up the navigation on devices with small screens/window sizes that use an input device with hover.

Solved with something like:

export const preventHover = (event: any) => {
  const e = event as Event
  if (window.innerWidth < 1024) e.preventDefault()
}

<NavigationMenu.Trigger
  onPointerMove={preventHover}
  onPointerLeave={preventHover}
> 

<NavigationMenu.Content
  onPointerEnter={preventHover}
  onPointerLeave={preventHover}
>

Based on my testing, only onPointerLeave closes the menu so it's not necessary to preventHover for onPointerEnter. This should be sufficient

<NavigationMenu.Content
  onPointerLeave={preventHover}
>

@justkahdri
Copy link

Hi @andy-hook! I tried using onPointerEnter and onPointerLeave and on fresh page load the navigation menu opens once with hover, then only open/close on click. How can I make it to only open on click so the behavior is consistent? Has something changed on the component behavior since this was discussed?

@joaom00
Copy link
Contributor

joaom00 commented Apr 11, 2023

Hey @justkahdri, I don't think anything has changed https://codesandbox.io/p/sandbox/pensive-shadow-j8m3lm

Can you provide a sandbox?

@justkahdri
Copy link

justkahdri commented Apr 11, 2023

You're right, nothing has changed. I was using onPointerEnter instead of onPointerMove and that caused the weird behavior. Realized after reading the sandbox 😅
Thanks!

@multiwebinc
Copy link

Just a note, on vercel.com (which was mentioned in the OP), the "Features" menu is now opened both on hover and on click. I'm wondering if they did any sort of use study and determined this was better than opening the menu only on click.

@JohnGemstone
Copy link

JohnGemstone commented May 12, 2023

I've found that a lot of mouse users will instinctively click the menu triggers to open them, which will close the menu as their "hover" state has already triggered the menu to open.

Here is Theo exhibiting this behaviour a few months ago.

theo_navigationmenu.mp4

A hack is to create mutation observers which listen to the data-state attributes of the trigger elements and prevent any onClick events within a given interval.

https://codesandbox.io/p/sandbox/peaceful-nobel-gmlbyy?file=%2FApp.tsx
(codesandbox typescript is weird so this example isnt fully typesafe)

While it's a bit messy this should give the best of both click and hover worlds. If anyone can foresee any problems with this let me know!

Cheers

@multiwebinc
Copy link

I've found that a lot of mouse users will instinctively click the menu triggers to open them

I find myself constantly doing that with this library.

@jpizzle34
Copy link

Hi all, how do you manually control the open and close state of the navigation menu in react? Do we just set the data-state attribute to open or closed on the NavigationMenu.Trigger component? It doesn't seem to be working for me at the moment.

@jpizzle34
Copy link

Hi all, how do you manually control the open and close state of the navigation menu in react? Do we just set the data-state attribute to open or closed on the NavigationMenu.Trigger component? It doesn't seem to be working for me at the moment.

oh nvm, i found the forceMount prop

@benoitgrelard
Copy link
Collaborator

Hi all, how do you manually control the open and close state of the navigation menu in react? Do we just set the data-state attribute to open or closed on the NavigationMenu.Trigger component? It doesn't seem to be working for me at the moment.

oh nvm, i found the forceMount prop

forceMount isn't how you control the open state (it's there for JS animation purpose). To control the open state, you use value and onValueChange.

pomubry added a commit to pomubry/pomubry.github.io that referenced this issue Aug 31, 2023
- radix-ui/primitives#1630 (comment)
- add `disableOutsidePointerEvents` to prevent interaction with elements outside the nav content
@coreybruyere
Copy link

@jpizzle34 @benoitgrelard can you provide an example to force an open state on a navigation item?

@joaom00
Copy link
Contributor

joaom00 commented Oct 30, 2023

@jpizzle34 @benoitgrelard can you provide an example to force an open state on a navigation item?

<NavigationMenu.Root defaultValue="some-value-item">
  ...
</NavigationMenu.Root>

// or

const [value, setValue] = React.useState('some-value-item')

<NavigationMenu.Root value={value} onValueChange={setValue}>
  ...
</NavigationMenu.Root>

@AurelianSpodarec
Copy link

So there is no official way to make the menu open on the click instead of horrible hover?

@GuiSim
Copy link

GuiSim commented Dec 13, 2023

Thanks for the workarounds!

I've also had to override onPointer events on the NavigationMenu.Viewport component to achieve the desired results.

@jwmann
Copy link

jwmann commented Jan 29, 2024

Not sure why this still isn't an option without workarounds.
Appreciate the tips! 👍

I tried adding these props:

onPointerEnter={preventHover}
onPointerLeave={preventHover}

On my Trigger, Content and Viewport but it still triggers the menu unfortunately.
Oddly it only triggers it on the first event. If I close the menu, it will no longer open on hover.

@elbotho
Copy link

elbotho commented Jan 30, 2024

@jwmann by now I prevent default the following:

  • on Trigger: onPointerEnter, onPointerLeave and onPointerMove
  • on Content: onPointerEnter, onPointerLeave

The onPointerMove could make the difference. Good luck 🤞

@jwmann
Copy link

jwmann commented Feb 2, 2024

@elbotho That definitely made the difference for me!
Cheers 👍

@Jdruwe
Copy link

Jdruwe commented Feb 12, 2024

I you as me are using a viewport element outside of the regular "flow", also add these handlers over there:

<NavigationMenu.Viewport
    onPointerEnter={event => event.preventDefault()}
    onPointerLeave={event => event.preventDefault()}
/>

@SalahAdDin
Copy link

SalahAdDin commented Mar 9, 2024

Thanks for the workarounds!

I've also had to override onPointer events on the NavigationMenu.Viewport component to achieve the desired results.

how? Can you point us to it?

I you as me are using a viewport element outside of the regular "flow", also add these handlers over there:

<NavigationMenu.Viewport
    onPointerEnter={event => event.preventDefault()}
    onPointerLeave={event => event.preventDefault()}
/>

What os the regular "flow"?

@GuiSim
Copy link

GuiSim commented Mar 9, 2024

@SalahAdDin

Like this

<NavigationMenu.Viewport
    onPointerEnter={event => event.preventDefault()}
    onPointerLeave={event => event.preventDefault()}
/>

@EastTexasElectronics
Copy link

Glad this got added in thanks for the help!!

@AChangXD
Copy link

AChangXD commented Jun 3, 2024

Hi @andy-hook! I tried using onPointerEnter and onPointerLeave and on fresh page load the navigation menu opens once with hover, then only open/close on click. How can I make it to only open on click so the behavior is consistent? Has something changed on the component behavior since this was discussed?

This worked for me, hope this gets added as a prop (disableHover) or something like that

@kdevay
Copy link

kdevay commented Jun 11, 2024

<NavigationMenu.Root value={value} onValueChange={setValue}>

Can anyone explain this further? It seems like you should be able to pass in an isOpen prop, but the value prop only accepts strings...do I pass in "closed"/"open"?

@daprieu
Copy link

daprieu commented Jun 12, 2024

Hey @kdevay, It looks like you're trying to pass isOpen as a boolean to the value prop, correct? Unfortunately, this won't work with this component. The value prop requires a string because the NavigationMenu can contain multiple items, and each needs a unique string identifier to determine which item to display.

I created a codesandbox demo hope this helps.

@bennkingy
Copy link

When we getting the prop? <3

@rhenandias
Copy link

rhenandias commented Sep 11, 2024

If anyone has gotten this far using Shadcn/ui:

For me, the solution was to add event.preventDefault() to the Trigger component for the onPointerEnter, onPointerMove and onPointerLeave.

I modified the component NavigationMenuTrigger in @components/ui/navigation-menu.tsx to receive the prop “triggerMode”, which accepts the values “hover” and “click” ("hover" by default).

It looks like this:

const NavigationMenuTrigger = React.forwardRef<
  React.ElementRef<typeof NavigationMenuPrimitive.Trigger>,
  React.ComponentPropsWithoutRef<typeof NavigationMenuPrimitive.Trigger> & {
    triggerMode?: "click" | "hover";
  }
>(({ className, children, triggerMode = "hover", ...props }, ref) => (
  <NavigationMenuPrimitive.Trigger
    ref={ref}
    className={cn(navigationMenuTriggerStyle(), "group", className)}
    {...props}
    {...(triggerMode === "click" && {
      onPointerEnter: (event) => event.preventDefault(),
      onPointerMove: (event) => event.preventDefault(),
      onPointerLeave: (event) => event.preventDefault(),
    })}
  >
    {children}{" "}
    <ChevronDown
      className="relative top-[1px] ml-1 h-3 w-3 transition duration-200 group-data-[state=open]:rotate-180"
      aria-hidden="true"
    />
  </NavigationMenuPrimitive.Trigger>
));
NavigationMenuTrigger.displayName = NavigationMenuPrimitive.Trigger.displayName;

Now just use it in the application composition:

<NavigationMenu>
  <NavigationMenuList>
    <NavigationMenuItem>
      <NavigationMenuTrigger triggerMode="click">
          ...
      </NavigationMenuTrigger>
      <NavigationMenuContent>
          ...
      </NavigationMenuContent>
    </NavigationMenuItem>
  </NavigationMenuList>
</NavigationMenu>

Again, this solution works if you use shadcn/ui, my intention is not to propose a change to Radix, I just couldn't find an appropriate issue in the correct repository

@MrOxMasTer
Copy link

MrOxMasTer commented Sep 26, 2024

In my opinion there is still missing functionality along with this click, if you leave elements in the main thread and not output them in absolute, then leave them enabled.

Tho not close an element if any of them was already open.

@heyitsarpit
Copy link

I've found that a lot of mouse users will instinctively click the menu triggers to open them, which will close the menu as their "hover" state has already triggered the menu to open.

Here is Theo exhibiting this behaviour a few months ago.

theo_navigationmenu.mp4
A hack is to create mutation observers which listen to the data-state attributes of the trigger elements and prevent any onClick events within a given interval.

https://codesandbox.io/p/sandbox/peaceful-nobel-gmlbyy?file=%2FApp.tsx (codesandbox typescript is weird so this example isnt fully typesafe)

While it's a bit messy this should give the best of both click and hover worlds. If anyone can foresee any problems with this let me know!

Cheers

I'm able to have the best of both solution by increasing the delayDuration, set delayDuration={1000}.
This way I get the onClick behaviour before the hover takes effect and ones the menu is opened the hover effect will keep working as usual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: react/navigation-menu Type: Enhancement Small enhancement to existing primitive/feature
Projects
None yet
Development

No branches or pull requests