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

CSS box sizing issues when using normalize.css #2481

Closed
cgwyllie opened this issue Dec 11, 2015 · 9 comments
Closed

CSS box sizing issues when using normalize.css #2481

cgwyllie opened this issue Dec 11, 2015 · 9 comments
Labels
bug 🐛 Something doesn't work

Comments

@cgwyllie
Copy link
Contributor

We encountered an issue with SvgIcon when using a reset stylesheet which sets box-sizing: border-box globally.

EDIT: we are using the rightIcon prop of the ListItem, which adds a padding to the icon

The reset uses the inherit technique described here: https://css-tricks.com/inheriting-box-sizing-probably-slightly-better-best-practice/

However, SvgIcon seems to depend on being laid out with box-sizing: content-box for its layout to function properly (see attached files).

screen shot 2015-12-11 at 13 11 19
ActionBackup with border-box

screen shot 2015-12-11 at 13 11 30
ActionBackup with content-box

So far only icons seem affected, but there could be other components depending on content-box that we haven't used.

I'd love to hear some feedback about this. Is it considered an issue, should Material UI play nice with CSS libraries?

For now we can work around it by giving all SVGs content-box, but perhaps we should rather patch the SvgIcon component to explicitly declare the box model it needs if this is a wider issue.

Happy to put a PR together but could use some feedback first.

Thanks,
Chris

@alitaheri alitaheri added the bug 🐛 Something doesn't work label Dec 11, 2015
@alitaheri alitaheri added this to the 0.14.0 Release milestone Dec 11, 2015
@alitaheri
Copy link
Member

@cgwyllie Thanks for reporting this.

@oliviertassinari This is somehow critical i think, we should fix it before 0.14.0, it will be trivial. What do you think?

@oliviertassinari
Copy link
Member

This issus is mysterious to me.
I can't see any margin or border in the implementation of the SvgIcon component https://github.com/callemall/material-ui/blob/master/src/svg-icon.jsx.
Any idea where this is from?

@cgwyllie
Copy link
Contributor Author

@oliviertassinari Ah, sorry, I forgot to mention we encountered this when using a ListItem and setting its rightIcon prop. Appears it's a padding that is applied from https://github.com/callemall/material-ui/blob/master/src/lists/list-item.jsx#L172 that causes the layout issue.

@cgwyllie
Copy link
Contributor Author

In this case where it is the parent component introducing padding, should it have the box model explicitly set there instead?

@alitaheri
Copy link
Member

@cgwyllie Please take a look and see if #2633 fixes it.

@alitaheri
Copy link
Member

@oliviertassinari I think this is safe to close

@cgwyllie
Copy link
Contributor Author

Yeah, sorry -- haven't tested it, but looks like the PR should work.

Thanks :)

On 23 December 2015 at 22:06, Ali Taheri Moghaddar <[email protected]

wrote:

@oliviertassinari https://github.com/oliviertassinari I think this is
safe to close


Reply to this email directly or view it on GitHub
#2481 (comment)
.

@oliviertassinari
Copy link
Member

@cgwyllie Thanks for responding 💯

@calbertts
Copy link

Hi guys,

I've been trying to figure out a way to decrease the fontSize to all the components set, they are too big for my taste, so I started creating a selectable list and using styles trying to decrease the fonts and sizes but I'm seeing isn't that easy.

I've started with List and ListItem, I was able to reduce all the elements except the rightToggle element, I'm not seeing a way to override its style.

Also I couldn't change the secondaryText font size.

Is there an specific approach to achieve this without to much pain?, I'm seeing there are a lot of "magic" numbers hardcoded for the paddings and positioning, wouldn't be better make them overridable?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

No branches or pull requests

4 participants