So as I read it the icons are organized by color. So the red #ea2800 isn't dark enough here:
core/misc/icons/ea2800/error.svg
On white the contrast ratio is only 4.37318, not 4.5
http://contrast-finder.tanaguru.com/result.html?foreground=%23EA2800&bac...
The red we've seemed to use in CSS is #e00 #e32700
Can we just use that for errors?
In which case we need a new error.svg icon & a core/misc/icons/e32700/ directory.
| Issue category | Task because coding standards |
|---|---|
| Issue priority | Not critical because coding standards |
| Unfrozen changes | Unfrozen because it only changes markup, images and CSS |
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | Screenshot 2015-09-02 12.51.33.jpg | 405.49 KB | lewisnyman |
| #18 | error.svg-color-contrast-18.patch | 7.27 KB | mgifford |
Comments
Comment #1
mgiffordSeven's color of red works too #e62600. Just not #ea2800.
Most folks would never even see the difference, but there is one.
Comment #2
mgiffordI think this will do the trick.
Comment #4
mgiffordLet's try that again.
Comment #6
jessebeach commentedLGTM. Ship it.
Comment #7
mgiffordLooks like this is the only reference to the hex ea2800 now. Realized in #2503453: Increase contrast on inline form error text that form-error-message is no longer in Core and that was the other css using it.
Comment #8
lewisnymanComment #10
lewisnyman@mgifford Should we change the color here so it matches the color in #2503453: Increase contrast on inline form error text?
Comment #11
mgiffordYes.. Thanks for pointing out that inconsistency @LewisNyman. Moving to #e32700.
Comment #12
lewisnymanNice! High five. See the color justification in #2503453: Increase contrast on inline form error text
Comment #13
mgiffordThanks @LewisNyman!
Comment #14
alexpottNeed to update all the css that refers to ea2800/error.svg then - right?
Comment #15
mgiffordArrg.. Sorry Alex..
Comment #16
lewisnymanGood catch Alex. Sorry I missed that. I applied the patch and grepped for ea2800 and found nothing. I also manually test just to make sure.
I don't want to postpone this issue but this will cause #2395853: Split system.module.css and system.theme.css files into SMACSS style components to need a reroll, which is a really tricky patch. This will be easy to reimplement after it goes in.
Comment #17
davidhernandezThe split issue is in.
Comment #18
mgiffordOk, here's a re-roll with the new system.admin.css file.
Comment #19
lewisnymanThanks for this. I manually tested the pages that include the CSS changes just to make sure we had the paths correct.
Comment #20
alexpottAccessibility is a priority during beta. Committed 5d896a2 and pushed to 8.0.x. Thanks!