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 fontSize sets by default to true and ignore width and height for SVG #10349

Closed
1 task done
palaniichukdmytro opened this issue Feb 19, 2018 · 5 comments
Closed
1 task done
Assignees
Labels
breaking change component: SvgIcon The React component. new feature New feature or request

Comments

@palaniichukdmytro
Copy link
Contributor

SVG fontSize sets by default to true and ignore width and height for SVG

  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Expected use root overrides style when fonwSize are not provided
So I have overrides for MuiSvgIcon

export default theme => ({
    root: {
        width: 18,
        height: 18,
    },
})

Current Behavior

But when I use my icon without fontSize props, it steal overrides the root style:
<Add />

screenshot_2

Context

Your Environment

Tech Version
Material-UI v1.0.0-beta.33
React 16.2
browser Chrome 64
etc
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 19, 2018

@palaniichukdmytro Would this change solve your issue?

https://github.com/mui-org/material-ui/pull/10348/files#diff-38574c080d4e2eb38c49b86e6588ad98R106

  • Remove the fontSize property from the Icon and SvgIcon components in order to make it the default behavior.
    It's already the default behavior of the Icon component. You will still be able to change the size of the icons with the width and height CSS properties. The difference is that they can use the font-size as a shorthand.

    -<SvgIcon style={{ fontSize: 20 }} fontSize />
    +<SvgIcon style={{ fontSize: 20 }} />

@palaniichukdmytro
Copy link
Contributor Author

@oliviertassinari actually better do not use fontSize as css api, and style as you mention in style.

@oliviertassinari
Copy link
Member

@palaniichukdmytro Sorry, I don't understand your point. What do you think we should change and how you would use the component with this change?

@palaniichukdmytro
Copy link
Contributor Author

As you can see here when I use svg icon itself without fontSize boolean props , it works as expected. But when I wrapped to IconButton the svg inside use fontsize , but I did not provide it
https://codesandbox.io/s/jnwrq7zv0v

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 19, 2018

@palaniichukdmytro Thanks. I confirm the plan change will solve your issue. Basically, we will remove the fontSize style to merge it into the root one. The ButtinIcon will apply a font size. Your override will work as expected.

@oliviertassinari oliviertassinari added new feature New feature or request component: SvgIcon The React component. breaking change labels Feb 19, 2018
@oliviertassinari oliviertassinari self-assigned this Feb 25, 2018
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Feb 25, 2018
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Feb 26, 2018
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Feb 26, 2018
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Feb 26, 2018
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Feb 26, 2018
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Feb 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: SvgIcon The React component. new feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants