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

[Button] stripping href from containerElement returning a tag #5968

Closed
petrogad opened this issue Jan 19, 2017 · 3 comments
Closed

[Button] stripping href from containerElement returning a tag #5968

petrogad opened this issue Jan 19, 2017 · 3 comments
Labels
bug 🐛 Something doesn't work component: button This is the name of the generic UI component, not the React module!

Comments

@petrogad
Copy link

Problem description

react-router Link elements do not support external links by default. There are comments in their library that show you how to handle externalizing these links by creating a HOC to essentially check for the host, if it's external return an a tag instead of the react-router link tag.

import React, {Component, PropTypes} from 'react';
import {Link as ReactLink} from 'react-router';

export default class Link extends Component {

    static propTypes = {
        to: PropTypes.string,
        children: PropTypes.element
    };

    render() {
        const {to, children, ...rest} = this.props;
        const toLocation = this.parseTo(to);
        const isInternal = this.isInternal(toLocation);

        const anchorAttrs = {};

        const toIsString = isString(to);
        if (toIsString && to.length > 0) {
            anchorAttrs.href = to;
            anchorAttrs.target = '_blank';
        }

        const toIsEmptyStr = toIsString && to.length === 0;

        if (isInternal && !toIsEmptyStr) {
            return <ReactLink to={toLocation.pathname} {...rest}>{children}</ReactLink>;
        }
        return <a {...anchorAttrs} {...rest}>{children}</a>;
    }

    isInternal(toLocation) {
        return window.location.host === toLocation.host;
    }

    parseTo(to) {
        const parser = document.createElement('a');
        parser.href = to;
        return parser;
    }
}

The problem is in Material-UI (a most wonderful library may I add) when you use a containerElement attribute on a MenuItem or a tab IF that container element returns an a tag, it strips out the href attribute. This is not the case on RaisedButton.

I would be happy to contribute and fix the issue myself, but I haven't been able to debug where this stripping of the href tag happens on MenuItem or Tab.

IE:

const page = {
   'slug': 'https://github.com/callemall/material-ui/issues/new',
   'title': 'Material-UI FTW'
};

<Tab label={page.title} containerElement={<Link to={page.slug}/>}/>
<MenuItem containerElement={<Link to={page.slug} />} primaryText={page.title} />

Versions

  • Material-UI: 0.16.7
  • React: 15.4.0
  • Browser: Chrome Version 55.0.2883.95
@petrogad petrogad changed the title MenuItem and Tab stripping href from containerElement returning a tag. [Bug] MenuItem and Tab stripping href from containerElement returning a tag. Jan 19, 2017
@mbrookes mbrookes changed the title [Bug] MenuItem and Tab stripping href from containerElement returning a tag. [MenuItem][Tab] stripping href from containerElement returning a tag. Jan 19, 2017
@mbrookes mbrookes added component: MenuItem component: tabs This is the name of the generic UI component, not the React module! labels Jan 19, 2017
@petrogad
Copy link
Author

Can anyone point me to the slot I could fix this at?

@oliviertassinari oliviertassinari added component: button This is the name of the generic UI component, not the React module! and removed component: MenuItem component: tabs This is the name of the generic UI component, not the React module! labels Apr 24, 2017
@oliviertassinari oliviertassinari changed the title [MenuItem][Tab] stripping href from containerElement returning a tag. [Button] stripping href from containerElement returning a tag Apr 24, 2017
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Apr 24, 2017
@oliviertassinari
Copy link
Member

This is not the case on RaisedButton.

I would expect the same issue on that component as the root cause seems to be in that EnhancedButton.js component.

Could you try the following diff on your custom Link component?

    render() {
-        const {to, children, ...rest} = this.props;
+        const {to, children, href, ...rest} = this.props;

@oliviertassinari
Copy link
Member

We have been porting the component on the v1-beta branch. We reimplemented it from the ground-up. While we haven't tested it, I think that the issue is most likely fixed on that branch. Hence, I'm closing it.
Still, we will accept PR fixes until v1-beta takes over the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: button This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

3 participants