Skip to content

LinkContainer for using arbitrary components as links #1965

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

Closed
taion opened this issue Sep 15, 2015 · 4 comments
Closed

LinkContainer for using arbitrary components as links #1965

taion opened this issue Sep 15, 2015 · 4 comments

Comments

@taion
Copy link
Contributor

taion commented Sep 15, 2015

Motivating example: React-Bootstrap has a NavItem component that's styled as a Bootstrap nav item. I want to be able to use this (and other components like it) as a link to a React Router route. I think the most straightforward way to do this is to have a LinkContainer component that injects href and onClick to its child. That way I can do e.g.

<LinkContainer to="/home">
  <NavItem>Home</NavItem>
</LinkContainer>

The LinkContainer component I'm using right now is:

export default class LinkContainer extends React.Component {
  constructor(props, context) {
    super(props, context);

    this.onClick = this.onClick.bind(this);
  }

  onClick(event) {
    if (this.props.disabled) {
      event.preventDefault();
      return;
    }

    Link.prototype.handleClick.call(this, event);
  }

  render() {
    const {history} = this.context;
    const {onlyActiveOnIndex, to, query, children, ...props} = this.props;

    delete props.state;
    delete props.onClick;
    props.onClick = this.onClick;
    props.href = history.createHref(to, query);
    props.active = history.isActive(to, query, onlyActiveOnIndex);

    return React.cloneElement(React.Children.only(children), props);
  }
}

I'm currently shipping this in React-Router-Bootstrap, but it might make sense as part of React Router, though I don't know if the special-casing for the disabled prop is something that makes sense outside of the React-Bootstrap context.

I already have code and tests for this, so I can PR it if you think it makes sense.

@mjackson
Copy link
Member

This has been discussed previously in #85 (and elsewhere). We're not going to support using arbitrary DOM elements as links because they are inaccessible. You're breaking the web when you do that.

@taion
Copy link
Contributor Author

taion commented Sep 20, 2015

I'm just as keen on not breaking a11y. The actual component in question looks something like:

const MenuItem = ({active, ...props}) => (
  <li className={classNames('some-bootstrap-stuff', {active})>
    <a {...props} />
  </li>
);

The intended use is something like

<Menu>
  <MenuItem href="foo">Foo</MenuItem>
  <MenuItem href="bar" active>Bar</MenuItem>
</Menu>

With React Router it'd look instead like:

<Menu>
  <LinkContainer to="/foo">
    <MenuItem>Foo</MenuItem>
  </LinkContainer>
  <LinkContainer to="/bar">
    <MenuItem>Bar</MenuItem>
  </LinkContainer>
</Menu>

As far as I'm aware, this maintains all the relevant a11y considerations, since we're still using <a>s for the links. The issue is more that the component itself has some deeper structure.

The alternative per #85 would be to re-implement every one of these components with a separate React Router-specific implementation that uses <Link> instead of <a>, but that essentially requires us to duplicate all of our components, and requires either setting them up in odd ways (why would you ever need a custom anchor component if you can just inject href?) or duplicating all the code, neither of which are particularly appealing to me.

@taion
Copy link
Contributor Author

taion commented Sep 23, 2015

@mjackson

Per our discussion earlier I concede your point that this makes it too easy to use inaccessible links and would not be a good fit to ship with React Router.

I do still need to have this in the React-Bootstrap integration, but that's my problem, not yours.

@IbiPabloM
Copy link

@taion I tried React-Router-Bootstrap but I could not get it to work. Probably not the best solution, but I did this to get react-bootstrap to play nice with react-router:

SafeAnchor.prototype.render =   function() {
    return (
      <Link to={this.props.href} {...this.props} >{this.props.children}</Link>
    );
  }

@lock lock bot locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants