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

Repository for design.va.gov website
https://design.va.gov
39 stars 58 forks source link

Button color choices are out of sync across va.gov #1585

Closed humancompanion-usds closed 1 year ago

humancompanion-usds commented 1 year ago

Bug Report

What happened

It appears that we have 3 different button styles in play across the site:

  1. Our web-component: va-button
  2. The default Formation styling for: <button class="usa-button>
  3. Instances of USWDS v1 styling for: <button class="usa-primary-button">

Each of those 3 use different colors for hover, focus, and active states. To make matters worse, the Sketch library dictates yet another version of those states.

What I expected to happen

I expect all buttons to behave the same way visually when I hover over them and click them.

Reproducing

Have a look at this page which has all 3 instances on one page: https://www.va.gov/health-care/apply/application/introduction

Default

screencapture-va-gov-health-care-apply-application-introduction-2023-02-28-13_55_23

Already we can see the default states don't match on this page as the web-component has an odd bolding of the text. The colors, however, are correct - all var-color-primary which is #0071bb. The hover state of all 3 is actually the same, yay!, it's var-color-primary-darker which is #003E73.

Focus, no hover

Screen Shot 2023-02-28 at 2 06 35 PM

Problems start when we apply focus via the keyboard (i.e. just focus with no hover). As seen above the .usa-button:focus style applies var-color-primary-darker (#003E73) whereas the other two button variations do not. The combination of :hover and :focus are the same and correct.

Active

Screen Shot 2023-02-28 at 2 19 57 PM

Our web-component doesn't change shade when moving from :hover to :active. The other two buttons go to $color-blue-darkest form USWDS v1 which is #112E51. This is not the same as $color-primary-darkest which is #102E51. This is one of the inconsistencies of v1, which is particularly annoying. In v3 they are both blue-warm-80v which is #162e51.

In addition, the Sketch library uses that blue-warm-80v, or $theme-color-primary-darker from USWDS v3. So it seems like we accidentally jumped the gun there.

Urgency

This is very broken and user visible (albeit we're talking about shades of blue so users may not notice). Thus I'd like us to start addressing it as soon as we can. We need to discuss as a team how to remedy this.

Details

Here's a list of all of the problems I found:

  1. va-button (web-component) on this page at least, has an odd double-bold state for the button text.
  2. Focus states out-of-sync: The .usa-button (item 2 in the list above) has a background-color applied that the other 2 buttons do not get.
  3. Active states out-of-sync: The web-component doesn't get darker when in active state and it should.
  4. Sketch library uses a v3 value for active state on the primary and secondary buttons and that color is not yet in our system.
humancompanion-usds commented 1 year ago

For problem #2 in the Details list above our sourcemap is pointing towards _m-vet-nav.scss. Not sure whether the sourcemaps are accurate or not but removing that override would at least get the focus states of usa-button and usa-primary-button to be the same.

Screen Shot 2023-02-28 at 2 34 56 PM

humancompanion-usds commented 1 year ago

One clarification, we correctly set our $color-primary-darkest to 112e51 in Formation. Thus we caught the USWDS v1 inconsistency in code but that probably led to some problems in Sketch.

Button variation State Hex value Token value Target value
va-button active #003e73 color-primary-darker color-primary-darkest (#112E51)
usa-button active #112E51 color-primary-darkest stays same
usa-button-primary active #112E51 color-primary-darkest stays same
va-button active & focus #003e73 color-primary-darker color-primary-darkest (#112E51)
usa-button active & focus #003E73 color-primary-darkest color-primary-darkest (#112E51)
usa-button-primary active & focus #112E51 color-primary-darkest stays same

Thus I think these are the 3 changes we need:

  1. In code fix va-button web-component to use color-primary-darkest for active state.
  2. In code fix usa-button to not have a focus color override.
  3. In Sketch fix default:active in the primary button to be color-primary-darkest and default:active plus default:hover in the secondary button to be color-primary-darkest.
GnatalieH commented 1 year ago

@humancompanion-usds thanks so much for this synopsis. It's clear from your comment what needs to change in the Sketch library.

UPDATE: I made the following changes:

In Sketch fix default:active in the primary button to be color-primary-darkest and default:active plus default:hover in the secondary button to be color-primary-darkest.

jamigibbs commented 1 year ago

I'm in the process of aligning the state colors but I wanted to make a clarification about the "bold" treatment that was mentioned.

The reason that there is an observable weight different between va-button and the Formation button classes (.usa-button, .usa-button-primary, etc) is because that latter is applying -webkit-font-smoothing: antialiased; to the font:

Screenshot 2023-03-06 at 10 01 40 AM

The correct approach here would be to remove -webkit-font-smoothing from Formation rather than add it to the web component.

Here are some references as to why we would want to do that:

My question then would be if we were comfortable keeping -webkit-font-smoothing on the Formation button classes or if we'd also like to remove it as part of this work.

humancompanion-usds commented 1 year ago

Let's remove webkit-font-smoothing.

dcloud commented 1 year ago

A response to "Stop fixing font smoothing" made me wonder if there may be a benefit: https://www.joshwcomeau.com/css/custom-css-reset/#five-font-smoothing-6

Here's the problem: that article was written in 2012, before the era of high-DPI “retina” displays. Today's pixels are much smaller, invisible to the naked eye.

and the kicker (maybe):

In MacOS Mojave, released in 2018, Apple disabled subpixel antialiasing across the operating system. I'm guessing they realized that it was doing more harm than good on modern hardware.

Confusingly, MacOS browsers like Chrome and Safari still use subpixel antialiasing by default. We need to explicitly turn it off, by setting -webkit-font-smoothing to antialiased.

From that post, it sounds like this is a reset-type adjustment to fix blurriness in macOS browsers that still do subpixel antialiasing by default.

jamigibbs commented 1 year ago

@dcloud This does seem to be a polarizing subject and I suspect we can find evidence to back-up either side. Like this one from the Google Fonts PR linked above from 2020 from someone who presents as a W3C WAI/WCAG expert. I think the biggest complaint is that it can reduce contract in a significant way which is an a11y concern.

I'm by no means an expert in this area and I'm happy to implement whatever we feel most comfortable doing. But I just wanted to make sure we understood the pros and cons of using it.

dcloud commented 1 year ago

Sorry, I should clarify that I arrived here via feedback on this PR; while I'm curious about whether one option is preferable, I don't have a preference.

It looks like fonts.google.com currently only applies rules to specific classes, e.g. mat-mdc-form-field. So I'm 🤷, but may tweak my page a bit.

Thanks for pointing me to more resources!

jamigibbs commented 1 year ago

@dcloud Oh no problem! I appreciate the discourse. I'm also trying to wrap my head around all of the resources as well because there does seem to be a lot of conflicting information. I suspect there are situations when it would make sense like for a logo using a font but it seems like when we start to apply the treatment globally to elements (button in our case here), there might be some drawbacks that don't seem initially obvious.

humancompanion-usds commented 1 year ago

Here is my, admittedly biased and short-term, goal: I want the ding dang buttons to all look the same and right now they do not. Long-term my goal is to get inline with what USWDS has done (https://github.com/search?q=repo%3Auswds%2Fuswds%20font-smoothing&type=code), which I must admit is a little confusing. They seem to still have the mixin around but are only using it in limited cases. We have a meeting with them today and I'll ask them what their intentions are.

jamigibbs commented 1 year ago

After connecting with the USWDS team today, they confirmed that font smoothing properties should not be used any longer. The mixins in their system will be or should be removed (ie. the CSS in the mixins should allow the device to control fonts).