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

Consider allowing children in Navigation element #144

Closed
drhayes opened this Issue Nov 20, 2015 · 12 comments

Comments

Projects
None yet
6 participants
@drhayes
Contributor

drhayes commented Nov 20, 2015

I'm using redux-router and I'm relying on their Link elements. Regular <a> tags, like react-toolbox's Link component, reload the entire app instead of making the right kind of state change.

If the Navigation element allowed custom children then I'd be able to use it. Maybe inside of a <NavigationItem> element or something?

@EnzoMartin

This comment has been minimized.

Show comment
Hide comment
@EnzoMartin

EnzoMartin Nov 20, 2015

Contributor

Why not just import the stylesheet from the Link tag and use the root class to style the router Link component?

You wouldn't want to wrap another Link element with it as it'll become problematic to ensure the styling is correct since the child element could have weird CSS styles that can't be handled easily

Contributor

EnzoMartin commented Nov 20, 2015

Why not just import the stylesheet from the Link tag and use the root class to style the router Link component?

You wouldn't want to wrap another Link element with it as it'll become problematic to ensure the styling is correct since the child element could have weird CSS styles that can't be handled easily

@drhayes

This comment has been minimized.

Show comment
Hide comment
@drhayes

drhayes Nov 20, 2015

Contributor

Oh, ack -- I meant react-router. Sorry I made that more confusing!

It's not about styling. react-router Link components transition the SPA by pushing state to the HTML5 history. I mean, they also handle styling their active states, but that's secondary in my case.

Contributor

drhayes commented Nov 20, 2015

Oh, ack -- I meant react-router. Sorry I made that more confusing!

It's not about styling. react-router Link components transition the SPA by pushing state to the HTML5 history. I mean, they also handle styling their active states, but that's secondary in my case.

@EnzoMartin

This comment has been minimized.

Show comment
Hide comment
@EnzoMartin

EnzoMartin Nov 20, 2015

Contributor

Hm, I'm not sure what you're getting at?

The Link component react-router provides can be styled to look like react-toolbox's Link by importing the CSS class, they don't provide much of anything other than a small convenience so you wouldn't be gaining anything by using react-toolbox's Link

Contributor

EnzoMartin commented Nov 20, 2015

Hm, I'm not sure what you're getting at?

The Link component react-router provides can be styled to look like react-toolbox's Link by importing the CSS class, they don't provide much of anything other than a small convenience so you wouldn't be gaining anything by using react-toolbox's Link

@drhayes

This comment has been minimized.

Show comment
Hide comment
@drhayes

drhayes Nov 20, 2015

Contributor

Again: it's not about styling.

Here's a link to the handleClick method in react-router's Link component: https://github.com/rackt/react-router/blob/53623216560e34fbee02a99d926b5c69f163db35/modules/Link.js#L42

That method is calling a pushState method on a history object. That's the functionality that I'd like to take advantage of. If I can't pass react-router's Link component in to react-toolbox's Navigation component then my app refreshes the entire page to render a new view instead of doing an HTML5 history transition when using the Navigation component.

There's other functionality in react-router's Link that I'd like too. So I was hoping that react-toolbox's Navigator component could render its children under certain circumstances, like its Drawer component does:

{ props.children }

Contributor

drhayes commented Nov 20, 2015

Again: it's not about styling.

Here's a link to the handleClick method in react-router's Link component: https://github.com/rackt/react-router/blob/53623216560e34fbee02a99d926b5c69f163db35/modules/Link.js#L42

That method is calling a pushState method on a history object. That's the functionality that I'd like to take advantage of. If I can't pass react-router's Link component in to react-toolbox's Navigation component then my app refreshes the entire page to render a new view instead of doing an HTML5 history transition when using the Navigation component.

There's other functionality in react-router's Link that I'd like too. So I was hoping that react-toolbox's Navigator component could render its children under certain circumstances, like its Drawer component does:

{ props.children }

@javivelasco

This comment has been minimized.

Show comment
Hide comment
@javivelasco

javivelasco Nov 21, 2015

Member

I see what you guys mean. @drhayes you can use a regular Link or React Toolbox's with react router routing. You just need to wrap your component in a new one implementing History mixin from React Router. Check this as an example.

Anyway we should give the user the ability to define content in a navigation with children but notice that it would be kind of a silly component since is not giving too much styling. We will allow it though, I'm going to label the issue as an enhancement to do so.

Thanks! :)

Member

javivelasco commented Nov 21, 2015

I see what you guys mean. @drhayes you can use a regular Link or React Toolbox's with react router routing. You just need to wrap your component in a new one implementing History mixin from React Router. Check this as an example.

Anyway we should give the user the ability to define content in a navigation with children but notice that it would be kind of a silly component since is not giving too much styling. We will allow it though, I'm going to label the issue as an enhancement to do so.

Thanks! :)

@drhayes

This comment has been minimized.

Show comment
Hide comment
@drhayes

drhayes Nov 21, 2015

Contributor

Ah, okay! Yeah, you're both right, I see that now. Yeah, the History mixin is what I'm looking for... except I'm using ES6, but I'll maybe make an exception for this component.

Thanks for sticking with me and helping me get it through my thick skull. :)

Contributor

drhayes commented Nov 21, 2015

Ah, okay! Yeah, you're both right, I see that now. Yeah, the History mixin is what I'm looking for... except I'm using ES6, but I'll maybe make an exception for this component.

Thanks for sticking with me and helping me get it through my thick skull. :)

@soyjavi soyjavi closed this Nov 21, 2015

@javivelasco javivelasco reopened this Nov 21, 2015

@javivelasco

This comment has been minimized.

Show comment
Hide comment
@javivelasco

javivelasco Nov 21, 2015

Member

No problem @drhayes!
Let's keep it open to remember adding children to the navigation component!

Member

javivelasco commented Nov 21, 2015

No problem @drhayes!
Let's keep it open to remember adding children to the navigation component!

@javivelasco

This comment has been minimized.

Show comment
Hide comment
@javivelasco

javivelasco Nov 21, 2015

Member

Actually @drhayes it's possible to give children to a navigation component. Try in the playground:

const NavigationTest = () => (
 <Navigation type='horizontal'>
   <Button label='Action 1' />
   <Button label='Action 2' />
 </Navigation>
);
Member

javivelasco commented Nov 21, 2015

Actually @drhayes it's possible to give children to a navigation component. Try in the playground:

const NavigationTest = () => (
 <Navigation type='horizontal'>
   <Button label='Action 1' />
   <Button label='Action 2' />
 </Navigation>
);
@BerndWessels

This comment has been minimized.

Show comment
Hide comment
@BerndWessels

BerndWessels Feb 2, 2016

Hi @javivelasco @drhayes
I ran into the same problem and can't quite see how you solved it.
Can you give an example here how to use the react-router Link with react-toolbox styling for it please?
Thanks
Bernd

BerndWessels commented Feb 2, 2016

Hi @javivelasco @drhayes
I ran into the same problem and can't quite see how you solved it.
Can you give an example here how to use the react-router Link with react-toolbox styling for it please?
Thanks
Bernd

@BerndWessels

This comment has been minimized.

Show comment
Hide comment
@BerndWessels

BerndWessels Feb 2, 2016

OK, no worries. I found the perfect solution.

The react-bootstrap guys had the same issue and someone wrote a LinkContainer component that solves the problem.

And these react-router-bootstrap LinkContainer and IndexLinkContainer components also work great with react-toolbox Link components.

Here is the project for everybody who has the same issue.

Cheers
Bernd

BerndWessels commented Feb 2, 2016

OK, no worries. I found the perfect solution.

The react-bootstrap guys had the same issue and someone wrote a LinkContainer component that solves the problem.

And these react-router-bootstrap LinkContainer and IndexLinkContainer components also work great with react-toolbox Link components.

Here is the project for everybody who has the same issue.

Cheers
Bernd

@retorquere

This comment has been minimized.

Show comment
Hide comment
@retorquere

retorquere Dec 23, 2016

@javivelasco the link you pointed to here (to navigation.jsx) no longer exists.

retorquere commented Dec 23, 2016

@javivelasco the link you pointed to here (to navigation.jsx) no longer exists.

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