This is a follow up from #2938194: Focus styles should meet accessibility guidelines in Umami theme

===
Underlines are best for indicating the presence of a link at all, in their resting style, especially inside sentences. The footer links would be better with an underline to distinguish them from nearby text, so they'll need an outline focus style.
===

CommentFileSizeAuthor
#26 02-after-patch-applied.png130.18 KBmarkconroy
#26 01-current.png129.17 KBmarkconroy
#25 interdiff.txt405 bytesVidushi Mehta
#25 2942489-25.patch1.61 KBVidushi Mehta
#22 2942489-22.patch2.21 KBmukeysh
#20 2942489-20.patch2.15 KBidebr
#20 interdiff-13-20.txt587 bytesidebr
#18 Screen Shot 2018-05-07 at 12.54.26.png1.2 MBHAL 9000
#15 After-core-links-in-footer-2942489-13-8.6.x.patch.png59.65 KBTaoStyle
#15 Before-core-links-in-footer-2942489-13-8.6.x.patch.png57.75 KBTaoStyle
#13 core-links-in-footer-2942489-13-8.6.x.patch2.27 KBcytherion
#11 2942489-mouse-hover.png412.95 KBmatias
#11 2942489-footer.png440.85 KBmatias

Comments

markconroy created an issue. See original summary.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Some reasons why this is important here http://www.webaxe.org/keep-the-underline-text-links/

markconroy’s picture

markconroy commented
Issue tags: +dclondon
markconroy’s picture

markconroy commented
Issue tags: +Umami stable blocker
ankitjain28may’s picture

ankitjain28may commented

As i can see, We have border-bottom: 1px dotted; style by default to all the links in the footer. Let me know your views over this - To underline the links in the footer section to make it more accessible.

markconroy’s picture

markconroy commented
Issue tags: +Nashville2018
cytherion’s picture

cytherion commented

Working on this issue with my team at the mentored core sprint - Drupalcon Nashville 2018

matias’s picture

matias commented

The links on the footer have no underline, but the rest of the links on the site are not underlined either.
What is your view on this. Should we underline all the links to keep it consistency or only the ones on the footer?

TaoStyle’s picture

TaoStyle commented

Working on this issue with my team at the mentored core sprint - Drupalcon Nashville 2018

seeallie’s picture

seeallie commented

Great to work with others on this! It's great to have an out-of-the-box profile for D8.6.

matias’s picture

matias commented
StatusFileSize
new 2942489-footer.png440.85 KB
new 2942489-mouse-hover.png412.95 KB

Working with susannecoates, TaoStyle and seeallie at DrupalCon Nashville sprint.
As you can see on the attached screenshots there is no underline on the footer, but there is an underline on hover.

Should we update the links style to underline all the links or only the ones on the footer?
Any accessibility experts could advise on how to procede.

cytherion’s picture

cytherion commented

WCAG 2.0 requires links be distinguished by a method other than colour and have a minimum contrast ratio of 3:1 (4.5:1 in WGAC 2.1) from surrounding text. Underlining maximises link accessibility without altering theme colours. Patching the theme to have footer links always underlined.

cytherion’s picture

cytherion commented
StatusFileSize
new core-links-in-footer-2942489-13-8.6.x.patch2.27 KB

@TaoStyle @seeallie @matias Created patch for issue - added underline to links in the footer region css and removed it from promo and menu footer blocks css.

cytherion’s picture

cytherion commented
Status: Active » Needs review
TaoStyle’s picture

TaoStyle commented
StatusFileSize
new Before-core-links-in-footer-2942489-13-8.6.x.patch.png57.75 KB
new After-core-links-in-footer-2942489-13-8.6.x.patch.png59.65 KB

Tested the patch core-links-in-footer-2942489-13-8.6.x.patch

Original State:
Before patch

With patch:
With patch applied

mgifford’s picture

mgifford
he/him
Primary language English
commented

I like this. The text-decoration: none; is generally a bad idea.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented 
+++ b/core/profiles/demo_umami/themes/umami/css/components/navigation/menu-footer/menu-footer.css
@@ -18,7 +18,7 @@
-  text-decoration: none;
+  /* text-decoration: none; */

Instead of commenting this, we should probably just remove this line?

HAL 9000’s picture

