Avoid rendering <a> element

[alitaheri]

This has caused many issues, when an element is created in this component there will be no way to support links, as nested elements is offensive in the eyes of HTML.

Closes #2178, #1979, #1823

@oliviertassinari Take a look at this. I don't think is can be a braeking change since an href was never specified on the link. But I'm not sure...

[oliviertassinari]

I don't think is can be a breaking change since an href was never specified on the link. But I'm not sure.

I think that it's going to be a breaking change.
I'm using the href property, I should probably not 😁.
Still, I think that we should do this change.
So, what is the best way to do the migration? Hum... hard to tell, that's a small breaking change.

[alitaheri]

@oliviertassinari How are you using href? can you give me an example? might help me better understand how we can migrate.

[oliviertassinari]

I'm using the href in a LinkExternal.browser.jsx component:

import React from 'react';
import PureRenderMixin from 'react-addons-pure-render-mixin';

const LinkExternal = React.createClass({
  propTypes: {
    children: React.PropTypes.element,
    href: React.PropTypes.string,
  },
  mixins: [
    PureRenderMixin,
  ],
  render() {
    const {
      href,
      children,
    } = this.props;

    return React.cloneElement(children, {
      href: href,
      target: '_blank',
    });
  },
});

export default LinkExternal;

@patrykkopycinski Is using it this way: https://jsfiddle.net/g3thx9e9/
@alitaheri But both examples are workarounds for the nested <a /> issue.

[alitaheri]

So you think we should introduce this breaking change? I mean it's not documented I don't think we should migrate. Is fixing those easy for you? Should I ease it a bit? I can check for the existence of href and use a if it's provided.

[oliviertassinari]

Is fixing those easy for you?

Yes, it's easy to fix.

Should I ease it a bit? I can check for the existence of href and use a if it's provided.

That sounds simple to do. I think that it's a good idea.
Since people can't use nested <a /> I suspect that they are using the href property. It could be good to add a deprecation warning too 👍.

[alitaheri]

Ok, I'll implement it tonight. 👍 Thanks for the feedback ^_^

[oliviertassinari]

I have noticed another case where the href property can be used. It's with the IconButton component. Actually, the documentation is using it.

[alitaheri]

😱 Crap. Alright then I won't deprecate it for now either. I'll check for the existence of href and render a if it's there, span otherwise... for now

[alitaheri]

@oliviertassinari I eased the migration a bit. If it's alright with you do the merging 😁

[oliviertassinari]

@alitaheri That should be good 👍.

[alitaheri]

Thanks ^_^ 🎉 🎈

[jgoux]

Could you document the usage of linkButton + containerElement={<Link to="/wherever" />} on both List and Menu section ?

[nathanmarks]

@jgoux I'm using this with success:

<Link to="/something">
  <ListItem
    primaryText="xsadasds"
  />
</Link>

[jgoux]

Indeed ! 👍
But what about Tab's link ? As Tabs component only accept Tab as child ? :)

Related issue : #3557

[nathanmarks]

@jgoux Not sure, I'll have to check that one out, thanks for bringing it up.

[mismith]

This remains an issue for me on iOS 10 Mobile Safari homescreen apps:

<MenuItem
    containerElement={<Link to={'/path'} />}
    primaryText="Link Name"
/>

This works in the regular Mobile Safari browser, but when you make that web app a homescreen app, these links begin to fail with no errors, the tap is simply eaten up. If you tap for slightly longer, it registers, but this is definitely broken. (I suspect it might have something to do with onTouchTap/onClick inconsistencies.)

Any suggestions to fix this? @nathanmarks solution works, but since it wraps the entire <MenuItem /> it also adds underlines to the contents (and likely has other side-effects).

[damianobarbati]

What about this situation? A listItem with a call-to-action in the right button.
I can't double use the containerElement cause this renders two nested and the warning

Warning: validateDOMNesting(...): cannot appear as a descendant of . See Navigation > ListItem > EnhancedButton > Link > a > ... > IconButton > EnhancedButton > Link > a.

<ListItem primaryText="Users" 
	leftIcon={<IconPerson />} containerElement={<Link to="/admin/user/list" />}
	rightIcon={<IconButton children={<IconAdd />} containerElement={<Link to="/admin/user" />} />}
/>

[oliviertassinari]

@damianobarbati That was solved on the next branch by using an absolute positioned block to escape the nesting issue. On the master branch, the best advice I can give you would be to use the onTouchTap handler to manually trigger a route action.

[anshchauhan]

@mismith could you solve your issue. I'm facing the exact same issue and its quite irritating. @oliviertassinari could you please suggest any workaround for the issue mentioned by @mismith