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 wrapped with Link from react-router does not work in Safari #850

Closed
mariusk opened this issue Jun 15, 2015 · 37 comments
Closed

Button wrapped with Link from react-router does not work in Safari #850

mariusk opened this issue Jun 15, 2015 · 37 comments
Labels
component: button This is the name of the generic UI component, not the React module!

Comments

@mariusk
Copy link

mariusk commented Jun 15, 2015

React-router uses ... type components to create SPA-type navigation events. If I wrap any kind of material-ui button with Link, it works fine in Chrome, but not in Safari. In Safari, no navigation events happen. A workaround is to add an explicit onTouchTap handler on the button itself, but I believe this is some kind of bug.

@mairh
Copy link

mairh commented Jun 15, 2015

@mariusk Could you please tell me how did you add the Link component from react-router to Button component. For example, if I do something like the following it doesn't work.

let editLink = <Link to="editTopic"/>;
<IconButton
    linkButton={true}
    onTouchTap={editLink}
    tooltip="Edit forum">
    <i className="material-icons">edit</i>
</IconButton>

I notice that if I add linkButton={true} I need to add the route to href element, but react router Link generates the anchor tag by default.

@mariusk
Copy link
Author

mariusk commented Jun 15, 2015

Here's the actual code:

 <Link to="/login"><FlatButton onTouchTap={this.clickHandler}>{l.l('no', 'Sign in')}</FlatButton></Link>

