Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: NavLink isn't triggered for routes with params that need uriEncoding #9604

Closed
lifeiscontent opened this issue Nov 17, 2022 · 5 comments · Fixed by #9589
Closed

[Bug]: NavLink isn't triggered for routes with params that need uriEncoding #9604

lifeiscontent opened this issue Nov 17, 2022 · 5 comments · Fixed by #9589
Assignees
Labels

Comments

@lifeiscontent
Copy link

What version of React Router are you using?

6.4.3

Steps to Reproduce

I have a route called:

/profiles/:username

where if someone has a username like Magda Parry the browser properly encodes the URI to /profiles/Magda%20Parry but NavLink doesn't trigger an active state when it should. e.g:

                  <NavLink
                    className={(props) =>
                      clsx("nav-link", {
                        active: props.isActive,
                      })
                    }
                    end
                    to={`/profiles/${username}`}
                  >
                    My Articles
                  </NavLink>

where username here is a param from useParams

Expected Behavior

I'd expect even in cercumstances where a route param needs URI Encoding, NavLink should still trigger its active states.

Actual Behavior

doesn't trigger an active state if there are spaces within the route param.

@brophdawg11
Copy link
Contributor

I think this is related to #9580 - should be able to fix in #9589

@brophdawg11
Copy link
Contributor

Actually, not quite related - looks like this was always an issue since the incoming to was never encoded - but we have the proper utilities available to more easily fix this now 👍

@brophdawg11
Copy link
Contributor

Fixed in #9589, should be out in a prerelease next week and a stable likely the following week 👍

@lifeiscontent
Copy link
Author

lifeiscontent commented Nov 18, 2022

@brophdawg11 thank you for the work, much appreciated :)

@brophdawg11
Copy link
Contributor

This is now available in 6.4.4-pre.0 if you want to give it a shot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants