feat: make Link accept function as children #3881
Conversation
|
Uhm, where should we put an example onto the docs? |
|
You would add a file here: https://github.com/ReactTraining/react-router/tree/v4/website/examples Just run Edit: Run that from within the |
timdorr
added
the
feature-request
modules/Link.js
Outdated
|
|
||
| // If children is a function, we are using a Function as Children Component | ||
| // so useful values will be passed down to the children function. | ||
| if(typeof children == 'function'){ |
There was a problem hiding this comment.
Minor style nitpick: but can you add spaces around the comparison parens? if (typeof children == 'function') {
modules/Link.js
Outdated
| ) { | ||
| event.preventDefault() | ||
| this.context.router.transitionTo(this.props.to) | ||
| this.handleTransition() |
There was a problem hiding this comment.
Not sure if this needs to be extracted here. I'd leave this out of the PR for now.
modules/Link.js
Outdated
| [ className, activeClassName ].join(' ').trim() : className | ||
| } | ||
| /> | ||
| >{children}</a> |
There was a problem hiding this comment.
Instead of including the children inside of the <a> (which makes a nested set of a's, which is against spec and your browser will mess with), I would if/else between children or the <a> and use cloneElement. I would also require a single child so you can do that easily.
There was a problem hiding this comment.
Uhm, sorry but I have'nt understand well. Instead of doing <a {...rest}>{children}</a> I should do <a {...rest} /> right? I don't have understand the cloneElement part.
There was a problem hiding this comment.
Ignore that part, I was thinking wrong 😄
|
Looking good! Thanks for being on the ball with this! |
Please refer to #3874
Feel free to purpose different name for children function parameters! I am not good at naming things. :(
This should also allow creating Link on react-native, not tested yet, I don't have the infra to test it right now!