-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(aria/toolbar): show outline on selected items when forced colors active in examples #32708
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
Conversation
68d5747 to
4494a25
Compare
|
Deployed dev-app for c0a5bfb to: https://ng-dev-previews-comp--pr-angular-components-32708-dev-juvesjka.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
| .example-button[aria-pressed='true'], | ||
| .example-button[aria-checked='true'], | ||
| .example-option[aria-selected='true'] { | ||
| outline: solid 1px; |
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.
We probably should use a system color to show it's selected: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/system-color. - SelectedItem?
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.
I tried that first, but there is an issue with the way we use icons here (and other places not SVG) where with active forced-colors the icon has a black opaque background.
Given this is just an example, I didn't want to change more and outline seemed good enough for now. We could also tweak the styling to be closer to material buttons but again, I don't think worth it in this case.
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.
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.
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.
I have never seen HCM use Highlight Color for focus -
For example: https://benmyers.dev/blog/whcm-outlines/ & https://sarahmhigley.com/writing/whcm-quick-tips/. Even for Google Material Wiz/ACX.
It's hard to find documentation on what is recommended. Personally, I prefer using a system color for select or 2px looks more clear in my opinion. But I will leave it up to you.
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.
I can go with the 2px, that seems okay. It's just an example.
We already use the highlight color for hover and focus in these examples.
I couldn't find a system color that shows up as anything other than black/white in the HCM on chrome (other than the highlight).
Given that this is just our examples, I don't think we should spent too much time on revisiting the simple theme we used here, since we're not shipping the styling portions.
c8083de to
c0a5bfb
Compare


Add additional styling when forced colors is active to show an extra outline in the toolbar examples.