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

[icons] Remove clip-path from all icons #12452

Merged

Conversation

kevinnorris
Copy link
Contributor

@kevinnorris kevinnorris commented Aug 9, 2018

There were 50 icons affected.

Before clip-path was removed:
icons-with-clip-path

After clip-path was removed:
icons-without-clip-path

Closes #12441

@oliviertassinari oliviertassinari changed the title Remove clip-path from all icons [icons] Remove clip-path from all icons Aug 9, 2018
@oliviertassinari
Copy link
Member

I confirm, it looks all right

capture d ecran 2018-08-09 a 10 21 24

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: SvgIcon The React component. labels Aug 9, 2018
@@ -2,5 +2,5 @@ import React from 'react';
import createSvgIcon from './utils/createSvgIcon';

export default createSvgIcon(
<React.Fragment><defs><path id="a" d="M24 24H0V0h24v24z" /></defs><clipPath id="b"><use overflow="visible" xlinkHref="#a" /></clipPath><path d="M3 4V1h2v3h3v2H5v3H3V6H0V4h3zm3 6V7h3V4h7l1.83 2H21c1.1 0 2 .9 2 2v12c0 1.1-.9 2-2 2H5c-1.1 0-2-.9-2-2V10h3zm7 9c2.76 0 5-2.24 5-5s-2.24-5-5-5-5 2.24-5 5 2.24 5 5 5zm-3.2-5c0 1.77 1.43 3.2 3.2 3.2s3.2-1.43 3.2-3.2-1.43-3.2-3.2-3.2-3.2 1.43-3.2 3.2z" clipPath="url(#b)" /></React.Fragment>
<React.Fragment><defs><path id="a" d="M24 24H0V0h24v24z" /></defs><clipPath id="b"><use overflow="visible" xlinkHref="#a" /></clipPath><path d="M3 4V1h2v3h3v2H5v3H3V6H0V4h3zm3 6V7h3V4h7l1.83 2H21c1.1 0 2 .9 2 2v12c0 1.1-.9 2-2 2H5c-1.1 0-2-.9-2-2V10h3zm7 9c2.76 0 5-2.24 5-5s-2.24-5-5-5-5 2.24-5 5 2.24 5 5 5zm-3.2-5c0 1.77 1.43 3.2 3.2 3.2s3.2-1.43 3.2-3.2-1.43-3.2-3.2-3.2-3.2 1.43-3.2 3.2z" /></React.Fragment>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the clipPath definition also be removed?

Copy link
Member

@oliviertassinari oliviertassinari Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<use overflow="visible" xlinkHref="#a" /> might prevent it. I know I couldn't remove all this crap at once. I have tried in the past. It was breaking some icons.
Given how paintful it's to iterate on the script that generates the icons 2min+ for each iteration. I would say, let's look into it in another pull request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevinnorris If you have some time, it would be great to explore @mbrookes suggestion :).

@oliviertassinari oliviertassinari merged commit 8029207 into mui:master Aug 9, 2018
@oliviertassinari
Copy link
Member

@kevinnorris It's a great first pull request on Material-UI 👌🏻. Thank you for giving it a shot!

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: SvgIcon The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants