-
Notifications
You must be signed in to change notification settings - Fork 88
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
enh(NcActions): Readability #4049
Conversation
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.
Maxcontrast is for borders ok.
Could you please check if background color for actions items is enough with this tool (or some similar tool): https://chrome.google.com/webstore/detail/wcag-color-contrast-check/plnahcmalebffmaghcpcmpaciebdhgdf ? I guess we have to put there higher contrast ratio.
I set the milestone to 8 since this goes against master. |
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.
Looks good for the text.
But for the input borders, didn't we introduce a specific lighter color than color-text-maxcontrast there since graphical elements don't need the same contrast amount as text? It is very dark now.
@jancborchardt We did, and the minimum color that meets the requirements is also quite dark That would result in a lighter looking border |
@jancborchardt what do you think about reducing the border thickness to 1px? screenshots in original comment show how it would look :) |
Regarding 2px to 1px border, I'd say no, several reasons:
(Apart from that, we can't switch it so close before release anyhow.) |
Signed-off-by: Christopher Ng <[email protected]>
/backport to stable7 |
Contrast
#222222
on#ffffff
is 15.9:1#222222
on#f5f5f5
is 14.59:1#949494
on#ffffff
is 3.03:1#006aa3
on#f5f5f5
is 5.36:1