nextui icon indicating copy to clipboard operation
nextui copied to clipboard

[BUG] - Navbar mobile menu not closed after item is clicked

Open grigorzyapkov opened this issue 10 months ago • 10 comments

NextUI Version

2.3.0

Describe the bug

NavbarMenuItem doesn't close the menu after clicked.

<NavbarMenuItem>
      <Link
        onClick={() => {
          // do something here...
        }}
      >
         Label
      </Link>
    </NavbarMenuItem>

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

  1. Copy "With Menu" example from https://nextui.org/docs/components/navbar#with-menu
  2. Add the following item
<NavbarMenuItem>
      <Link
        onClick={() => {
          // do something here...
        }}
      >
         Label
      </Link>
    </NavbarMenuItem>
  1. Try to click on the item
  2. Navbar is not closed automatically

Expected behavior

Close navbar mobile menu after item is clicked.

Screenshots or Videos

No response

Operating System Version

macOS

Browser

Chrome

grigorzyapkov avatar Apr 16 '24 14:04 grigorzyapkov

@grigorzyapkov As of now, clicking the menu item won't close the menu. You have to listen to router's changes and close the modal in a controlled way.

wingkwong avatar Apr 16 '24 15:04 wingkwong

@wingkwong

Actually, the menu item does close the menu if href prop is provided. For example, the following will close the menu.

<NavbarMenuItem>
      <Link
        href="/some-url"
      >
         Label
      </Link>
    </NavbarMenuItem>

where, the following code won't close the menu.

<NavbarMenuItem>
      <Link
        onClick={() => {
          router.push("/some-url")
        }}
      >
         Label
      </Link>
    </NavbarMenuItem>

grigorzyapkov avatar Apr 16 '24 16:04 grigorzyapkov

please solved

nazrul-dev avatar Apr 18 '24 05:04 nazrul-dev

@nazrul-dev @grigorzyapkov please create a stackblitz project

wingkwong avatar Apr 18 '24 05:04 wingkwong

Probably need prop closeWhenClicked

Zz-ZzzZ avatar Apr 18 '24 05:04 Zz-ZzzZ

<NavbarMenu className='z-50 items-center flex flex-col'>
                {
                    NavbarLinks.map((link, index) => {
                        if (user?.accountType === "Merchant" || user?.accountType === "Admin") {
                            if (user?.accountType !== link.type) return null;
                            else {
                                return (
                                    <NavbarMenuItem

                                        onClick={() => setIsMenuOpen(false)}
                                        key={index}>
                                        {
                                            <Link href={link?.path} >
                                                <p

                                                    className={`${location.hash === '#product' ? '' : matchRoute(link?.path) ? "text-secondary-yellow" : "text-richblack-25"}`}>
                                                    {link?.title}
                                                </p>
                                            </Link>
                                        }
                                    </NavbarMenuItem>
                                )
                            }
                        }
                        else if (!link?.type) {
                            return (
                                <NavbarMenuItem key={index}>
                                    {
                                        link.title === "Shop" ? (
                                            <a href={`${link?.path}`}>
                                                <p className={`${location.hash === '#product' ? "text-secondary-yellow" : "text-richblack-25"}`}>
                                                    {link?.title}
                                                </p>
                                            </a>
                                        ) : (
                                            <Link href={link?.path} >
                                                <p className={`${location.hash === '#product' ? '' : matchRoute(link?.path) ? "text-secondary-yellow" : "text-richblack-25"}`}>
                                                    {link?.title}
                                                </p>
                                            </Link>
                                        )
                                    }
                                </NavbarMenuItem>
                            )
                        }
                    })
                }
            </NavbarMenu>

When my Link href="#product" , then the NavbarMenu will not be closed and in any other link It is working. Please give a solution.

BidyasagarAnupam avatar Apr 22 '24 10:04 BidyasagarAnupam

As in the previous comments, my links have the href prop, using the Link component from NextUI. It is only working on destop! iOS/Android completes the navigation but the menu it is not closing.

<NavbarMenuItem isActive={pathname === '/faq'}>
    <Link size="lg" href="/faq" color="foreground">
        Preguntas frecuentes
    </Link>
</NavbarMenuItem>

I added an onClick event to close the menu after clicking the link. It works correctly, but for the visitors using iOS Safari/Chrome, when they click the link, the menu closes, but it does not navigate to the corresponding page.

<NavbarMenuItem isActive={pathname === '/faq'}>
    <Link size="lg" href="/faq" color="foreground" onClick={() => setIsMenuOpen(false)}>
        Preguntas frecuentes
    </Link>
</NavbarMenuItem>

My solution

It seems to be something related with WebKit security filters. I had issues with opening links using window.open. In this case, probably, the link element is removed from the DOM before the navigation is triggered, so the event is cancelled.

The solution I developed was to add a delay to the setIsMenuOpen invocation.

const delayedSetIsMenuOpen = (val: boolean) => {
    setTimeout(() => {
        setIsMenuOpen(val)
    }, 500);
}
<NavbarMenuItem isActive={pathname === '/faq'}>
    <Link size="lg" href="/faq" color="foreground" onClick={() => delayedSetIsMenuOpen(false)}>
        Preguntas frecuentes
    </Link>
</NavbarMenuItem>

alvarolozano avatar May 10 '24 13:05 alvarolozano

Solution (Fixed)

The following solution work well for me. Infact i had implemented Accordions in <NavbarMenu />. Still this solution work great in <AccordionItem />

Important Steps to Controlled NavbarMenu

  1. Create a state to control the open and close state of the navbar
  2. Pass the state value isMenuOpen and state setter function setIsMenuOpen as a prop isMenuOpen and onMenuOpenChange respectively to <Navbar />
  3. Alter the state in a handle to onClick event of <Link /> or <Button/> what ever you want to use
const [isMenuOpen, setIsMenuOpen] = useState(false);

return (
  <Navbar
    isMenuOpen={isMenuOpen} // state value
    onMenuOpenChange={setIsMenuOpen} // state setter function
  >
    {/* Rest of Navbar code */}
    <NavbarMenu>
      <NavbarMenuItem>
        <Link
          onClick={() => {
            // do something more here before closing...
            setIsMenuOpen((prev) => !prev);
            // do something more here after closing...
          }}
        >
          Label
        </Link>
      </NavbarMenuItem>
    </NavbarMenu>
  </Navbar>
);

m-naeem66622 avatar Jun 10 '24 20:06 m-naeem66622

The most elegant solution right now, at least if you're doing straightforward links, seems to be what the NextUI website itself does:

https://github.com/nextui-org/nextui/blob/78d4216bff293ac6d673658716582caf76269de3/apps/docs/components/navbar.tsx#L63-L67

Effectively, still track open state, but rather than handling it on link clicks like others here suggest, just close it when the pathname (via Next's usePathname) changes. Note that the react-hooks/exhaustive-deps lint rule (if you're using it) is going to complain if you don't include isMenuOpen in the dependencies array. It's wrong - we don't want the effect to trigger when the state changes as it makes it impossible to open the menu. It sees isMenuOpen is different (now true) and immediately sets it to false.

FWIW I'm not sure if this is exactly what @wingkwong meant in https://github.com/nextui-org/nextui/issues/2736#issuecomment-2059411516 but they work on this project so I trust them here :)

All that said, I agree with the sentiment of the issue - in an ideal world the closing behavior is handled automatically in the component library. Needing to use Link directly here isn't ideal. IMO NavbarMenuItem should probably take an href prop, just like DropdownItem (which then also closes its menu on click)

zpao avatar Jun 28 '24 05:06 zpao