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

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

Hover state on Expandable alert error does not meet contrast requirements #2839

Open laflannery opened 1 month ago

laflannery commented 1 month ago

Bug Report

What happened

On Hover, the red background color becomes darker (#f2938c). Unfortunately, this color does not pass WCAG AA color contrast requirements against the gray text color (#3d4551). The ratio is only 4.3:1 instead of 4.5:1

What I expected to happen

The color contrast should meet WCAG AA requirements in all states.

Reproducing

Steps to reproduce:

1. 2. 3. 4.

Urgency

How urgent is this request? Please select the appropriate option below and/or provide details

Details

I reviewed the other alert types and the error was the only one I found that had a contrast issue. But, if the colors are going to change in the other related ticket, it would be good to retest and be sure the new colors pass in all states

danbrady commented 1 month ago

Hi @laflannery - Thanks for filing this issue. Just a clarification, this is for the "Error" variation of Expandable Alert, correct?

laflannery commented 1 month ago

Ya sorry, the red one. We use it when a Facility is closed which is what I came across today

danbrady commented 1 month ago

That makes sense. The red/error variation was removed from guidance and Figma because it's not recommended, however it remains in Storybook. I don't like having an accessibility issue in production so I think it should be fixed. But in the interest of time, would the Alert - Error or it's slim counterpart work for that context?

laflannery commented 4 weeks ago

@danbrady I am going to bring this to my team for discussion but here is some additional detail/context:

While we try to figure out the best way to handle this for existing our existing product, @humancompanion-usds would it be considered launch-blocking at a staging review for a new product if we are using this error state component since it has been removed from the official guidance?

humancompanion-usds commented 4 weeks ago

Within the context of the page, it looks to me at least like the most important thing here is "Need help after hours?". Can a Veteran get help after hours from a facility that is closed? I see it is an 800 number so perhaps that is the case. Still, if the Info Alert is important enough to not collapse then I don't understand why Facility closed would not be of at least equal importance.

Screenshot 2024-05-20 at 10 50 10 AM

That said, the header of Alert - Expandable should not be vads-color-base-darker. Our headers are not gray! Those all should get fixed.

laflannery commented 4 weeks ago

Thanks for the feedback @humancompanion-usds. We are currently working on a redesign of the Vet Center product and the "Need help" alert is being looked at for some of the reasons you mentioned. However I do want to make sure we focus on the expandable alert just for now. A couple clarifying questions:

  1. You mentioned that the header should not be vads-color-base-darker but in Storybook it is using this color.
    • If this were to change, to the standard heading color (`#1b1b1b), you are correct that it would resolve the original accessibility issue.
    • I do want to confirm though that changing this color would be a DST update as part of the component?
      Screenshot 2024-05-20 at 11 03 48 AM
  2. Would using the error state expandable alert be considered launch-blocking at a staging review? I know it has been removed from the DS guidance but it is still in Storybook so I want to be sure my team knows the priority of the tasks they need to get done for the new VBA product.
humancompanion-usds commented 3 weeks ago

I do want to confirm though that changing this color would be a DST update as part of the component?

Yes, that was directed at Dan and the Design System team.

Would using the error state expandable alert be considered launch-blocking at a staging review?

Given that we didn't remove it from Storybook, I'm not going to block you because of our bug. But I feel that errors should not have their content hidden. Expandable alerts only make sense for non-critical things. So you could either switch to a standard Alert, or switch to a Warning or Info Alert - Expandable.

laflannery commented 3 weeks ago

Thanks @humancompanion-usds, I appreciate that this wouldn't be blocking and I know my team does as well.

I have already brought this up with @mmiddaugh as something that we need to take a look at and I I will also talk to @aklausmeier next week when she gets back