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

NavLink should render child components wrapped by withRouter #5141 #5142

Closed
wants to merge 2 commits into from

Conversation

penx
Copy link
Contributor

@penx penx commented May 18, 2017

Alasdair McLeay added 2 commits May 18, 2017 20:25
NavLink should render child components wrapped by withRouter
@pshrmn
Copy link
Contributor

pshrmn commented May 18, 2017

This falls under the same umbrella as #4704. The issue is a bit broader than just <NavLink>s because it applies to any withRouter wrapped component whose parent match is null (which can only happen inside of a parent <Route children>) .

@penx
Copy link
Contributor Author

penx commented Sep 4, 2017

@ryanflorence @timdorr any chance of some feedback on this, or some progress on #4704? I've created a simple failing test in this PR which should illustrate the issue

@penx
Copy link
Contributor Author

penx commented Feb 13, 2018

@ryanflorence @timdorr @pshrmn any chance on an update on this or #4704?

Is there any point in me fixing the conflicts here or is this fix not wanted?

I would hope that the unit test I have included, along with the codepen, illustrate what the issue is, but if you want more information please ask.

@pravdomil
Copy link

#5963

@timdorr
Copy link
Member

timdorr commented Feb 22, 2018

Superceded by #5964

@timdorr timdorr closed this Feb 22, 2018
@penx
Copy link
Contributor Author

penx commented Feb 22, 2018

@timdorr unit tests in this branch are still relevant, useful and not covered by #5964, please reopen for this for tests and I'll rebase from master

@timdorr
Copy link
Member

timdorr commented Feb 22, 2018

I agree. Can you do a new PR? It'll be cleaner that way. Thanks!

@penx
Copy link
Contributor Author

penx commented Feb 22, 2018

sure

penx added a commit to penx/react-router that referenced this pull request Feb 22, 2018
timdorr pushed a commit that referenced this pull request Feb 22, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants