feat(focus indicators): Add config map to base focus indicators mixin and tweak some default styles. #19206
Conversation
googlebot
added
the
cla: yes
| /// } | ||
| @mixin mat-strong-focus-indicators-theme($themeOrColor) { | ||
| @mixin mat-strong-focus-indicators-theme($theme-or-color) { |
There was a problem hiding this comment.
@jelbourn & @mmalerba: Do we still want to have a standalone mat-strong-focus-indicators-theme mixin? One option is to merge this into the mat-strong-focus-indicators mixin (i.e. add a border-color key into the $config), but then if developers want to toggle focus indicator styles on light/dark backgrounds, a bunch of redundant styles are re-declared. Thoughts?
There was a problem hiding this comment.
IMO it makes sense to keep it, because the color depends on the theme.
| } | ||
|
|
||
| // By default, all focus indicators are flush with the bounding box of their | ||
| // host element. For particular elements (listed below), default inset/offset | ||
| // values are necessary to ensure that the focus indicator is sufficiently | ||
| // contrastive and renders appropriately. | ||
|
|
||
| .mat-focus-indicator.mat-button-base::before, |
There was a problem hiding this comment.
I updated the default inset/offset styles to (1) work nicely with the $border-width parameter and (2) more-accurately target the elements that actually need default inset/offset values. For example, previously all mat-button-base had an offset focus indicator, now only flat button, raised button, fab, and mini fab do. The rationale behind this is that offseting focus indicators is a bit dangerous from a design perspective, as it's easy for them to be clipped by adjacent elements or containers with overflow: hidden.
| } | ||
|
|
||
| // By default, all focus indicators are flush with the bounding box of their | ||
| // host element. For particular elements (listed below), default inset/offset | ||
| // values are necessary to ensure that the focus indicator is sufficiently | ||
| // contrastive and renders appropriately. | ||
|
|
||
| .mat-focus-indicator.mat-button-base::before, |
There was a problem hiding this comment.
@jelbourn & @mmalerba: If a developer wishes to add an offset to flat & raised buttons but not text buttons, how would we allow that with the per-component customization mixins? We were thinking there would be a mat-button-strong-focus-indicator mixin, but maybe we need to be a bit more specific and have mat-flat-button-strong-focus-indicator, mat-raised-button-strong-focus-indicator, etc. The button mixin could also accept a $type parameter. WDYT?
There was a problem hiding this comment.
Probably separate mixins, to be consistent with what we've done so far. so... many... mixins...
| } | ||
|
|
||
| // By default, all focus indicators are flush with the bounding box of their | ||
| // host element. For particular elements (listed below), default inset/offset | ||
| // values are necessary to ensure that the focus indicator is sufficiently | ||
| // contrastive and renders appropriately. | ||
|
|
||
| .mat-focus-indicator.mat-button-base::before, |
There was a problem hiding this comment.
Note: I was thinking that in a follow-up PR these default inset/offset styles would be replaced by the per-component customization mixins with the default inset/offset values.
| } | ||
|
|
||
| .mat-focus-indicator.mat-tab-link::before, | ||
| .mat-focus-indicator.mat-tab-label::before { | ||
| margin: $mat-focus-indicator-border-width * 2; | ||
| margin: 5px; |
There was a problem hiding this comment.
FYI: The reason why some of these. values are a function of $border-width and others are not is because the focus indicator grows inward as its width is increased.
| /// } | ||
| @mixin mat-strong-focus-indicators-theme($themeOrColor) { | ||
| @mixin mat-strong-focus-indicators-theme($theme-or-color) { |
There was a problem hiding this comment.
IMO it makes sense to keep it, because the color depends on the theme.
|
Looks like you just need to rebase this branch on master |
… and tweak some default styles.
zelliott
force-pushed
the
focus-indicators
branch
from
3ea0b54 to
d87c7ce
Compare
jelbourn
added
merge safe
action: merge
|
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 is the first PR in a series of PRs to refresh the focus indicators API in preparation for formal documentation being added. This PR includes the following changes:
mat-strong-focus-indicators(and the MDC equivalent) now both accept a$configmap that developers can use to configure focus indicator styles (e.g.border-style,border-width,border-radius).Future PRs will: