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

SVG Icons: Unable to Supply <defs> Element #4489

Closed
joncursi opened this issue Jun 13, 2016 · 9 comments
Closed

SVG Icons: Unable to Supply <defs> Element #4489

joncursi opened this issue Jun 13, 2016 · 9 comments
Labels
component: SvgIcon The React component. good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request

Comments

@joncursi
Copy link

joncursi commented Jun 13, 2016

Although the SVG font icons support a children prop, I am unable to pass in a defs element to define a custom linear gradient. I.e.:

         import Stars from 'material-ui/svg-icons/action/stars';

          <Stars
            children={
              <defs>
                <linearGradient id="MyGradient">
                  <stop offset="5%" stopColor="#F60" />
                  <stop offset="95%" stopColor="#FF6" />
                </linearGradient>
              </defs>
            }
            className="star--half"
          />

The icon renders without the defs element.

@mbrookes
Copy link
Member

mbrookes commented Jun 13, 2016

@joncursi The pre-built icons don't currently support children, as the path for the icon itself is the SvgIcon children.

import React from 'react';
import pure from 'recompose/pure';
import SvgIcon from '../../SvgIcon';

let ActionStars = (props) => (
  <SvgIcon {...props}>
    <path d="M11.99 2C6.47 2 2 6.48 2 12s4.47 10 9.99 10C17.52 22 22 17.52 22 12S17.52 2 11.99 2zm4.24 16L12 15.45 7.77 18l1.12-4.81-3.73-3.23 4.92-.42L12 5l1.92 4.53 4.92.42-3.73 3.23L16.23 18z"/>
  </SvgIcon>
);
ActionStars = pure(ActionStars);
ActionStars.displayName = 'ActionStars';
ActionStars.muiName = 'SvgIcon';

export default ActionStars;

@mbrookes
Copy link
Member

@joncursi If the prebuilt SvgIcons supported defs as children, how would you apply them to the built-in SVG paths?

@joncursi
Copy link
Author

@mbrookes I was looking to apply a linear gradient over the SVG path rather than a solid color. CSS can't do this alone, it requires DOM elements be defined like this: http://www.w3schools.com/svg/svg_grad_linear.asp

The fill name can then either be passed in as an additional prop, or specified to be used in CSS.

@mbrookes
Copy link
Member

@joncursi The problem is that some icons have multiple paths (or other elements circle etc.), so which to apply what id to?

@psylum
Copy link

psylum commented Jun 13, 2017

Requiring that SvgIcons embed the raw source of an SVG instead of some way to link or load from an external source is rather limiting. Creating these in TypeScript, you can't embed an icon-specific <style> tag if there's a class to be reused by some/all of the contained paths. it fails to compile because it interprets the css. I know the styles could potentially be applied by the caller creating the icon, or at a global level, but this seems really awkward if the goal is to encapsulate an 'icon' with this pattern.

@oliviertassinari oliviertassinari added component: SvgIcon The React component. v1 new feature New feature or request labels Aug 22, 2017
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 11, 2018

We have 3 paths going forward:

  1. We tell users to copy and paste the icon path. So we don't change anything.
  2. We make all our icons exporting the path.
  3. We add an extra property to the SvgIcon component. extraChildren or something like that.

Option 1 or 3 sounds like the best ones to me.

@oliviertassinari
Copy link
Member

@dustingraves People can already do that with Material-UI. I don't follow.

@dustingraves
Copy link

I didn't read the original problem fully... I will remove my comment to keep the issue clean

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jun 18, 2018
@oliviertassinari
Copy link
Member

Actually, I think that we have an even better solution to the problem. We can make the SvgIcon supports a component property. It's a pattern we use everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: SvgIcon The React component. good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants