Skip to content
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

Should all colours be mapped to a variable name by default #920

Closed
Nooshu opened this issue Jul 19, 2018 · 7 comments
Closed

Should all colours be mapped to a variable name by default #920

Nooshu opened this issue Jul 19, 2018 · 7 comments

Comments

@Nooshu
Copy link

Nooshu commented Jul 19, 2018

After using the colour palette I'm wondering if all the colours should be mapped to a variable name? I was having to jump back and forth between code and documentation to see if a particular colour was already a predefined variable or a reference in the govuk-colour map. e.g.

$govuk-colour-black;
$govuk-colour-grey-1;
$govuk-colour-grey-2;
$govuk-colour-grey-3;
$govuk-colour-grey-4;
$govuk-colour-white;
@36degrees
Copy link
Contributor

For context, the colours used to be defined as variables, but this was changed to a map in #777.

The intention behind this change was to avoid hard-coding our palette into the public API, because:

  • it makes it possible to theme GOV.UK Frontend to be used in other contexts that don't use our colour palette – otherwise, if there is a variable called $govuk-green this 'hard-codes' the fact that there must be a 'green' (and only one green) in the palette. Using a map to represent the palette allows for an an unspecified number of colours with whatever names are appropriate.
  • it gives us more control over e.g. deprecating colours, because we could make e.g. calling govuk-colour('grey-4') throw a deprecation warning.

It also:

  • makes it easier to extend the colour function in the future to be able to return tints or shades.
  • provides a single map that we can iterate over to e.g. test colour contrast combinations

One suggested alternative is that we could move the 'applied' colour palette (e.g. govuk-button-colour) to also exist within the map, but this means that:

  • we have to define the colours for every component in one central place, away from the code they belong to (i.e. $govuk-button-colour should be defined in the button component - the only exceptions are colours that are used across multiple components or in our core styles, like $govuk-text-colour))
  • if someone theming Frontend wants to change the button colour they have to re-define the entire map to do so

I appreciate that it seemingly makes things slightly harder for the user, because there are two different ways to get a colour depending on whether you're referencing the palette or an 'application' of a colour.

One final thing to note is that I think that whilst we're not there yet, we want to be moving towards consistently abstracting component colours, so every component would have e.g.

$govuk-tag-background-colour: govuk-colour("blue") !default;
$govuk-tag-text-colour: govuk-colour("white") !default;

.govuk-tag {
  color: $govuk-tag-text-colour;
  background: $govuk-tag-background-colour;
}

There then would be consistency (of sorts) because you would never (or at least rarely) be referencing the palette from the CSS itself.

@joelanman
Copy link
Contributor

I follow the reasons, but this feels like exposing technical limitations to the user. As a user it is significantly harder to know about and remember two different approaches to using colours, and know about and remember which to use when.

@joelanman
Copy link
Contributor

joelanman commented Nov 14, 2018

Just to note we had a support request where part of the problem was using $:

$govuk-colours("white")

when this is correct:

govuk-colour("white")

@edwardhorsford
Copy link
Contributor

I just came across this in the design system. It has two ways of doing colours:
screenshot 2019-01-18 at 13 05 07

I'm... not sure what to do. Does one method not support all colours? why are some one way and some the other? is one more correct?

@36degrees
Copy link
Contributor

The colour palette (blue, red, green…) is defined using a map and accessed using a function.

Applications of colour (error colour, focus colour) are defined as variables.

The reason for the different treatments is covered in #920 (comment)

govuk-colour("blue") being listed under 'status' does stand out as a bit of an outlier – I wonder if we should either remove it or replace it with $govuk-brand-colour. That would be slightly clearer – the colours at the top would then all be applications, with the full palette listed out underneath.

@aliuk2012
Copy link
Contributor

I've added the Triage label to this as I think this issue might be something we can address or close as part of the colour palette and WCAG changes.

@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label May 15, 2019
@36degrees
Copy link
Contributor

We're going to close this as we think this is the right approach, for the reasons outlined in the initial reply.

We have updated the Design System to be more consistent about how we talk about the colour palette and the various applications of colour (e.g. $govuk-text-colour).

If we do continue to see confusion around the different ways of accessing colours we could consider ways that we could improve the documentation to make it clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants