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.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because coding standards
Issue priority Not critical because coding standards
Unfrozen changes Unfrozen because it only changes markup, images and CSS
CommentFileSizeAuthor
#19 Screenshot 2015-09-02 12.51.33.jpg405.49 KBLewisNyman
#18 error.svg-color-contrast-18.patch7.27 KBmgifford
#15 error.svg-color-contrast-15.patch6.97 KBmgifford
#11 error.svg-color-contrast-11.patch1.73 KBmgifford
#4 error.svg-color-contrast-4.patch1.73 KBmgifford
#2 error.svg-color-contrast.patch1.61 KBmgifford
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Seven's color of red works too #e62600. Just not #ea2800.

Most folks would never even see the difference, but there is one.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Active » Needs review
FileSize
error.svg-color-contrast.patch1.61 KB

I think this will do the trick.

Status: Needs review » Needs work

The last submitted patch, 2: error.svg-color-contrast.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Needs review
FileSize
error.svg-color-contrast-4.patch1.73 KB

Let's try that again.

mgifford queued 4: error.svg-color-contrast-4.patch for re-testing.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented

LGTM. Ship it.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented

Looks 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.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Issue tags: +frontend, +styleguide

mgifford queued 4: error.svg-color-contrast-4.patch for re-testing.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented

@mgifford Should we change the color here so it matches the color in #2503453: Increase contrast on inline form error text?

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Issue summary: View changes
FileSize
error.svg-color-contrast-11.patch1.73 KB

Yes.. Thanks for pointing out that inconsistency @LewisNyman. Moving to #e32700.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Reviewed & tested by the community

Nice! High five. See the color justification in #2503453: Increase contrast on inline form error text

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented

Thanks @LewisNyman!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work 
diff --git a/core/misc/icons/ea2800/error.svg b/core/misc/icons/ea2800/error.svg
...
diff --git a/core/misc/icons/e32700/error.svg b/core/misc/icons/e32700/error.svg

Need to update all the css that refers to ea2800/error.svg then - right?

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Status: Needs work » Needs review
FileSize
error.svg-color-contrast-15.patch6.97 KB

Arrg.. Sorry Alex..

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Postponed
Related issues: +#2395853: Split system.module.css and system.theme.css files into SMACSS style components

Good 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.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez as a volunteer commented
Status: Postponed » Needs review

The split issue is in.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
FileSize
error.svg-color-contrast-18.patch7.27 KB

Ok, here's a re-roll with the new system.admin.css file.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
Screenshot 2015-09-02 12.51.33.jpg405.49 KB

Thanks for this. I manually tested the pages that include the CSS changes just to make sure we had the paths correct.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Accessibility is a priority during beta. Committed 5d896a2 and pushed to 8.0.x. Thanks!

  • alexpott committed 5d896a2 on 8.0.x 
    Issue #2334067 by mgifford, LewisNyman: error.svg doesn't have...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.