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

FCL-152 | add colour variable naming convention ADR #181

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

jlhdxw
Copy link
Collaborator

@jlhdxw jlhdxw commented Nov 7, 2024

Proposed naming for CSS colour variables across all the various apps for FCL.

Copy link

@Terry-Price Terry-Price left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to introduce tint of the same colour anywhere. So 50 = 50%. if that's what these numbers imply.

I know this isn't a standard solution (out the box). but what about:

grey
grey-lighter
grey-darker

This would limit ourselves to three shades and stop the bloat that's happening recently.

I'd be keen for this and if we are using more than three different greys then that's just silly.

Blues are more tricky e.g. notifications are brighter blue. so could be:
bright-blue
bright-blue-lighter

blue
blue-ligher
blue-darker

@jlhdxw
Copy link
Collaborator Author

jlhdxw commented Nov 7, 2024

@Terry-Price

The proposal is that we would effectively have that, with the colours being defined like this:

$color-grey-200 - this being "$color-grey-lighter" in your example
$color-grey-500 - this being "$color-grey" in your example
$color-grey-800 - this being "$color-grey-darker" in your example

The reason for not having "darker"/"lighter" is what happens if we do want 4 grey colours? Which is very common. We have to have "light" or "mid" or "lighter" or "lightest" etc. This also means that you need context of the original colour to know what this is in reference to.

The difference with having a numerical scale is we know where that is on the scale without having context of what it could be "lighter" than.

Copy link
Collaborator

@jacksonj04 jacksonj04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Is it worth mentioning a second layer of things like $color-body-text, which is set to one of the actual colours, and then all the different texts which need a colour setting can lean on that?

I have no real feelings either way but I've seen the pattern elsewhere.

@dragon-dxw
Copy link
Collaborator

Out of interest, what does the number mean? (is it a luminosity?)

Additionally I'd like to have a place to go for "here are the colour sets we've used to communicate this sort of vibe" (e.g. warnings, alerts...)

Probably in the style guide? Possibly SCSS comments?

Don't feel I do enough front end to have a strong opinion in general, so happy for the strong steer.

@Terry-Price
Copy link

James,
Can you have a conversation with AJ who is our lead FE at TNA. he has the colour variables set up and he would likely want us to follow the same naming convention. We are trying to align where practical so that we have the same standards.

Copy link
Member

@Donna-H Donna-H left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlhdxw fine with this update to naming convention maybe just as @Terry-Price as mentioned check with @ahosgood if it is supposed to align with rest of TNA colour naming conventions

@ahosgood
Copy link
Member

ahosgood commented Nov 8, 2024

The set of web colours we have worked on standardising for TNA Frontend can be seen here: https://nationalarchives.github.io/design-system/styles/colours/#brand-colours

There are CSS variables defined in TNA Frontend for all of these with schemes for light and dark, high and normal contrast themes: https://github.com/nationalarchives/tna-frontend/blob/main/src/nationalarchives/variables/_colour.scss (you can ignore anything other than the light theme for the purposes of this discussion)

Personally, I would like for us to just keep to this established colour palette. There is enough variation in the brand colours (main, dark and light) to keep things visually interesting without adding more tints to manage and there has been a LOT of work gone into checking the colour combinations of all possible scenarios (https://nationalarchives.github.io/tna-frontend/?path=/story/utilities-colours-combinations--light) and testing them against the WCAG contrast criteria.

If there are arguments for introducing more tints or text colours, I would like to hear them but I feel very strongly that we should all be moving towards using a single standardised colour palette. This doesn't mean we can't tweak and add to what we already have but I don't think we should essentially be starting anew.

@Terry-Price
Copy link

Thanks @ahosgood
I agreed we should align the structure and syntax for colour variables.
Less certain that we can match our FCL colours with the TNA palette in all circumstances; as you say there may be some flexibility required on both parties. After all we are one party really and so this is a great opportunity.

Firstly let me speak with Nicki about this, but I'd be up for taking a look at what colours we can adopt from the TNA palette and then work on what extra colours we may need to add, e.g. our gold yellow colour has been the FCL personality colour since day one and to just drop that without UXR might be too far.

@ahosgood
Copy link
Member

ahosgood commented Nov 8, 2024

That's fine. You could always create your own accent (https://nationalarchives.github.io/design-system/styles/colours/#accent-colours) for FCL (tna-template--gold-accent) and use a similar palette to the yellow variant.

@jlhdxw
Copy link
Collaborator Author

jlhdxw commented Nov 8, 2024

@ahosgood thanks for the comments. I would be very happy if we could just use TNA standard colours and not have to even have this concern of how to define our own colour variable naming convention. The reason this discussion came about in the first place is because the colour scheme we have currently is not very consistent (we have 9 different shades of grey!) and the names don't make much sense. If TNA colours had been used in the first place we likely wouldn't have such an issue of deviating colours and different shades.

The TNA standard colour scheme does look quite different to what we have in the FCL colour scheme, so converting over would be quite a significant change. With that said, it would be a better user experience if the colour palettes across TNA sites were more consistent.

@Terry-Price once you've spoken with Nikki it would be a good exercise to go through our colour palette and see what the site would look like with the standard TNA colours.

@dragon-dxw
Copy link
Collaborator

dragon-dxw commented Nov 8, 2024

@jlhdxw Can you run pre-commit install and pre-commit run --all so it'll lint the doc nicely?

@jlhdxw
Copy link
Collaborator Author

jlhdxw commented Nov 8, 2024

@dragon-dxw thanks for the prompt! my first contribution to this repo so forgot to instal the pre-commit hook :) all done now

@jacksonj04
Copy link
Collaborator

This doesn't mean we can't tweak and add to what we already have but I don't think we should essentially be starting anew.

@ahosgood we're definitely not planning on starting anew, especially since (as you say) the legwork around things like contrast has already happened; this is primarily a consolidation exercise so we're in a better place to just move towards the TNA palette (where it makes sense) in future.

@ahosgood
Copy link
Member

It sounds like some consolidation work needs to be done with colours so I am expecting decisions to be made where we drop some of the many shades of grey we have and replace them with others. In that sense, this IS making new colour decisions and the contrast ratios will need to be revisited and re-tested.

My concern is over using tints like Material UI, Bootstrap et al because they have been designed without a colour scheme in mind; the user is free to pick and choose from the provided palette. The National Archives does now have a very specific palette of both greyscale colours for text and brand colours for accent. There is no option to introduce new tints.

As I am not wanting to hold up this PR and we don't currently have any specifications for CSS variable names, I think we need to revisit this conversation further down the line.

I would advise against having shades and tints and instead just naming colour variables to describe their colour rather than purpose, something like:

// Bad
$body-text: #343338;
body { color: $body-text; }

// Good
$charcol-grey: #343338;
body { color: $charcol-grey; }

@jlhdxw jlhdxw merged commit 4c88564 into main Dec 13, 2024
3 checks passed
@jlhdxw jlhdxw deleted the FCL-152/color-naming-convention-adr branch December 13, 2024 14:08
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

Successfully merging this pull request may close these issues.

6 participants