Change link underline technique to linear-gradient#27
Conversation
There was a problem hiding this comment.
You've got a "To be documented" note here. Can you add that documentation please?
There was a problem hiding this comment.
Converted to .u- mixin and moved to cf-utilities.less.
|
We should offer a way to break the underlines without having to use a mixin. Like maybe Also the demo doesn't seem to be working right. The first link looks ok but the rest don't appear to have the breaking underline, or it's very hard to tell. The "Breaking link underlines mixin" demo
|
|
I considered doing that. The prospect of changing background colors worries me slightly, but it probably makes sense to have a utility class with a default of white. Perhaps including white in the name would be good for awareness. Hmm, yeah. The breaking effect is not as good as I thought, either. It's working, just not well enough for some letters. Maybe a difference in browser rendering. What are these screenshots from? |
|
Chrome |
|
Let's roll with it as-is for now, since it's not the core purpose of this change. In the near future, I'll investigate possible optimizations to the breaking technique, starting with this excellent article: https://eager.io/blog/smarter-link-underlines/ |
|
I'd like for the breaking underlines to be more than a mixin though. That should be easy to add right? |
|
Sure, I'll add that. |
|
I think |
|
Updated. |
|
Thanks. Let's merge after the Design Manual meet up tomorrow. |
|
Status: waiting for others to play around with the demo as stated here: cfpb/design-manual#244 |
- Breaking underlines now interact better with serifed descenders - Text shadow removed when selecting text that includes links - Link color darkened by 10% when selecting text that includes links
|
Ping! |
|
Sorry, this is a really far-reaching change. I think it's on hold until we get all the DM surge work out the door. Then we can buckle down and address all of the places that this change will affect. |
|
Should we close it and start over? Things have changed quite a bit in this time |
|
Not so much has changed in cf-core that this PR isn't still useful. I still have an |
4 participants
If I did this right, @himedlooff, all you should need to do is pull this down, re-run
npm installandgrunt vendor, and rebuild the docs.Let me know how it goes.