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

Support react-router Link into Navigation component #851

Closed
thomasthiebaud opened this issue Oct 10, 2016 · 9 comments
Closed

Support react-router Link into Navigation component #851

thomasthiebaud opened this issue Oct 10, 2016 · 9 comments

Comments

@thomasthiebaud
Copy link

@thomasthiebaud thomasthiebaud commented Oct 10, 2016

Is it possible to support react-router Link component into the Navigation component ? I did succeeded to do it and I can not found a working example.

Even in the documentation website you are using a workaround :

import { Link } from 'react-router';
const Navigation = (props) => {
  return (
    <nav className={props.className}>
      <ul>
        <li><Link activeClassName={props.activeClassName} to='/install'>Installation</Link></li>
        <li><Link activeClassName={props.activeClassName} to='/components'>Components</Link></li>
        <li><a href='http://www.github.com/react-toolbox/react-toolbox' target='_blank'>Github</a></li>
      </ul>
    </nav>
  );
};
@javivelasco
Copy link
Member

@javivelasco javivelasco commented Oct 12, 2016

Did you figure it out?? That component does not have the best implementation though

@grrseguin
Copy link

@grrseguin grrseguin commented Oct 12, 2016

Hello,

An example will be welcomed :-).

Is there another way to implement it than with react-router-bootstrap ?

@javivelasco
Copy link
Member

@javivelasco javivelasco commented Oct 12, 2016

Actually the best solution would be to make Link aware of react router by declaring it's context so you can access the history functions to perform a redirect. I'm not considering to directly support React Router in react-toolbox because that would couple their implementation.

You can do something like this. If you need it in the navigation you can always require the factory function and inject react router Link component or your custom implementation wrapping it. Check here

@thomasthiebaud
Copy link
Author

@thomasthiebaud thomasthiebaud commented Oct 12, 2016

It is working not that bad out of the box. I close the issue because the problem came from my code. So I'm using Link from react router like this

 import React from 'react'
 import { Link } from 'react-router'
 import { AppBar, Navigation } from 'react-toolbox'
 import logo from '../assets/logo.png'
 import style from './Header.scss'

 export const Header = () => (
   <AppBar>
     <Link to='/'>
       <img src={logo} className={style.logo} />
     </Link>
     <Navigation type='horizontal' className={style.navigation}>
       <Link to='/login'>Login</Link>
       <Link to='/register'>Register</Link>
     </Navigation>
   </AppBar>
 )
@grrseguin
Copy link

@grrseguin grrseguin commented Oct 12, 2016

In fact, the react-router Link component is unusable with react-toolbox link component because the twice render a a HTML tag.

@javivelasco
Copy link
Member

@javivelasco javivelasco commented Oct 12, 2016

But still you can make it aware of router context and interact with the api writing your own wrapper :)

@retorquere
Copy link

@retorquere retorquere commented Dec 23, 2016

@javivelasco any examples? I'm trying to move a project to react-toolbox and I'm hitting this specific issue. I'm already using react-router links, and I'm looking to use it in combination with the Navigation component -- which uses Links.

@javivelasco
Copy link
Member

@javivelasco javivelasco commented Dec 23, 2016

You can decorate a Button or Link with the code shown here #984 (comment) maybe it's missing something but it exposes the idea

@retorquere
Copy link

@retorquere retorquere commented Dec 25, 2016

But the problem outlined in #1059 remains, right? Is the css-next branch a solution to these issues, and is the css-next branch broadly usable right now?

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.