-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(menu): support icons #1702
Conversation
```html | ||
<md-menu #menu="mdMenu"> | ||
<button md-menu-item> | ||
<md-icon> dialpad </md-icon> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why so much indent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inattention :) Will fix
@@ -18,6 +18,10 @@ | |||
color: md-color($foreground, 'disabled'); | |||
} | |||
|
|||
md-icon { | |||
color: md-color($foreground, 'icon'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the color in the spec and in Material 1's version. It looks pretty dramatic without it.
@@ -42,4 +42,12 @@ $md-menu-vertical-padding: 8px !default; | |||
&[disabled] { | |||
cursor: default; | |||
} | |||
|
|||
md-icon { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a link to the spec we could include here for this 16px value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, as far as I could tell, no exact measurement for this case in the spec. This was the value in Material 1.
LGTM |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR adds CSS support for icons.
Closes #1388
r: @hansl
cc: @jelbourn