fix(material/stepper): add styling to stepper for high contrast mode #23526
Conversation
google-cla
bot
added
the
cla: yes
amysorto
added
Accessibility
| @@ -15,6 +16,25 @@ | |||
| $warn: map.get($config, warn); | |||
|
|
|||
| .mat-step-header { | |||
| @include a11y.high-contrast(active, off) { | |||
| margin-bottom: 1px; | |||
| margin-top: 1px; | |||
There was a problem hiding this comment.
Why do we need this margin? In general I've been trying to avoid changing any layout properties for high contrast mode.
There was a problem hiding this comment.
Yeah I was pretty hesistant about this, but removing it causes slight weird overlapping (like the overhead text when typing into the input) over some of the outlines. I am not sure how big of an issue that is but I ended up removing it.
| margin-top: 1px; | ||
|
|
||
| &:not([aria-disabled]), | ||
| &[aria-disabled='false'] { |
There was a problem hiding this comment.
Shouldn't we have the outline around all steps, even if they're disabled?
There was a problem hiding this comment.
I thought about that, but I removed the outline since now if you hover over the disabled button there is no styling so it feels similar here.
Although since I changed the selected step to have an underline, it looks very strange without an outline so now all buttons have outlines.
| } | ||
|
|
||
| &[aria-selected='false'] { | ||
| .mat-step-icon, .mat-step-label { |
There was a problem hiding this comment.
Wouldn't this make the icons harder to see? We could potentially use text-decoration: underline to distinguish the selected step.
There was a problem hiding this comment.
Yeah the icons are harder to see, although adding the text-decoration: undeline looks good to me so I changed it.
| @@ -15,6 +16,25 @@ | |||
| $warn: map.get($config, warn); | |||
|
|
|||
| .mat-step-header { | |||
There was a problem hiding this comment.
The HC styles should be in the _stepper.scss, not in the theme.
There was a problem hiding this comment.
I can't find that file
There was a problem hiding this comment.
Ah sorry, I was referring to this file: https://github.com/angular/components/blob/master/src/material/stepper/stepper.scss
| .mat-stepper-vertical-line::before { | ||
| border-left-color: theming.get-color-from-palette($foreground, divider); | ||
| @include a11y.high-contrast(active, off) { |
There was a problem hiding this comment.
Why do we need to remove this border?
There was a problem hiding this comment.
I removed it at first because I thought there was too many lines with the outlines over the buttons in the step header. Although looking at it again it looks fine so I added it back in.
|
+1 to all of Kristiyan's comments |
amysorto
force-pushed
the
high-contrast-fixes-for-stepper
branch
from
4516ff1 to
9e37957
Compare
|
@amysorto can you also post a screenshot of the HC appearance? |
|
Here are the screenshots in high contrast mode: Horizontal Stepper Vertical Stepper |
|
@amysorto looks like the only change left to make is moving the styles to |
amysorto
force-pushed
the
high-contrast-fixes-for-stepper
branch
from
9e37957 to
6120cfd
Compare
|
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. |
No description provided.