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

Figma update: Add white link variation to va-link #2776

Closed
3 tasks
caw310 opened this issue Apr 25, 2024 · 16 comments
Closed
3 tasks

Figma update: Add white link variation to va-link #2776

caw310 opened this issue Apr 25, 2024 · 16 comments

Comments

@caw310
Copy link
Contributor

caw310 commented Apr 25, 2024

Description

There's a request to allow to be a white link instead of default blue #2602. We need to add this to Figma

Details

Tasks

  • Update va-link component with a white link instead of the default blue
  • Get review from designer(s)

Acceptance Criteria

  • va-link design has a variation with a white link
@caw310
Copy link
Contributor Author

caw310 commented Apr 25, 2024

Hey team! Please add your planning poker estimate with Zenhub @babsdenney @danbrady @LWWright7

@Andrew565 Andrew565 changed the title Figma update: Add while link variation to va-link Figma update: Add white link variation to va-link Apr 25, 2024
@LWWright7
Copy link

@danbrady @babsdenney - This one is ready for review!

@LWWright7 LWWright7 self-assigned this May 24, 2024
@danbrady
Copy link
Contributor

@LWWright7 Thanks for taking this on, Lucas! This is looking great. I think we should restructure a bit though, because "reverse" is more of a "variation" (or maybe a "theme") than a "state". In other words, for example, the link can be both "reverse" and have a "hover" state. Or it can be "reverse", "active", and "visited" at the same time.

So every combination that currently existed before, needs to be reverses. The end result essentially double the total number of combinations.

@LWWright7
Copy link

LWWright7 commented May 28, 2024

@danbrady ahhhh....I see what you're saying.....it should have been a column instead of a row, basically....right? or do they all need to be created in reverse?

@danbrady
Copy link
Contributor

Yeah, multiple columns though. Think of it this way: The current component has 7 variations, 5 states for single line and 5 states with 2 lines. That's 70 combos. We need all those combos on a reverse background, so another 70 combinations. So, essentially, the whole component is getting duplicated and put on a dark background. (We don't want an actual duplicated component though because we want to handle it all changing properties.) Make sense?

@LWWright7
Copy link

@danbrady Yep, totally. makes sense One question though....for the variant, would it make sense to just create a boolean value for the reverse variations (ex: Reverse? T/F) or how do you think that should be handled?

@danbrady
Copy link
Contributor

@LWWright7 Hmm... good question. A boolean could work as there are just two "modes". Although, if we ever add did a third mode though that would make updating a bit harder. It might be safer to make it a variant property so it gets a dropdown list that we can add/subtract too as needed.

@LWWright7
Copy link

@danbrady Good point! I went with "Dark/Light" mode instead of the boolean.

One last question...I looked on the VADS Storybook for documentation on the "visited" link for the dark ("reverse") and didn't really see that topic covered or an example of it there. Do you know what it should be?

Screen Shot 2024-05-28 at 12.25.03 PM.png

@danbrady
Copy link
Contributor

@LWWright7 How does the web component currently handle it? I don't think we would want that purple as it would be hard to see on dark backgrounds. (BTW, I think majority of the time dark backgrounds are either "primary dark" or "primary darker".) Maybe run the contrast reqs by Ryan to get an accessibility standpoint?

@LWWright7
Copy link

@danbrady That's why I ask...the web component doesn't even seem to have a "visited" state...the text stays the same color (white) and nothing changes. It behaves that way in storybook and on va.gov live atm. So I'm guessing that's the expected behavior....so no visited variation/state?

@danbrady
Copy link
Contributor

Yeah, that seems like a potential error to me but maybe accessibility weighed in on that? It's probably better that we find out if that's acceptable or not.

@LWWright7
Copy link

@rsmithadhoc Would you happen to know the answer to this question? Is this behavior mentioned above normal and expected (that is, no visited state for link in reverse/dark mode?) or is there another color that would meet a11y as the normal state for visited doesn't seem that it would pass contrast testing?

@rsmithadhoc
Copy link
Contributor

rsmithadhoc commented May 28, 2024

@LWWright7

I think the issue here is it would have to pass WCAG AA on the lightest background that it would be used on. I think that might be the light blue used in the gradient on the hero section on va.gov, --vads-color-primary: #005ea2;.
image

To achieve that for normal-sized text, it would need to have a 4.5:1 contrast ratio. If we went with our visited link purple, it would need to be #DBD0E7. The action link had a white link before we updated the standard link and it also doesn't have a visited state. I'm thinking that visited state may have been intentionally kept white for the action link, because the color would have to be pretty close to white anyway, so the contrast between active and visited links would be insufficient (very light purple vs. white).
image

According to WCAG 2.2, setting a link's visited status is not a responsibility of the author. However, if we do, it has to meet contrast ratio requirements. I'm okay with omitting the visited state from this (keep it white), unless we have a suitable color that looks good.

@LWWright7
Copy link

@babsdenney & @danbrady This one is ready for another look!

@danbrady
Copy link
Contributor

Thanks for tackling all those variations! We should remove "reverse" from the "state" property because we're using the "mode" property instead. Also, even though the visited reverse link is visually the same as default, I think we still need to create those variations. This way there is no confusion for a designer who's looking to set those specific properties in their local design file. If those properties are not there, they might think it's a DS error.

@LWWright7
Copy link

@danbrady ahh.....good call about that reverse state! I neglected to clean that up after my first attempt at tackling this ticket. Should be all cleaned up now.

And good point about the visited variation. I went ahead and added that too. Oh, and also changed the background to the primary blue...since, as you said before, that's the color mostly used instead of black.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants