department-of-veterans-affairs / vets-design-system-documentation

Repository for design.va.gov website
https://design.va.gov
36 stars 55 forks source link

Formation Deprecation - Step 2 - Transition global color variables to USWDS3 colors #2228

Closed micahchiang closed 6 months ago

micahchiang commented 7 months ago

Description

One group of global styles that need to be migrated from Formation to CSS Library is our color palette. The first part of color migration proposed in the migration strategy RFC is to update the hex values for color variables directly to USWDS hex values, in Formation itself. To kick things off, we'll start with updating all of our primary blues.

Considerations

Tasks

Acceptance Criteria

caw310 commented 7 months ago

Hey team! Please add your planning poker estimate with Zenhub @Andrew565 @ataker @harshil1793 @it-harrison @jamigibbs @micahchiang @nickjg231 @powellkerry @rmessina1010 @rsmithadhoc

powellkerry commented 7 months ago

@humancompanion-usds Here are the mappings for the new USWDS colors. Can you verify they are correct? I could not find a value in AirTable for color-primary-alt-dark there was one for color-primary-alt-darkest is this the value that should be used?

//================
//  USWDS Colors
//===============
$color-aqua:                 #00bde3;
$color-aqua-dark:            #07648d;
$color-aqua-lightest:        #e1f3f8;
$color-blue:                 #005ea2;
$color-blue-darkest:         #162e51;

$color-primary:              $color-blue;
$color-primary-darker:       #1a4480;
$color-primary-darkest:      $color-blue-darkest;

$color-primary-alt:          $color-aqua;
$color-primary-alt-light:    #97d4ea;
$color-primary-alt-lightest: $color-aqua-lightest;
$color-primary-alt-dark:     $color-aqua-dark;
jamigibbs commented 7 months ago

How significant of a difference will this color change be? I ask because I'm wondering if we might want to make an announcement before this is merged into vets-website (in the #vfs-change-announcements Slack channel maybe?). That might save us from getting issues/support pings from teams maybe thinking something is wrong.

humancompanion-usds commented 7 months ago

Jami is correct. We absolutely need to warn folks before we do this. I've only let OCTO know. @caw310 and I have not let VFS teams know this is happening yet.

I had thought our plan was to do this only in our playground or in a specific application first so that we could review the result. If that's not the case then we definitely want to make sure we get eyes on the change after it is made in case we need to rollback for any reason. But I'd much prefer that I have a chance to review the changes in some form before they are auto-deployed (even if I need to do that in a local build).

humancompanion-usds commented 7 months ago

@powellkerry - Good catch on primary-alt-dark being missing. I did skip over that somehow. The value should be #00a6d2. I've added it to AT. I was also missing aqua-dark. That should be #009ec1.

I don't love how we're not using tokens here. The variable names are confusing and pointing in the wrong direction. For example:

$color-primary-alt: $color-aqua;

This should really be the other way around because aqua doesn't exist going forward and has no semantic meaning. Same goes for $color-blue. I had put notes to this effect in the AT. If we can swap those that would be great.

micahchiang commented 7 months ago

@humancompanion-usds - We can review this in a local build, maybe even a review instance.

The fact that this work is initially being done in Formation has a couple of implications:

  1. It will be a global change, there isn't a way around that. Though like I mentioned, we can still review locally before merging any sort of PR and decide if we want to reduce the number of colors we are changing, or even change the range of colors we are changing.
  2. Formation doesn't use tokens which is why we've chosen to move forward with using hex values directly. While CSS Library will absolutely be using tokens, I think what matters in Formation the most is getting the colors right since it'll be sunsetted anyway.
powellkerry commented 7 months ago

Here is a screenshot of the changes being made:

Screenshot 2023-11-02 at 2 38 51 PM

I will also try and get a screenshot of what the va.gov homepage will look like, though all of the color changes seem very small, so the untrained eye may not be able to tell a difference.

humancompanion-usds commented 7 months ago

@powellkerry - Those values all look correct to me.

One variable I'm not seeing that we should change however is vads-color-link ($color-link-default). Currently, in v1 we have a different primary blue and link blue. But in v3 they are the same (i.e. $color-link-default should point to $color-primary). We should also update the visited and active link colors as well. Our hover state is a bit odd and uses our base color and a gray background thus that doesn't change.

$color-link-default: $color-primary (#005ea2) $color-link-visited: #54278f $color-link-active: #0b4778

powellkerry commented 7 months ago

@humancompanion-usds it looks like formation has $color-visited, I am assuming the value of that should be #54278f . I don't see a variable for $color-link-active in formation. The only other variable related to links is $color-focus, should that variable be update to the value you have for $color-link-active?

All the naming discrepancies should be fixed when formation is deprecated and the component-library becomes the source of truth. 🤞

humancompanion-usds commented 7 months ago

Yes, $color-visited and $color-link-visited are the same and should be #54278f. Regardless if there is a variable the active color for links needs to be set correctly to #0b4778. $color-focus is in the tokens as vads-color-action-focus-on-light, it's value is #face00.

powellkerry commented 7 months ago

Updated color diff:

localhost_8080_color-diff (4)

caw310 commented 7 months ago

Work for this is done. Will merge on Dec 4 when all the global blues on VA.gov will be USWDS v3 blue.

humancompanion-usds commented 6 months ago

Okay, I found an issue that I somehow failed to note until I saw it in action on the home page. There was a naming change from v1 to v3 that we should also make, at some point.

Essentially, the values just shift up a level and it's just a name change. I updated AT to reflect this but we can make the name change at a later date. I just wanted to note it here.