-
Notifications
You must be signed in to change notification settings - Fork 355
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
Ratings Slider: Use buttontext instead of linktext system color in high contrast mode #3025
Conversation
aria-practices
Outdated
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.
What is this? Pls remove
seems like there is accidental addition of an empty file named aria-practices |
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Topic: Change high contrast system color in ratings slider<jugglinmike> github: https://github.com//pull/3025 <jugglinmike> jongund: I changed the system color link text to "button text". It just makes our widgets look more like form controls than links <jugglinmike> s/button text/buttontext/ <jugglinmike> Matt_King: why are two files changed? <jugglinmike> jongund: cspell checks CSS files for spelling <jugglinmike> Matt_King: That seems undesirable <jugglinmike> s/cspell/I also had to change the cspell configuration. cspell/ <jugglinmike> howard-e: I can look into that <jugglinmike> Matt_King: Does this color change impact all contexts? Or only high-contrast mode? <jugglinmike> jongund: Only high-contrast mode <jugglinmike> jongund: Depending on how you test it, you don't necessarily need Windows <jugglinmike> jongund: In Chrome, for instance, you can use the developer tools to simulate high-contrast mode <jugglinmike> jongund: This probably won't affect macOS users <jugglinmike> jongund: The only place I know this pull request will make a difference is in Windows <jugglinmike> Matt_King: So the pull request should have screenshots of the difference (before and after), and also instructions for how a reviewer can observe the difference themself <jugglinmike> Jem: I can review once jongund adds that information <jugglinmike> Matt_King: Okay, I'll assign you, then <jugglinmike> Matt_King: is this a change you were just trying out for the one slider? Do you think it should be applied to the other examples if people like it? <jugglinmike> jongund: Yes, I think we should use it elsewhere. Though we only use the "forced colors" media query in one other pattern--feed. I'd also like to add it to other patterns |
@jongund I will be waiting for the compared views of forced colors for the review as we discussed at the APG meeting. |
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.
Thanks for all the screenshots. These met color contrast requirements and look good.
I think the use of
buttontext
system color is a better choice thanlinktext
for styling the slider when in Windows 10/11 high contrast modes. This change makes no difference on high contrast on macOS.Preview of updated rating slider
WAI Preview Link (Last built on Tue, 04 Jun 2024 18:26:17 GMT).