Without the onTouchTap handler, it works in Chrome but not in Safari. With the explicit handler (which again calls react-router's transitionTo(...) it also works in safari.

@hai-cea
Copy link
Member

hai-cea commented Jun 15, 2015

This might be because FlatButton generates a <button /> element and I believe <Link /> generates an <a /> element. So you'll have:

<a><button /></a>

One solution that I can think of is to change enhanced-button to accept an element prop that will allow users to customize the container element. So you'll be able to do:

<FlatButton containerElement={<Link to="/login"/>} />

@mariusk
Copy link
Author

mariusk commented Jun 15, 2015

You're right; button inside an a element is not valid html5. Another possible solution would be to have the button styles available for a elements (in addition to button), and then simple style it with className.

@hai-cea
Copy link
Member

hai-cea commented Jun 15, 2015

Actually with #846, you'll be able to do:

<FlatButton>
  <Link to="/login/" />
</FlatButton>

@mariusk
Copy link
Author

mariusk commented Jun 15, 2015

I just tested #846. It broke the styling (button text is white on white), and the link isn't active (doesn't work). Here's the actual code I tested:

<FlatButton><Link to="/login">{l.l('no', 'Sign in')}</Link></FlatButton>

@mariusk
Copy link
Author

mariusk commented Jun 16, 2015

In addition to the color issue (which was caused by my own styling), and the fact that the link still does not work, there's also a focus issue. When hovering over the embedded a link, only the link parts gets highlighted, not the whole button.

@hai-cea
Copy link
Member

hai-cea commented Jun 17, 2015

@mariusk You should be able to do this now:

<FlatButton
  containerElement={<Link to="/login" />}
  linkButton={true}
  label={l.l('no', 'Sign in')}/>

@mariusk
Copy link
Author

mariusk commented Jun 17, 2015

That looks and works perfect, thanks, great work!

@gmaclennan
Copy link

When I switch to this I am getting EventEmitter memory leak warnings. I'm not sure whether it is the click handlers on <Link> or those on <EnhancedButton> which are not being removed properly? Maybe <Link> is not being properly unmounted?

@antoinerousseau
Copy link
Contributor

Looks like it's now working even without linkButton={true}, right? When the button has Link for containerElement it is turned into a <a> element rather than a <button>?

@sovanna
Copy link

sovanna commented Jan 31, 2016

@hai-cea nice, thx!
@antoinerousseau yes, turn into <a>, broke the one style... the text originally set to text-align: center 's changed into left..as its now a <a> element

@aolu11
Copy link

aolu11 commented Sep 6, 2016

containerElement is not documented :(

@dominikbulaj
Copy link

I see that this setup (e.g. RaisedButton width containerElement={<Link to="/register>} has CSS issue. Ripple effect is not firing properly, Actually it's firing but with so much delay that I nearly can't see it (until I tap longer such button). I'm using v0.15.4.

Any hint how to speed it up?

@wzup
Copy link

wzup commented Jan 28, 2017

Doesn't work any longer

Warning: Unknown prop linkButton on tag. Remove this prop from the element.

link button error - google chrome 2017-01-28 21 05 23

@waclock
Copy link

waclock commented Feb 6, 2017

@wzup just remove the linkButton={true}

@Shahrukh-Zindani
Copy link

@wzup If you write your code like this without linkButton prop, it should work.

<FlatButton
  containerElement={<Link to="/login" />}
/>

@RakeshBKrishnamurthy
Copy link

@hai-cea ,
hello,

how can i make a function on login page [say modal function] to popup,in the above code once it navigates after button click

@MatthewEdge
Copy link

Pardon the bump but the following still generates a warning on 0.17:

<RaisedButton
      label="Sign In"
      containerElement={<Link to="/login" />}
/>

Generates the following:

Warning: Unknown prop onTouchTap on tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop

@StephanBijzitter
Copy link

StephanBijzitter commented Apr 2, 2017

I had the same problem, so I created a reusable component that works for me.

import React from 'react';
import {Component, PropTypes} from 'react';
import FlatButton from 'material-ui/FlatButton';

export default class FlatLinkButton extends Component {

    static propTypes = {
        ...FlatButton.propTypes,
        to: PropTypes.string.isRequired,
        link: PropTypes.func.isRequired,
        label: PropTypes.node.isRequired,
    };

    render() {
        const style = Object.assign({
            color: 'white',
            marginTop: '7px'
        }, this.props.style);

        const props = {
            ...this.props,
            style,
        };

        delete props.to;
        delete props.link;

        const Link = this.props.link;

        return (
            <Link to={this.props.to}>
                <FlatButton {...props}/>
            </Link>
        );
    }
}

Usage:

import {Link} from 'react-router';
import FlatLinkButton from './FlatLinkButton';

<FlatLinkButton to="/login" link={Link} label="Login"/>

@iabouhashish
Copy link

@MatthewEdge #4670 should solve the issue

@vahdet
Copy link

vahdet commented Jul 2, 2017

In Material UI v1.0.0-alpha20, this does not seem to work for me:
<Button color="contrast" containerElement={<Link to="/login" />} linkButton={true} > Login </Button>

@HerrVoennchen
Copy link

HerrVoennchen commented Jul 9, 2017

Hi,

same problem, using v1.0.0-alpha21 for prototyping and got the message containerElement is unknown. So the property seems dropped and I ended up with this solution:

<Button raised dense color="primary" onClick={() => this.props.history.push('/project/4711')}>
  <i className="fa fa-search fa-on-button" aria-hidden="true" />Search
</Button>

@HiroAgustin
Copy link

@HerrVoennchen with the latest version v1.0.0-beta.3 you should be able to do:

const LinkWrapper = ({ ...props }) => (
  <Link {...props} />
)

And then use the wrapper as the component

<Button to={`/project/${id}`} component={LinkWrapper}>Go</Button>

@hubgit
Copy link
Contributor

hubgit commented Aug 2, 2017

Thanks for the pointer @HiroAgustin; this seems to work in v1.0.0-beta.3:

<Button
  color="primary"
  to={`/projects/${project.id}`}
  component={props => <Link {...props}/>}
>View</Button>

@m-h-t
Copy link

m-h-t commented Aug 25, 2017

In 1.0.0-beta.6 using @HiroAgustin and @hubgit's solution I'm getting a warning:

Warning: Material-UI: please provide a class to the component property.
The keyboard focus logic needs a reference to work correctly.

Any suggestions?

@limakayo
Copy link

limakayo commented Sep 1, 2017

@m-h-t for me it worked when I removed the curly braces:

<Button component={Link} to="/customers"> Customers </Button>

@m-h-t
Copy link

m-h-t commented Sep 1, 2017

@limakayo But what do you do when you would like to dynamically create the link?

@limakayo
Copy link

limakayo commented Sep 5, 2017

@m-h-t good question, I tested with the curly braces now and it worked, I don't know why

@webmobiles
Copy link

anyone does know how to add a in the title of AppBar ?, I need be able to add a link on the title for 'Travel Agency'

<AppBar
              title={"Travel Agency"}
              iconElementRight={
                <div>
                  <Link to="/customers" className="navbar"><FlatButton label="Customers" /></Link>
  									<Link to="/tours" className="navbar"><FlatButton label="Tours" /></Link>
                </div>
  							}
            />

@palaniichukdmytro
Copy link
Contributor

we can just use like this with v1-beta

<Button
  component={({...props}) => <Link to='/path' {...props} />}
>Path</Button>

@webmobiles
Copy link

webmobiles commented Oct 4, 2017

I think material-ui belong still old style ui frameworks , where you can't find all the button styles that you find, or simple are too much styles, if someone need customize in a low level like me , finally I'm happy with 'styled-components' no need more about css, saas,etc, just react pure components.. you lost a lot of time, days, hours, trying figure out how or where change something of if there is a bug, or wait the future release ..

@yofriadi
Copy link

@palaniichukdmytro thanks for the solution, but it breaks the ripple styling in BottomNavigationAction

@nayema
Copy link

nayema commented May 25, 2019

This worked for me on a material-ui button:

import React from 'react'
import Button from '@material-ui/core/Button'
import { Link } from 'react-router-dom'

const SubmitButton = () =>
  <Button
    type="submit"
    fullWidth
    variant="contained"
    color="primary"
    component={Link}
    to="/form"
  >
    Submit Form
  </Button>

@albertpak
Copy link

Tried to render button with Link with the code below, but nothing's getting rendered on the view:

<Button
  component={() => (
    <Link
      to={{
        pathname: 'hello-there',
        state: {
          hello: 'HELLO THERE!',
        },
      }}
    />
  )}
  size="small"
  variant="contained"
  color="primary"
  fullWidth={true}
>
  HELLO BUTTON
</Button>

How else can I pass props along to the next route using material-ui?

@Lee-BS-AMS
Copy link

Lee-BS-AMS commented Feb 11, 2020

<Button
component={() => (
<Link
to={{
pathname: 'hello-there',
state: {
hello: 'HELLO THERE!',
},
}}
/>
)}
size="small"
variant="contained"
color="primary"
fullWidth={true}

HELLO BUTTON

I have the same question - ever solve this?

@Sanskar49
Copy link

This worked for me on a material-ui button:

import React from 'react'
import Button from '@material-ui/core/Button'
import { Link } from 'react-router-dom'

const SubmitButton = () =>
  <Button
    type="submit"
    fullWidth
    variant="contained"
    color="primary"
    component={Link}
    to="/form"
  >
    Submit Form
  </Button>

I tried this but it gives an error saying Dont use link outside a router or something.. How do I solve this now?

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

No branches or pull requests