Skip to content

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

@willbee28

Description

@willbee28

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>

Activity

manonthemoon42

manonthemoon42 commented on Feb 18, 2019

@manonthemoon42

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?

willbee28

willbee28 commented on Feb 18, 2019

@willbee28
Author
v12

v12 commented on Apr 5, 2019

@v12
Member

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.

taion

taion commented on Apr 5, 2019

@taion
Member

In previous versions we actually just injected the active prop to children: 2c28b9d#diff-7d608450e48ccf30094f19938db3948cL95

Could we just make that the behavior again?

v12

v12 commented on Apr 5, 2019

@v12
Member

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...

taion

taion commented on Apr 5, 2019

@taion
Member

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

davidjb

davidjb commented on Mar 20, 2020

@davidjb

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

alekseykarpenko

alekseykarpenko commented on Apr 15, 2020

@alekseykarpenko

@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.

Lyubomyr

Lyubomyr commented on Mar 11, 2021

@Lyubomyr

@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.

einbulinda

einbulinda commented on Aug 2, 2021

@einbulinda

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

3 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @Lyubomyr@davidjb@v12@manonthemoon42@taion

        Issue actions

          Standard <Link> tags causing two LinkContainers to be active at once. · Issue #242 · react-bootstrap/react-router-bootstrap