Problem/Motivation
The styles we currently have for links changes the default text color to the blue one. But just a color change is not enough for some uses to differentiate normal text from links.
Proposed resolution
Add an underline to all links by default as it is defined on the Design System, and let it be overridden on specific situations if needed.
This means reversing the way CSS is currently arranged. Instead of removing underlines globally, and repairing them where needed, we will preserve the underline globally and remove it in cases where it is safe to do so.
CSS Before:
/* Remove underlines globally */
a, .link { text-decoration: none; }
/* Need to repair the underline. */
.messages a { text-decoration: underline; }
/* Inherits global style, no underline. */
.tabs_link { }
CSS After:
/* Preserve link underlines globally. */
a, .link { }
/* Inherits global style, links underlined. */
.messages a { }
/* Tabs have distinct navigation group styling, safe to remove the underline. */
.tabs_link { text-decoration: none; }
Remaining tasks
- Remove
a, .link { text-decoration: none; }
from elements.css - Update the global hover/focus styles, remove
text-decoration: underline;
. - Clean up any other
text-decoration: underline;
which are no longer necessary. - Identify any links where we can safely remove the underline because there is an alternative signifier (e.g. primary local tasks).
User interface changes
Links will be underlined from now on, as the global default style.
Comment | File | Size | Author |
---|---|---|---|
#31 | interdiff_23-31.txt | 235 bytes | kostyashupenko |
#31 | claro-underline_link-3079134-31.patch | 4.69 KB | kostyashupenko |
| |||
#28 | Screen Shot 2019-09-26 at 17.30.52.png | 10.69 KB | lauriii |
#25 | colors.png | 101.62 KB | saschaeggi |
#23 | claro-underline_link-3079134-23.patch | 4.39 KB | charlieweb82 |
|
Comments
Comment #2
Swapnil_Kotwal CreditAttribution: Swapnil_Kotwal at Asentech LLC commentedComment #3
ckrinaUpdating with the hover styles
Comment #4
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedThanks for filing this. I'm adding some demo code for the resolution.
Basically it means reversing the current CSS policy. Right now we are removing the underlines globally, then having to anticipate all the places where the underlines must be restored. But we'll never anticipate them all. Instead, it's safer to keep the link underlines globally, then remove them from places where we are confident that an alternative signifier has been provided (e.g. primary local tasks block).
The essential change here is removing the
a, .link { text-decoration: none; }
rule. I think it's a good idea to commit that change ASAP, then follow-up fixing the other instances where we do want to remove the underline. Are we OK with having multiple commits on the same issue?Underlining links by default, for their resting state, means we can't use underlines as a focus style. So
a:focus, .link:focus { text-decoration: underline; }
needs to go too. That's why we were pushing for a focus style which surrounds the element on all 4 sides.Hover styles are a nice bonus, but less essential because the pointer itself is the main signifier (mouse cursor, or actual fingertip). The proposal here for underlines to disappear on hover will work nicely when paired with the focus outline. So on hover the outline disappears, and the cursor changes to a hand icon. On focus, the underline is replaced by a 4-sides outline.
Comment #5
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedRelated issue for the Seven theme. This has more information and examples. #2958282: Links inside surrounding text fail WCAG Use-of-Color in Seven theme.
Comment #6
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedWCAG 1.4.1 Use of Color is a level A success criterion, so bumping this to major. Specifically, it's about avoiding F73: Failure of Success Criterion 1.4.1 due to creating links that are not visually evident without color vision
Comment #7
Swapnil_Kotwal CreditAttribution: Swapnil_Kotwal at Asentech LLC commented@Cristina Chumillas (ckrina). I have uploaded a patch file for the issue. Kindly review.
I doubt it would work.
Kindly review.
Comment #8
fhaeberleSetting to NR because of patch.
Comment #9
fhaeberle... and undo assignment and adjust priority, sorry.
Comment #10
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedKeep this at major please. The accessibility maintainers generally treat WCAG level-A issues as major.
Comment #11
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented@Swapnil_Kotwal - thanks for working on this.
I tried the patch from #7. Unfortunately it's a bit of a mess, so let's look at the problems:
css/src/base/elements.css
this is the biggest problem. The patch in #7 replaces the entireelements.css
file with a patch file. So it's no longer a CSS file, instead it has lots of git information. This causes the Yarn CSS build commands to fail.There isn't a quick fix for this. I think the easiest thing to do would be to start again with this file, by resetting the changes and checking out a fresh copy.
We're only interested in the line which removes
text-decoration: underline;
. Drupal 8 ships with an.editorconfig
file, to help us avoid indentation changes like this. Check to see it is working:.editorconfig
, but some of them need a plugin for it. See https://editorconfig.org/.editorconfig
file is missing from your development environment. It's a hidden file (name starts with a dot) so it sometimes gets missed when copying files.Once these are dealt with, the patch file should be a lot simpler (and shorter).
Comment #12
charlieweb82 CreditAttribution: charlieweb82 at Chapter Three commentedPatch update remove the underline to links and focus
Comment #13
ckrinaThanks for working on this @charlieweb82! I've found some issues on the patch.
This is removing all the code inside the declaration. I think we shouldn't leave empty declarations, and specially extra spaces.
Here's the same: it leaves an empty space and an empty declaration.
Why is this removing the underline from messages? We should keep it.
Another empty declaration after changes.
Empty declaration after removing the code.
Also, probably reviewing @andrewmacpherson's suggestion from comment #11 to config your editor might help you avoiding empty spaces added to the patch. :)
Comment #14
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented#13.3:
Because links will be underlined by default, aftee elements.css has been fixed. We don't need to add the underline in messages.css
Comment #15
charlieweb82 CreditAttribution: charlieweb82 at Chapter Three commentedbase on the comment #4 remove the a, .link rule the other section have underline everywhere so I added base on the component the text-decoration: none to remove underline on some elements. I fix the spacing and empty rules.
Comment #16
lauriiiIs there a particular reason we want to remove the underline from links in tables?
Comment #17
charlieweb82 CreditAttribution: charlieweb82 at Chapter Three commentedWhere the update of the patch base on comment #16
Comment #18
fhaeberleCorrecting custom commands fail: formatting issue.
@charlieweb82 Before creating the patch, please run `yarn lint:css` (in terms of css changes) to check for errors. Thanks for your help! :)
Comment #19
lauriiiWe should remove the underline from links in pager:
Comment #20
charlieweb82 CreditAttribution: charlieweb82 at Chapter Three commentedUpdate patch base on comment #19 from @lauriii
Comment #21
charlieweb82 CreditAttribution: charlieweb82 at Chapter Three commentedupdate patch fail test
Comment #22
lauriiiComment #23
charlieweb82 CreditAttribution: charlieweb82 at Chapter Three commentedupdate patch with fix.
Comment #24
fhaeberleOut of the meeting yesterday:
Patch applies and works for me. Underline in pager was removed.
Comment #25
saschaeggiAlso making sure buttons don't use the underline as well. We should also use a darker blue color for :hover & :active. The colors are specified:
also you can have a look here: https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Drupal-Design-system...
Comment #26
fhaeberle@saschaeggi Changing the (underline) colors sounds like a follow-up (EDIT: I created one.) and should be also applied globally.
Comment #27
saschaeggi@fhaeberle should be the default for all links. Thanks. I linked it.
Comment #28
lauriiiFor more consistency, we should extend this to
button.link
elements as well:Comment #29
Gayathri J CreditAttribution: Gayathri J commented#23
I applied this patch works for me underline in pager was removed.
Comment #30
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented#28 - yes, good catch.
Comment #31
kostyashupenkoComment #32
lauriiiComment #34
lauriiiThis looks great! 🚀 Thanks everyone for your help!
Comment #35
Gayathri J CreditAttribution: Gayathri J at UniMity Solutions Pvt Limited commented