Material-ui: [icons] Port icons

Created on 7 Feb 2017  路  20Comments  路  Source: mui-org/material-ui

Please port icons to next release. Or are you planning to move them into separate package? Cause now i can't move from 0.16.7 to next

SvgIcon discussion

Most helpful comment

I have just published it https://www.npmjs.com/package/material-ui-icons. Any feedback welcomed.

All 20 comments

I guess it's time to make a decision on that. @callemall/material-ui thoughts?

I think that it would be better to ship the icons in a separate package (packages/material-ui-svg-icons). It's a matter of porting the generation script that we have on the master branch.
Some points I have in mind

  • The current classification on the master branch isn't friendly. It's not straightforward to know the category we should be looking at. I suggest flattening the name of the icons, as it's done with the font ones.
  • We could consider using lerna. I have tried it. It's great.
  • I don't think that material-ui should have a dependency on material-ui-svg-icons, but the other way around. If we need some icons internally. We can duplicate them.

Okay great - we're on the same wavelength here!

I agree on the namespacing - as you know I did that already for the icons currently used for the next.

I don't really understand the problem that lerna is trying to solve.

I agree on the dependency direction. We'll be able to reduce the number of icons published with next and remove those only used for the docs. The rest (e.g. the close icon for Chip can move to internal.

Finally, we can update the script to CamelCase the generated file names.

Any update on this? :) @mbrookes I'm watching your profile carefully for the release of material-ui-icons package. Or am I wrong and are we supposed to build them ourselves using icon-builder?

@mbrookes You see, we should call that folder material-ui-icons. People get confused regarding what's it's for 馃槢 .
More seriously, what's missing in order to publish the icons?

@oliviertassinari LOL :rofl:

Nothing missing AFAIK, so long as you're happy with the published package name.

Sweet, I'm gonna try publishing this package. Then, I will give core contributors publish right.

@oliviertassinari Did you get around to publishing the package yet?

I have just published it https://www.npmjs.com/package/material-ui-icons. Any feedback welcomed.

Looks like we need a README in the published package...

Yes we do. I'm wondering if we should create a new one or improve the root one.

It needs its own. I'll see what I can come up with.

Don't icons need to be compiled in package?

@kybarg That's my mistake - I changed the target directory for the icon-builder script, forgetting that in master they subsequently get recompiled when the package is built.

Looking at the mess that babel makes in the compiled icons, I wonder if we shouldn't just rewrite template in CommonJS!

Looking at the mess that babel makes in the compiled icons, I wonder if we shouldn't just rewrite template in CommonJS!

I have looked at the output. Sure, it's not perfect but that's how most of the code is transpiled. I would avoid tweeking on that side but rather exposing an ES6 version of the icons. I mean, that I don't see much value.

@oliviertassinari returning to my question

Don't icons need to be compiled in package?

Will it be fixed or I need to tweak my webpack config to be able to use icons in my project?

@kybarg Sorry, that sentence was confusing. I think that we should be exposing two versions, an ES5 and ES2015 one.

@oliviertassinari That would be great 馃槃

@oliviertassinari That sounds like a good compromise. How would you like to structure the published package?

@mbrookes I have no solution for you. The only ES5/ES2015 diff we do with the main package is for the index. I guess ES5 version will do the job 馃槵 .

Was this page helpful?
0 / 5 - 0 ratings