fix(button): focus state matches spec #467
Conversation
googlebot
added
the
cla: yes
| background-color: md-color($md-foreground, base, 0.12); | ||
| @include md-button-theme('background-color', 600); | ||
| &.md-button-focus:after { | ||
| // The button spec calls for focus on rasied buttons (and FABs) to be indicated with a |
There was a problem hiding this comment.
typo: rasied => raised
| bottom: 0; | ||
| right: 0; | ||
| content: ''; | ||
| background-color: rgba(black, 0.12); |
There was a problem hiding this comment.
Probably want to include pointer-events: none. Not sure if it's fully necessary in our use-case, but it's possibly that the pseudo element can intercept click events without it.
There was a problem hiding this comment.
This color should be able to be picked from $md-foreground with the hue-key divider? But I see, it was previously already taken from the palettes. Maybe it's intentionally?
There was a problem hiding this comment.
According to the spec, this shade should always be black 12% opacity regardless of theme.
There was a problem hiding this comment.
The UX treatments LGTM. However this looks like it only applies to raised buttons. Flat buttons still seem to be styled using background color. Is the intent to style the two types of buttons using different strategies?
|
@robertmesserle comments addressed |
| background-color: md-color($md-accent, 600); | ||
| // The focus shading for FABs needs to match the FAB's circular shape. | ||
| &.md-button-focus:after { | ||
| border-radius: 50%; |
There was a problem hiding this comment.
Would it be better to use border-radius: inherit in the declaration above instead? This way it will also respect the border radii for normal flat + raised buttons.
|
@kara I followed Travis' advice and unified the focus styles to all be done the same way. Can you take another look? |
|
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. |
R: @robertmesserle @kara
Also, @traviskaufman, could you verify that I'm interpreting the spec correctly here?
Closes #420