Implement new Gray scale on Claro

Colors are updated in the Drupal Design System (including Colors Library & Claro in Figma): https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Drupal-Design-system...

Kindly review the attached patch.

Thanks!

Sorry, I missed one variable to update. Kindly review the new updated patch.

Thanks!

1. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -4,25 +4,29 @@
   -  --color-lightgray: #d4d4d8;
   +  --color-lightgray: #d3d4d9;
   ...
   -  --color-lightgray-active: #adaeb3; /* 10% darker than base. */
   +  --color-lightgray-active: var(--color-gravity); /* 10% darker than base. */
   

   Regarding the code comment /* 10% darker than base. */It's not clear from this patch that the --color-gravity is in fact 10% darker than base. Base I presume in this case is --color-lightgray (?), in which case seashell is the next color down in the scale, that i'd presume would be 10% apart, but maybe not. ¯\_(ツ)_/¯ I think it makes sense to just remove this code comment (and any other similar ones that can be found related to the gray scales).

    

2. What would make this patch overall much nicer to maintain and review would be to use HSL colors to more easily confirm that the colors are all in the same Hue and only change in Lightness or Saturation to achieve the correct tone. For instance, a quick run through using a color picker shows that the bottom 4 colors have a hue of 231 while the lighter shades have a hue of 230. Very minor difference, most likely imperceptible to the eye but in the end, it is an annoyance that could be fixed and standardized.

   Given that the contributors on the design side have provided only RGB colors, maybe this is out of scope, but I feel link this is an integral part of building a proper grayscale using more precise intervals and colorspace that is native for the web.

    

3. Instead of naming color variables that are near to impossible to remember if you need to lighten or darken one, I'd suggest naming the css variables on a numeric scale.

   --color-gray-100
   --color-gray-200
   --color-gray-300
   --color-gray-400
   --color-gray-500
   --color-gray-600
   --color-gray-700
   --color-gray-800
   --color-gray-900
   --color-text
   

   You put the 100 level scale there to allow for future expansion if it were ever needed. This would vastly improve themers experience, maybe designers too.

   Of the 11 design systems I checked, 8 of them standardize around a decimal system for their neutral color variables.

   • https://material.io/design/color/the-color-system.html#color-usage-and-p...
   • https://developer.microsoft.com/en-us/fluentui#/styles/web/colors/neutrals
   • https://atlassian.design/guidelines/brand/color
   • https://style.helpscout.com/visual-elements/#color
   • https://www.carbondesignsystem.com/guidelines/color/overview/#themes
   • https://ux.mailchimp.com/patterns/color
   • https://getbootstrap.com/docs/4.0/getting-started/theming/#grays
   • https://www.lightningdesignsystem.com/design-tokens/#category-color

   The 3 design systems that I looked at that broke from this trend were:

   • https://brand.uber.com/guide#color-specialty-colors
   • https://polaris.shopify.com/design/colors
   • https://v1.designsystem.digital.gov/components/colors/

   It is clear that the reason these outliers do not use a decimal system is because they don't have a complete grayscale, but only offer a few select neutral tones. Because we're adding several more grays here, it makes sense to switch to a decimal system.

    

4. It probably makes sense to standardize on one single spelling of the word gray in the color variables. I see "lightgray" and "davysgrey" as two color names. This issue also makes a similar problem (the issue title says "gray" but the description uses "grey" in several places). Gray is the American spelling and therefore the recommended spelling because there is precedence in core standardizing on American spellings.

   I've filed a follow-up issue for this where the topic can be hashed out in the wider community, since the problem affects all of Drupal, not just this theme. #3155258: Use American English spelling of "gray"

    

Answers to @jwilson3 points:

1. As the comment is outdated it can be removed
2. Not in the scope of this ticket
3. Very good idea, I've created a follow-up ticket for this #3155531: Naming convention of color shades in the Drupal Design System (e.g. for Claro), not in the scope of this ticket though
4. You already created a follow-up ticket, thanks

Thank you @saschaeggi for your reply on #8 and As per your reply #11 patch is updated, Kindly review the attached patch.

Thanks!

@kiran.kadam911,

I have reviewed your patch and it looks good to me.

Screenshot for reference:

Thanks!

1. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -134,14 +138,14 @@
   -  --button--disabled-fg-color: var(--color-grayblue);
   +  --button--disabled-fg-color: var(--color-moiety);
      --button-fg-color--primary: var(--color-white);
      --button-bg-color--primary: var(--color-absolutezero);
      --button--hover-bg-color--primary: var(--color-absolutezero-hover);
      --button--active-bg-color--primary: var(--color-absolutezero-active);
      --button--focus-bg-color--primary: var(--button-bg-color--primary);
      --button--disabled-bg-color--primary: var(--color-lightgray);
   -  --button--disabled-fg-color--primary: var(--color-oldsilver);
   +  --button--disabled-fg-color--primary: var(--color-moiety);
   

   The disabled color for button will likely be different than the disabled color for an input. This is because inputs will be on a white background, but the button text is on top of the button's background color. This could either be decided in this issue, or the changes to button color could be removed and addressed in a followup.

2. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -91,7 +95,7 @@
   +  --input--disabled-fg-color: var(--color-moiety);
   

   Figma does say "disabled 4:5.1", for but the issue summary also says "Also it should be discussed which color we should finally use for the disabled elements." and "Decide which color to use for disabled".

   If the decision has been made, that should be made more evident in the issue summary as there's no discussion of it in the comments. If it hasn't, then that needs to happen.