HAL 9000 commented
Status: Needs review » Needs work
StatusFileSize
new Screen Shot 2018-05-07 at 12.54.26.png1.2 MB

The core-links-in-footer-2942489-13-8.6.x.patch doesn't work anymore because of the block-type-footer-promo-block and 'footer-promo-content a' classes were given a 'display: block' properties in the footer-promo.css.

dinesh18’s picture

dinesh18 commented

I tried applying the patch mentioned in #13, but the patch is not getting applied cleanly. Throws error as mentioned in #18

idebr’s picture

idebr commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-13-20.txt587 bytes
new 2942489-20.patch2.15 KB
  1. Reroll after #2960797: Footer of Umami, [find out more] links the whole div was committed
  2. #17 removed the commented css
markconroy’s picture

markconroy commented
Status: Needs review » Needs work

Hi Folks,

Thanks for working on this issue.

I'm going to mark it as 'Needs work' again. At the moment with the patch from #20 we have underlines in the links in the footer, but there is no change when we hover over the links. We'll need to have the underline removed on hover, so we fulfill the criterion from #12 "WCAG 2.0 requires links be distinguished by a method other than colour and have a minimum contrast ratio of 3:1 (4.5:1 in WGAC 2.1) from surrounding text."

Also, there is no change of colour for the links on hover, maybe that's acceptable if we have the underline being removed on hover. @mgifford can you verify as core a11y maintainer?

mukeysh’s picture

mukeysh commented
StatusFileSize
new 2942489-22.patch2.21 KB

@markconroy as you suggested i have removed underline from hover. Please review.

mukeysh’s picture

mukeysh commented
Status: Needs work » Needs review
markconroy’s picture

markconroy commented
Status: Needs review » Needs work 
+++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/footer-promo/footer-promo.css
@@ -20,14 +20,12 @@
   background-color: inherit;
   color: #fff;
   font-weight: bold;
-  text-decoration: none;
 }
 
 .block-type-footer-promo-block .footer-promo-content a:active,
 .block-type-footer-promo-block .footer-promo-content a:focus,
 .block-type-footer-promo-block .footer-promo-content a:hover {
   background-color: transparent;
-  text-decoration: underline;
 }
 

+++ b/core/profiles/demo_umami/themes/umami/css/components/navigation/menu-footer/menu-footer.css
@@ -18,7 +18,6 @@
-  text-decoration: none;

@@ -27,7 +26,6 @@
 }

These changes mean we are now using the default underline settings for links from base.css

Actually, we can probably also remove the lines in that file that set the underline, since it should be there by default in browsers.

+++ b/core/profiles/demo_umami/themes/umami/css/components/regions/footer/footer.css
@@ -14,6 +14,14 @@
+.footer a {
+  text-decoration: underline;
+}
+
+.footer a:hover {
+  text-decoration: none;
+}
+

This section seems like it is not needed, given the changes above it. Given the changes in the menu-footer.css file, it looks like we are now using the default styling for links - underline as default, no underline on hover.

Can we remove these lines from the patch, and I can test again. Thanks.

Vidushi Mehta’s picture

Vidushi Mehta commented
Status: Needs work » Needs review
StatusFileSize
new 2942489-25.patch1.61 KB
new interdiff.txt405 bytes

Removed the lines as mentioned by #24.

markconroy’s picture

markconroy commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 01-current.png129.17 KB
new 02-after-patch-applied.png130.18 KB

Thanks @Vidushi Mehta and others for working on this. I'm marking it as RTBC now.

Current links in footer:

After patch applied:

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 25: 2942489-25.patch, failed testing. View results

markconroy’s picture

markconroy commented
Status: Needs work » Reviewed & tested by the community

According to the test results, this passed. https://www.drupal.org/pift-ci-job/968994

Can we have it committed please?

  • lauriii committed 03ab0f8 on 8.6.x 
    Issue #2942489 by idebr, Vidushi Mehta, susannecoates, Mukeysh,...

  • lauriii committed 6619dd6 on 8.5.x 
    Issue #2942489 by idebr, Vidushi Mehta, susannecoates, Mukeysh,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Looks good! ✨Thanks everyone!

Committed 03ab0f8 and pushed to 8.6.x. Also cherry-picked a href="http://cgit.drupalcode.org/drupal/commit/?id=6619dd6">6619dd6 and pushed to 8.5.x 🚀Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.