radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.68k stars 810 forks source link

Cannot close nested NavigationMenu.Sub #2089

Open tilds opened 1 year ago

tilds commented 1 year ago

Bug report

I am seeing some weird behaviour when creating a navigation menu with nested sub menus.

Current Behavior

When creating a nested navigation menu like in the documentation example, then I cannot close the content of a nested sub menu by clicking the trigger. It works fine on the outermost trigger, but if I click on one of the nested Sub's triggers, then I can open them but I cannot close them.

Expected behavior

I would expect the nested sub menus to close when clicking their trigger, just like the outermost trigger behaves.

Reproducible example

CodeSandbox Example

Your environment

Software Name(s) Version
Radix Package(s) react-navigation-menu 1.1.2
React n/a 17.0.1
Browser chrome 112.0.5615.121
Operating System Microsoft Windows 11 Pro 10.0.22621 Build 22621
andy-hook commented 1 year ago

Hi @tilds, the default behaviour was designed to support this style of menu.

Though depending on your use case you could control the value and close it by setting the value to empty string on trigger click.

LimeWub commented 1 year ago

Just to chime in here~

I'm also using (trying to use) the NavigationMenu for a vertical navigation and having issues with it. I understand the use case/example has been built with a horizontal menu in mind but considering orientation="vertical" is supported I would expect mine and @tilds use case to be working as well (without needing to hack them in).

Because multiple open dropdowns are not supported (makes sense on horizontal/popup version of navigation but not on vertical/mobile version) I'm trying to use nested submenus to allow for more levels of dropdowns (new value context). I'm also disabling the hover events on them as this sort of menu works better with click/tap.

FWIW: I would also expect multiple open dropdowns to be supported in the vertical setup. /Moreover/ I'm also experiencing some erratic tab focusing when using nested submenus from this implementation attempt (but these are separate bugs/issues so I don't intend to take up this thread)

Unless NavigationMenu is only actually supposed to be used to build the horizontal version of a Navigation and not the vertical (ie a stacked Mobile navigation/navigation in a sidebar) such as these? I'd expect it would only be opinionated about a11y and not about layout; but maybe it is?

The alternative is doing this functionality custom by using Accordions etc but I'd think NavigationMenu is more accessible...so I'd have to reconsider all the a11y for the mobile menu.

So, yes, +1 to this thread highlighting one of the issues with the more complex vertical use case, which doesn't seem to be playing well with some elements of vertical navigation menus which I'd expect to: Nested submenus, tab focus within nested Submenus and multiple dropdowns with the same root.

For reference here's a basic example of navigation menu style we're trying to build:

Screenshot 2023-09-15 at 14 53 08
daguitosama commented 4 months ago

Hi @tilds, the default behaviour was designed to support this style of menu.

Could anyone point on the right direction of a sample implementation or any other kind of resource on how to achieve the referenced Nested Submenu Navigation Menu with several levels please ? I'm building one like that, and some how as you hover from top to bottom on the first level item menus, the avobe items get block from changing back on hover. For instance I hover first level item-2 item-1 get's blocked for beeing selected againg on hover.

| first level item 1 | - | | first level item 2 (this is active, now i can't go back first level item 1) | - |

zjy365 commented 4 months ago

I'm facing the same problem, is there any good solution?

daguitosama commented 4 months ago

I manage to make it work đź‘Ť , using a good amount of "I would do this no matter what" vibes, definitely writing about this in the near future. The core unlocker idea (at least for me) was:

Some quick example here:

// on first level menu
// ...
<NavigationMenu.Content className=''>
       <SubMenu categoryMenus={nLink.categoryMenus} />
</NavigationMenu.Content>
</NavigationMenu.List>
<NavigationMenu.Viewport className=' absolute top-[90px] left-0 w-full  px-10 ease-in-out data-[state=open]:animate-[mega-menu-show_300ms] data-[state=closed]:animate-[mega-menu-hide_300ms]' />
// this viewport would render the submenu
//...

// submenu - level 2 and forward (the one that pops out on first level item hover)
<NavigationMenu.Sub
            onValueChange={updateSelectedImage}
            defaultValue={`${firstCategoryMenuId}`}
            className='bg-white text-black rounded-b-2xl shadow-2xl grid grid-cols-7 px-8'
        >
            <NavigationMenu.List className='py-5 text-black '>
                {categoryMenus.map((cMenu) => {
                    return (
                        <NavigationMenu.Item
                            key={cMenu.id}
                            value={`${cMenu.id}`}
                            className=''
                        >
                            <NavigationMenu.Trigger className='  w-full h-10 leading-none text-start px-2 py-3 hover:text-brand-purple transition-all ease-in-out duration-200  data-[state=open]:font-bold data-[state=open]:border-r-2 data-[state=open]:border-r-brand-purple  '>
                                {cMenu.label}
                            </NavigationMenu.Trigger>
                            <NavigationMenu.Content>
                                <NavigationMenu.List>
                                    {cMenu.links.map((link) => {
                                        return (
                                            <NavigationMenu.Item
                                                key={`inner-link-${link.id}`}
                                                className='  '
                                            >
                                                <Link
                                                    to={link.route}
                                                    className=' leading-none block h-10 px-4 py-3 text-sm hover:text-brand-purple transition-colors duration-200'
                                                >
                                                    {link.label}
                                                </Link>
                                            </NavigationMenu.Item>
                                        );
                                    })}
                                </NavigationMenu.List>
                            </NavigationMenu.Content>
                        </NavigationMenu.Item>
                    );
                })}
            </NavigationMenu.List>

            <NavigationMenu.Viewport className='py-5 col-span-4 ' />
            <div className='w-full h-full py-5 min-h-[380px] col-span-2 '>
                <div className=' relative w-full h-full rounded-2xl overflow-hidden'>
                    <img
                        src={selectedImage}
                        alt=''
                        className='absolute inset-0 w-full h-full object-cover'
                    />
                </div>
            </div>
        </NavigationMenu.Sub>

But definitely recommend getting familiiar with the primitives, and trying it out on a blank canvas environment!