[SvgIcon] Add component property

[stephenway]

Added the defs prop with testing and an example on the docs icon page.

💥 Fixes #4489

[oliviertassinari]

@stephenway What's your thought on #4489 (comment)?

[oliviertassinari]

By the way, I see that you have some experience with Sketch. Do you think it would be possible to follow https://medium.com/seek-blog/sketching-in-the-browser-33a7b7aa0526 to provide a Sketch version of Material-UI?

[stephenway]

@oliviertassinari I liked your component prop idea as it is spread out through the library, but I wasn't sure how that applied to a sub-element in this case. I opted to make a defs prop so that it stayed flexible for one or more gradients. Maybe you could elaborate on your idea?

Sketch! Yes, I haven't stayed up to date on the latest in Sketch, but I'll take a look and let you know. I've been working with Figma for material design as of late, since it's native to the browser.

[stephenway]

Curious why the ci test_browser is rejecting a markdown document that hasn't changed?

[stephenway]

@oliviertassinari Just checked out that sketch html-sketchapp article. Really love how simple it is to convert a component from react to sketch. Could prove to be a nice addition to this library!

After thinking more about your component prop idea, I am still wondering how the dev would handle the svg tag if the root element got changed to something like a div or span. Mainly the concern is around the props that are already built into SvgIcon like focusable, viewBox, color, aria-hidden.

I have also observed regressions in layout on my own projects, when trying to wrap an icon around a span in certain scenarios, the box-model and spacing could change on the dev.

If there are any changes you would like in this PR just let me know. Thanks

[oliviertassinari]

Maybe you could elaborate on your idea?

I have updated the pull request to reflect this idea. The motivation is that defs is quite an edge case, I would rather provide people with full flexibility rather than having a specific solution for each edge cases. But we can still add the defs property in the future if more people ask for it.

Curious why the ci test_browser is rejecting a markdown document that hasn't changed?

We are generating the markdown with the source code. We commit it to git to prevent regression in the generation logic. It needed to be updated with the yarn docs:api command.

Could prove to be a nice addition to this library!

Yeah, I wish someone will work on that :)

If there are any changes you would like in this PR just let me know. Thanks

We are good.

[oliviertassinari]

@stephenway Thanks for the pull request and your interest in the project since 2015 :)

[stephenway]

@oliviertassinari Looked through your changes! Love the approach because it's way more flexible for more use-cases. Thanks for working with me on this and getting it merged. I'll be doing what I can in the future on new issues to help out since I am using this project for work.

I'd love to help out with the sketch html, however I don't have a lot of extra time at the moment. Maybe as things change I can look at that again.

Thanks!

[spencerdcarlson]

@stephenway @oliviertassinari can either of you give me an example of how I could use the component to add a gradient fill to an SVGIcon?

[oliviertassinari]

@spencerdcarlson The example is in this pull request.

[stephenway]

@spencerdcarlson Here's the isolated example from the docs:

import React from 'react';
import PropTypes from 'prop-types';
import { withStyles } from '@material-ui/core/styles';
import red from '@material-ui/core/colors/red';
import blue from '@material-ui/core/colors/blue';
import SvgIcon from '@material-ui/core/SvgIcon';

const styles = theme => ({
  root: {
    display: 'flex',
    justifyContent: 'center',
    alignItems: 'flex-end',
  },
  icon: {
    margin: theme.spacing.unit * 2,
  },
});

function HomeIcon(props) {
  return (
    <SvgIcon {...props}>
      <path d="M10 20v-6h4v6h5v-8h3L12 3 2 12h3v8z" />
    </SvgIcon>
  );
}

function SvgIcons(props) {
  const { classes } = props;
  return (
    <div className={classes.root}>
      <HomeIcon
        className={classes.icon}
        color="primary"
        style={{ fontSize: 36 }}
        component={svgProps => (
          <svg {...svgProps}>
            <defs>
              <linearGradient id="gradient1">
                <stop offset="30%" stopColor={blue[400]} />
                <stop offset="70%" stopColor={red[400]} />
              </linearGradient>
            </defs>
            {React.cloneElement(svgProps.children[0], { fill: 'url(#gradient1)' })}
          </svg>
        )}
      />
    </div>
  );
}

SvgIcons.propTypes = {
  classes: PropTypes.object.isRequired,
};

export default withStyles(styles)(SvgIcons);

[spencerdcarlson]

Is this available in v1.3.0 ?

[oliviertassinari]

No