I removed the Figma screenshot from the issue summary as it is outdated and can be confusing when checking multiple colors against the patch.

An update on the disabled state front. Cristina & I discussed this today and we decided on the following option:
We decided to use gray-blue for disabled text. We also streamlined it accross elements (see example with input and buttons):

I hope this unblocks this issue.

(Note that this change is currently not everywhere reflected in the design system yet, but it's on our task list to do so)

Thanks @saschaeggi for your reply to go head.

I updated the patch as per #17 with New disabled color(gray blue).

Kindly review the attached patch.

Thanks!

This needs a reroll, also noticed a few things, some may require design input. It's possible some are more suitable for followup issues so I'm open to that.

1. --input--disabled-bg-color: #f2f2f3; /* Light gray with 0.3 opacity on white bg. */ The color assigned to this variable is not any one of the new grays. I assume it's pretty close to one of them, and should equal that instead of #f2f2f3
3. Similar to the previous item, I noticed several other variables that are assigned some kind of gray color that isn't an exact match for any of the grays introduced here, although it's possible some may be if converted to hex.
   We should either change these to one of the grays added in this issue, or explicity state that it's OK for them to keep their value despite them adding to the number of grays presented by Claro:

     --color-divider: rgba(142, 146, 156, 0.5);
   
     --input--disabled-color: rgba(84, 85, 96, 0.6); /* Davy's grey with 0.6 opacity. */
   
     --input--disabled-border-color: #bababf; /* Old silver with 0.5 opacity on white bg. */
   
    --details-bg-color: rgba(243, 244, 249, 0.4);
    --details-border-color: rgba(216, 217, 224, 0.8);
     --jui-dropdown-border-color: rgba(216, 217, 224, 0.8); /* Light gray with 0.8 opacity. */
     --jui-dropdown-shadow-color: rgba(34, 35, 48, 0.1); /* Text color with 0.1 opacity. */
   
   

   I suspect some of the rgba ones can stay as-is, but if they can be converted to hex thats better as the color will be consistent regardless of its background

4. Was keeping --color-lightgray-o-80: rgba(212, 212, 218, 0.8); intentional or should it be replaced by one of the new grays?

5. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -4,25 +4,29 @@
   +  --color-grayblue: #919297;
   +  --color-oldsilver: #828388;
   +  --color-davysgrey: #55565b;
   

   The reroll should include making sure that any instances of "grey" are now "gray", as that change was made in a commit that occurred since the last patch was uploaded.

6. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -4,25 +4,29 @@
   +  --color-moiety: #75767b;
   +  --color-gravity: #adaeb3;
   +  --color-seashell: #c1c2c7;
   +  --color-lightdiamond: #dedfe4;
   

   May as well add these next to the other grays instead of at the end of the list. I considered also suggesting reordering by darkness, but that can happen in the variable renaming issue.

7. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -4,25 +4,29 @@
   -  --color-lightgray-active: #adaeb3; /* 10% darker than base. */
   +  --color-lightgray-active: var(--color-gravity);
   

   Is is actually specified anywhere that --color-gravity should be the value of --color-lightgray-active? It's possible I missed this, but this and the disabled fg/bg colors should be explicitly stated in the issue summary or figma (ideally both but as long as theres a single place to consult it should be fine).

Rerolled the patch and implemented 5 and 6.

1. The patch needs to include the compiled CSS. I notice that the patches have only been including variables.pcss.css, but the changes there will significantly impact the compiled CSS and those changes need to be part of the patch. yarn build:css in /core should do it.
2. Items 1,3,4,7 from #20 still need to be addressed. Will likely require design input. It's possible they'll just say "these grays can stay, they don't have to be part of the new grayscale", but that should be confirmed since the goal of this issue was to reduce the amount of different grays in use.

On the Claro weekly call @ckrina confirmed that items 1,3,4,7 from #20 don't need to be in the scope of this issue.

This has the missing compiled CSS from #21

Reroll

I think I'm missing something. The changes that I can see which would affect color contrast are just moving from: core/themes/claro/css/base/variables.pcss.css

--color-sunglow-shaded: #c;

to this:
--color-sunglow-shaded: #977405;

These are commented out, right? core/themes/claro/css/base/variables.css

/* Variations. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 10% darker than base. */ /* 20% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */

/* Variations. */ /* 10% darker than base. */ /* 20% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */

Most of the changes aren't where I would be expecting them from looking at @bnjmnm's code.

Sounds good to me for using only 025, 050, (potentially 075 in the future) and 100. Keeps things a little bit simpler.

@ckrina --- If I'm not mistaken you have a PR in for this via this commit right? https://git.drupalcode.org/project/drupal/-/merge_requests/427/diffs#dif...

I can help here if needed, but seems just needs merged?

I took a look to the color names on the merge request and seems right to me now, here's a screenshot with the latest values:

I can't get the current MR, or a new branch to be based on 9.3 (perhaps it's just a Gitlab blindspot on my part). This addresses the CI failures in the most recent MR + rerolls to 9.3. I didn't realize I couldn't get this to cooperate with the MR until it's too late, but here are the changes that can hopefully be part of an MR that is against 9.3!

The patch in comment #45 correctly changes the css variables names e values.

Great work here all! I really like end result of this issue!

Did some manual testing and confirmed did my best to confirm there are no regressions to color contrast as a result of this. Wasn't able to find anything so I think we are ready to proceed with this.

Committed ffb4778 and pushed to 9.3.x. Thanks!