react-router-bootstrap icon indicating copy to clipboard operation
react-router-bootstrap copied to clipboard

Standard <Link> tags causing two LinkContainers to be active at once.

Open willbee28 opened this issue 5 years ago • 10 comments

I am using <LinkContainer>s for my <NavDropdown.Item> s and have a sidenavigation with standard <Link> tags. Problem arises when I click on normal links, the linkcontainer follows and sets that linkcontainer to active but doesn't make the previous linkcontainer not active, so I end up with two active linkcontainers. Anyone have a fix? Code is here:

<LinkContainer exact to="/init-raw" > <NavDropdown.Item onClick={() => this.setState({ isProcessOpen: false, isAnalyzeOpen: false })}> Open Analysis </NavDropdown.Item> </LinkContainer> <LinkContainer exact to="/init-alt" > <NavDropdown.Item onClick={() => this.setState({ isProcessOpen: false, isAnalyzeOpen: false })}> Open process </NavDropdown.Item> </LinkContainer>

willbee28 avatar Oct 29 '18 16:10 willbee28

I also have the exact same issue. A regular link would add active to the Nav.Link, but it doesn't remove the active from the last one.

@willbee28 , did you find any solutions for that issue?

manonthemoon42 avatar Feb 18 '19 00:02 manonthemoon42

I simply just set active to false for all links, that way it just didn't show. Worked with me.

On Sun, Feb 17, 2019, 7:50 PM Sandro <[email protected] wrote:

I also have the exact same issue. A regular link would add active to the Nav.Link, but it doesn't remove the active from the last one.

@willbee28 https://github.com/willbee28 , did you find any solutions for that issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/react-bootstrap/react-router-bootstrap/issues/242#issuecomment-464537073, or mute the thread https://github.com/notifications/unsubscribe-auth/AZHNAXWThVPGNwMOPnWuF-wBaWYkFHWOks5vOfjkgaJpZM4X_lvb .

willbee28 avatar Feb 18 '19 02:02 willbee28

I'm afraid but this seems to be related to react-bootstrap as it relies on active property passed to <Nav.Link>, however, <LinkContainer> does handle className itself and doesn't do anything with active property. Thus, a workaround is to explicitly set active to false on each <Nav.Link>, leaving the rest to react-router-bootstrap to handle.

Or, as an option, you can create a helper component like this:

const RouterNavLink = ({ children, ...props }) => (
  <LinkContainer {...props}>
    <Nav.Link active={false}>
      {children}
    </Nav.Link>
  </LinkContainer>
)

However, I think we should consider adding a way to control the behaviour of an active prop passed to the component that is wrapped. @taion, @jquense, what do you think about this? I think it's required for hassle-free use of react-router and react-bootstrap, which we try to make friendly to each other by using react-router-bootstrap.

v12 avatar Apr 05 '19 16:04 v12

In previous versions we actually just injected the active prop to children: https://github.com/react-bootstrap/react-router-bootstrap/commit/2c28b9d8bb4b38af09417e42da7de411a5936f86#diff-7d608450e48ccf30094f19938db3948cL95

Could we just make that the behavior again?

taion avatar Apr 05 '19 16:04 taion

I think we can but I'd also consider adding a mechanism to control this behaviour as it's not just react-bootstrap this library is used for. Some of the use cases would require wrapped component to not have active property changed.

Maybe additional property (something like setActive), which is set to true by default? If truthy, then active property is explicitly set for a wrapped component, otherwise, do not touch it at all.

Will require another version bump if we implement it though as the default behaviour will change and new property will be set for a wrapped component...

v12 avatar Apr 05 '19 16:04 v12

so in other places i just have an activePropName prop on the container

the bigger question might be "what's a good default?" and i think passing down the active prop as the default behavior might be better than making assumptions about class names

taion avatar Apr 05 '19 17:04 taion

Just encountered this issue -- is there any further progress? @v12's workaround still works and is pretty clean.

davidjb avatar Mar 20 '20 00:03 davidjb

@v12 Thanks for your suggestion, it almost worked for me.

ℹ️ For those who will still fail with two different links being active even after providing @v12's workaround: make sure those links are always rerendered when changing your location from outside. In my code I used key with pathname passed into:

export const MyMenu = () => {
  const {pathname} = useLocation() // previously imported from 'react-router-dom'
  return (
    <ListGroup key={pathname}>
      <LinkContainer exact to="/path/to/first"><ListGroup.Item action active={false}>First Item</ListGroup.Item></LinkContainer>
      <LinkContainer exact to="/path/to/second"><ListGroup.Item action active={false}>Second Item</ListGroup.Item></LinkContainer>
      <LinkContainer exact to="/path/to/third"><ListGroup.Item action active={false}>Third Item</ListGroup.Item></LinkContainer>
    </ListGroup>
  );
};

Hope this may help someone in future.

alekseykarpenko avatar Apr 15 '20 01:04 alekseykarpenko

@alekseykarpenko Thank you. Your solution is working just fine. But be aware that sometimes such a solution could be overkill and cause side effects. This means every time pathname will change (even some small part of it like id) ListGroup will re-render from scratch. And if you have some bigger logic inside ListGroup (e.g. list of the link depending on some data from the server) it could cause heavy re-rendering on each user click.

Lyubomyr avatar Mar 11 '21 13:03 Lyubomyr

Just used NavLink from react-router-dom and passed className="nav-link" to it. Worked just fine.

einbulinda avatar Aug 02 '21 21:08 einbulinda