-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Update shortcut opacity #10584
Update shortcut opacity #10584
Conversation
Opacity at .84, the lowest it can be while still passing WCAG AA standards
LGTM! Nice tweak. 👍 |
By looking at the screenshot, I'd argue that while this fixes the accessibility issue for people with sight issues, it creates usability issues for regular people as it's very hard to distinguish the shortcuts from the menu captions. And since the shortcuts are visible more clearly in the "Shortcuts modal", I'd be more in favor of leaving the current color. |
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 agree with @youknowriad; this creates issues for users without vision issues–and we do have keyboard hints elsewhere that are much more accessible. If we could rely on symbols everywhere like MacOS does we could do this, as the keyboard shortcuts would be quite small and far away enough from the menu item to not confuse things. As-is this is unreadable to me.
Could a compromise be to increase the opacity but lower the font size? How would that look? 🤷♂️
IMHO, it would be better either to increase contrast on menu captions (to better distinguish them from shortcuts) since they're "only" 6.47:1, or to increase their Don't know how to play with this, but FWIW the issue with shortcuts and captions looking almost the same isn't that shortcuts are too legible, is it? Make every thing more readable, please :) |
See #10576 (comment) |
Noting that @afercia's suggestions are to widen the settings area and make the keyboard shortcut the same color of the item labels and that a decision is needed for this issue to go forward. |
I copied #10576 (comment) to make it easier to follow this PR:
@alexroseb are you fine addressing this feedback? |
I think widening this to As for the color of the shortcut labels, is there a reason we use opacity here instead of just swapping out with one of our actual gray values (like |
Worth noting that in #13732 I'm changing the shortcut opacity to be fully opaque when hovered. I also updated the normal opacity to match this PR, so the rebase should be easier, which ever one gets approved first. |
The specific changes proposed by this pull request were encompassed separately as part of #13732 . There seems to be a separate specific recommendation about the width of the menu, though I think it would be fair to acknowledge as a separate task tracked by #10576 (also in mind of lack of progress here). For that reason, I'd suggest it be closed, and a separate pull request created for remaining items. |
Description
Increased opacity at .84, the lowest it can be while still passing WCAG AA standards. Fixes #10576.
How has this been tested?
Doesn't affect anything except this small section because it's very specific CSS.
Screenshots
Original:
Updated:
Types of changes
Very small CSS change
Checklist: