Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-icon inconsistant #7599

Closed
manish2535 opened this issue Mar 16, 2016 · 8 comments
Closed

md-icon inconsistant #7599

manish2535 opened this issue Mar 16, 2016 · 8 comments
Assignees
Labels
P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: merge ready This PR is ready for a caretaker to review type: bug

Comments

@manish2535
Copy link

Using <md-icon>cancel</md-icon> and <md-icon md-svg-src="/images/icons/ic_stop_24px.svg"/>

gives a slightly different behavior when the width of the parent is changed.

demo2

The circle icon is using the md-svg-src which shrinks as the viewport shrinks whereas the icons using <md-icon>blah</md-icon> hold their minimum width of 24px.

@topherfangio topherfangio added the needs: demo A CodePen demo or GitHub repository is needed to demonstrate the reproduction of the issue label Mar 16, 2016
@topherfangio
Copy link
Contributor

Can you please provide a simple Codepen so we can debug the issue further?

@jakubweber
Copy link

I found this issue when md-icon is a child of flexbox parent (display: flex) and hasn't enough space and md-icon doesn't hold min-width, icon and ripple is displayed wrong.

Ad-hoc fix is add to the md-button-icon min-width value.

@manish2535
Copy link
Author

Here is the demo -

http://codepen.io/manish2535/pen/VaPWBw

The last icon is using warning whereas the others are not. I think its happening because when you use md-svg-src it inserts

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fit="" height="100%" width="100%" ....

here instead of 100% in the height and width it needs 24.

If you look at the SVG files themselves they all use height="24" width="24" which is why <md-icon>warning</md-icon> works.

@topherfangio topherfangio added type: bug P1: urgent Urgent issues that should be addressed in the next minor or patch release. and removed needs: demo A CodePen demo or GitHub repository is needed to demonstrate the reproduction of the issue labels Mar 17, 2016
@devversion
Copy link
Member

@manish2535 @topherfangio I'm a bit unsure how the actual behavior should be.

If we apply a min-width/height property, then we'll break the flexbox support for md-icon directives.
In general, I can see the inconsistency but I'm thinking about the right behavior.

Maybe you have some thoughts, let me know..

@topherfangio
Copy link
Contributor

@devversion I think the issue is that the <md-icon md-svg-src="..."> icon doesn't behave like the plain <md-icon> which is unexpected.

How will it break flexbox support if we set the min-width/height like the other ones?

@devversion
Copy link
Member

@topherfangio With breaking flexbox support I mean, that these icons aren't responsive anymore. So if you have multiple icons in a row, and the viewport becomes too small, then an overflow will appear.

And in my opinion, that's taking the advantages of flexbox.
But also if we keep them responsive, then we will have really weird looking icons.

Really hard decision I think 😄

@topherfangio
Copy link
Contributor

@devversion I'm a bit confused. How does setting the width/height of these icons affect the layout of their container?

devversion added a commit to devversion/material that referenced this issue Apr 3, 2016
* Icons should not shrink on smaller viewport. (At both axis, looks weird and is inconsistent)
* Also fixes an issue in the toolbar demo, where the icons shrink, when the viewport is to small.

Fixes angular#7599
@devversion devversion added the pr: merge ready This PR is ready for a caretaker to review label Apr 3, 2016
@devversion
Copy link
Member

References #7558

ilovett pushed a commit to ilovett/material that referenced this issue Apr 22, 2016
* Icons should not shrink on smaller viewport. (At both axis, looks weird and is inconsistent)
* Also fixes an issue in the toolbar demo, where the icons shrink, when the viewport is to small.

Fixes angular#7599

Closes angular#7860
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: merge ready This PR is ready for a caretaker to review type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants