themesberg / flowbite-react

Official React components built for Flowbite and Tailwind CSS
https://flowbite-react.com
MIT License
1.77k stars 395 forks source link

`Link` from react-router-dom isn't working with Navbar.Link & Sidebar.Link #1279

Closed dhavalveera closed 4 months ago

dhavalveera commented 4 months ago

Hello,

I was working with Nextjs App, especially Pages Router and late on Jan 24 I started exploring & loving the Tailwind CSS and then Flowbite React, and I started using Navbar component from the Flowbite React package, where in the Navbar.Link we can able to pass the Link component from the next/link package, and it was working perfectly fine.

I came across the Remix recently and was just exploring how good & better it is compared to Nextjs, and I was just playing around and I found some issues especially with Navbar.Link & Sidebar.Item components respectively. As the Remix is built by the Developer of React Router the Link component is the same as:

import { Link } from 'react-router-dom'; but in Remix it is as import { Link } from '@remix-run/react', and the working is quite same,

import { Link } from '@remix-run/react

return (
<Navbar fluid rounded>
        <Navbar.Brand as={Link} href="https://flowbite-react.com">
          <img src="/favicon.svg" className="mr-3 h-6 sm:h-9" alt="Flowbite React Logo" />
          <span className="self-center whitespace-nowrap text-xl font-semibold dark:text-white">Flowbite React</span>
        </Navbar.Brand>
        <Navbar.Toggle />
        <Navbar.Collapse>
          <Navbar.Link href="/" active>
            Home
          </Navbar.Link>
          <Navbar.Link as={Link} href="/about">
            About
          </Navbar.Link>
          <Navbar.Link href="/services">Services</Navbar.Link>
          <Navbar.Link href="/priicng">Pricing</Navbar.Link>
          <Navbar.Link href="/contact">Contact</Navbar.Link>
        </Navbar.Collapse>
      </Navbar>       <Navbar.Brand as={Link} href="https://flowbite-react.com">
          <img src="/favicon.svg" className="mr-3 h-6 sm:h-9" alt="Flowbite React Logo" />
          <span className="self-center whitespace-nowrap text-xl font-semibold dark:text-white">Flowbite React</span>
        </Navbar.Brand>
        <Navbar.Toggle />
        <Navbar.Collapse>
          <Navbar.Link href="/" active>
            Home
          </Navbar.Link>
          <Navbar.Link as={Link} href="/about">
            About
          </Navbar.Link>
          <Navbar.Link href="/services">Services</Navbar.Link>
          <Navbar.Link href="/priicng">Pricing</Navbar.Link>
          <Navbar.Link href="/contact">Contact</Navbar.Link>
        </Navbar.Collapse>
      </Navbar>
)

as you can see in the above code snippet, where I am passing the Link component to Navbar.Link as shown above, but in the DOM it's getting rendered as:

<a class="block py-2 pr-4 pl-3 md:p-0 border-b border-gray-100 text-gray-700 hover:bg-gray-50 dark:border-gray-700 dark:text-gray-400 dark:hover:bg-gray-700 dark:hover:text-white md:border-0 md:hover:bg-transparent md:hover:text-cyan-700 md:dark:hover:bg-transparent md:dark:hover:text-white" href="/">About</a>

the hack over here is Link from React-Router using to params for the URL path, whereas in Nextjs, it's using href.

but I am unable to figure out why this is not working especially with React & Remix but working correctly with Nextjs, and this is a potential bug, as this package/library can be used by React, Remix & Nextjs, and similar React based Framework as well.

I would love & happy to help/contribute to fixing this bug.

SutuSebastian commented 4 months ago

Did u try simply replacing href with to prop and see if it works as intended?

dhavalveera commented 4 months ago

Did u try simply replacing href with to prop and see if it works as intended?

I just tried now, and it worked, as I wasn't sure that replacing the href to to will work or not, as in the Navbar.Link code, we're creating an interface using ComponentProps<'a'> and in that we're having href as an optional value, so got confused due to which I created this Bug Issue.

dhavalveera commented 4 months ago

Furthermore @SutuSebastian , I want to ask, like show we've a updated Documentation for both Navbar.Link & Sidebar.Item that if anyone is using React-Router/Remix Link component, use to instead of href?

does that make sense? just a thought, I can update the docs by adding a kind of note somewhere.

SutuSebastian commented 4 months ago

Furthermore @SutuSebastian , I want to ask, like show we've a updated Documentation for both Navbar.Link & Sidebar.Item that if anyone is using React-Router/Remix Link component, use to instead of href?

does that make sense? just a thought, I can update the docs by adding a kind of note somewhere.

We cannot have example docs for every single technology out there, it would be a mess to maintain, at least for now. We can maybe reference in the Remix Guide that components that have navigation tags (such as a or Link) need to follow Remix's API layer, instead of default.

dhavalveera commented 4 months ago

Furthermore @SutuSebastian , I want to ask, like show we've a updated Documentation for both Navbar.Link & Sidebar.Item that if anyone is using React-Router/Remix Link component, use to instead of href? does that make sense? just a thought, I can update the docs by adding a kind of note somewhere.

We cannot have example docs for every single technology out there, it would be a mess to maintain, at least for now. We can maybe reference in the Remix Guide that components that have navigation tags (such as a or Link) need to follow Remix's API layer, instead of default.

Yeah, that would be great, would you like me to add a small section like a note in the Remix Guide?

SutuSebastian commented 4 months ago

There will be a global navigation context where the user passes the navigate function, this way there is no need for as={Link} anymore, anywhere, since all the navigation links from internal components (Navbar, Breadcrumbs, etc) will be using it.

Note: this is coming in the near future.

SutuSebastian commented 4 months ago

Furthermore @SutuSebastian , I want to ask, like show we've a updated Documentation for both Navbar.Link & Sidebar.Item that if anyone is using React-Router/Remix Link component, use to instead of href? does that make sense? just a thought, I can update the docs by adding a kind of note somewhere.

We cannot have example docs for every single technology out there, it would be a mess to maintain, at least for now. We can maybe reference in the Remix Guide that components that have navigation tags (such as a or Link) need to follow Remix's API layer, instead of default.

Yeah, that would be great, would you like me to add a small section like a note in the Remix Guide?

Sure.