alphagov / govuk_frontend_toolkit

❗️GOV.UK Frontend Toolkit is deprecated, and will only receive major bug fixes and security patches.
MIT License
403 stars 107 forks source link

Department colours are not always accessible #316

Closed edwardhorsford closed 7 years ago

edwardhorsford commented 8 years ago

Several department colours do not have sufficient contrast, and are being used for text.

Example: Defra org page

Departments with issues:

Note I think there's several issues:

Update: The issue with web-safe is that it has a specific meaning. It's also been used to refer to a hex version of another named colour. Such as when an org has a Pantone colour as their brand colour, and a 'web version' for internet use.

For our use, all the hex colours can be used on the web, but only some will have suitable contrast for text.

We must meet WCAG 2.0 AA, which requires a contrast against the background of 4.5:1 or greater.

NickColley commented 8 years ago

Hey @edwardhorsford

Thanks for raising I've just added a bug card to the Core Format team's planning board over at https://trello.com/c/4QPPRvuy/680-make-sure-all-organisation-colours-are-accessible

Thanks for raising 👍

edwardhorsford commented 7 years ago

Hi @nickcolley has there been any progress on this?

robinwhittleton commented 7 years ago

Doesn’t look like it:

Shilpa Acharya moved this card from Proposed stories from the Core team to Support icebox – Aug 24 at 2:44 PM

edwardhorsford commented 7 years ago

@nickcolley @robinwhittleton can you suggest who I can talk to to try to get this prioritised? Is it Shilpa Acharya?

It's been 3 months since I raised this issue - I'd like to get this accessibility issue addressed.

robinwhittleton commented 7 years ago

@tvararu @fofr any idea who could prioritise this?

fofr commented 7 years ago

My guess is that it was marked as low priority. Shilpa is the person to talk to.

fofr commented 7 years ago

What should the DEFRA and HMRC text colours be?

fofr commented 7 years ago

Regarding #249, from the ticket it looks like the colour was approved for text but wasn't re-tested:

Mark H also looked at this later, and proposed the colour for text to ensure accessibility. This has been confirmed by David Pearson of DEFRA that we are ok to proceed on the revised colour.

I vaguely recall it was judged against the large text 3:1 ratio. Large text being 18px or over. Though clearly there's text on the page that's in that colour and at 14px.

edwardhorsford commented 7 years ago

Point taken about large text. I'll @aduggin weigh in for that, but in practice we have been aiming for 4.5:1 or greater for all text.

It's hard to control how these colours are used, so IMHO it's easier to aim for the higher contrast and then know it's 'safe' for all type sizes.

Since the colour is used below 18px (and moreso on mobile), this is somewhat a moot point - this still needs to be addressed.

fofr commented 7 years ago

@edwardhorsford I agree. It's good to know how we got it wrong so we don't make the same mistakes again.

edwardhorsford commented 7 years ago

Talking with @accessiblewebuk, large text is actually 18.66px (14pt) for bold and 24px (18pt) for non bold.

fofr commented 7 years ago

We've prioritised this now, but aren't sure what the new colours should be.

edwardhorsford commented 7 years ago

I've had a go darkening the colours so they meet our 4.5:1 minimum contrast requirement. HMRC is a very minor change, whilst DEFRA is more significant.

Guessing these should go as 'websafe' versions whilst leaving the real deparment colour alone.

HMRC: #008670 (contrast 4.52:1 from 4.47:1) DEFRA: #008938 (contrast 4.53:1 from 3.37:1)

Examples:

HMRC before:

screen shot 2017-01-04 at 15 26 17

HMRC after:

screen shot 2017-01-04 at 15 26 40

Defra before:

screen shot 2017-01-04 at 15 24 44

Defra after:

screen shot 2017-01-04 at 15 24 58

edwardhorsford commented 7 years ago

NB: I've not audited every department colour. There may be additional departments with issues that haven't been spotted.

NickColley commented 7 years ago

Just going to reopen this since there's potentially more departments that Ed didn't notice, so we should close this once we're confident they're all accessible.

fofr commented 7 years ago

I'd like to include some automated tests to check each text colour value in this Sass list: https://github.com/alphagov/govuk_frontend_toolkit/blob/master/stylesheets/colours/_organisation.scss#L75

Here's a pure Sass way of calculating colour contrast: https://gist.github.com/voxpelli/6304812

It has a method of warning if contrast is below a certain ratio: https://gist.github.com/voxpelli/6304812#file-_contrast-scss-L48

robinwhittleton commented 7 years ago

That Sass method is supported by @voxpelli at https://github.com/voxpelli/sass-color-helpers

accessiblewebuk commented 7 years ago

@fofr that works if you are sure they are only being used against white (or you have a list of colours they will be used against. Then it need to be made clear in any statement what combinations have been tested.

NickColley commented 7 years ago

@accessiblewebuk definitely worth making a note in the implementation that we're testing against the background colour due to how we're currently using the department colours (which as far as I know is always against our background color)

edwardhorsford commented 7 years ago

Since we support $panel-colour there's an argument to checking all our colours against that - in effect requiring a slightly higher contrast than 4.5:1 - roughly 6:1 from a quick test.

fofr commented 7 years ago

This is where I got to with a proof of concept on raising Sass warnings. It doesn't currently work because it's not loading in the Sass files from the colour helpers correctly, but it should do what we want:

https://github.com/alphagov/govuk_frontend_toolkit/compare/sass-warn

voxpelli commented 7 years ago

@fofr You need to add a new loadPathin the grunt-contrib-sass settings: https://github.com/alphagov/govuk_frontend_toolkit/blob/ef06147803755152f8796c57e4493cd5c1f933a6/Gruntfile.js#L49

Adding eg. ./node_modules should make the sass-color-helpers file load if you remove the node_modules prefix from their paths.

Also: I just released a new patch version that fixes https://github.com/voxpelli/sass-color-helpers/issues/5, so a good idea to upgrade to version 2.0.3.

fofr commented 7 years ago

PR to protect against poor contrast in the future: https://github.com/alphagov/govuk_frontend_toolkit/pull/374