This is a followup issue from last night's Drupal Accessibility Office Hours.

The current implementation of the skip link shifts the entire page down when it has focus. This can be disorienting to users when they quickly tab around. It can be seen in the video recording of the focus-style reviews. Demo and discussion of the skip-link starts at around the 4 minute mark.

We can use the method similar to comments #5 and #8 from #3119789: Make skip-link style at consistent at all breakpoints

1. The skip link should be absolutely positioned so it does not affect layout when in focus
2. We need to make sure it respects the administration toolbar placement and is always visible when in focus.

CommentFileSizeAuthor
#35 Before Patch 3153265.png168.43 KBchetanbharambe
#35 After Patch 3153265.png339.72 KBchetanbharambe
#34 interdiff_29-34.txt574 byteskiran.kadam911
#34 3153265-34.patch1023 byteskiran.kadam911
#31 after-patch.png75.36 KBbhumikavarshney
#31 BEFORE-PATCH.png163.79 KBbhumikavarshney
#29 3153265_26_skip_link_focus.patch987 bytesmherchel
#26 interdiff-21-26.txt394 byteskishor_kolekar
#26 3153265_26_skip_link_focus.patch987 byteskishor_kolekar
#23 Screenshot 2021-04-11 at 11.38.44.png90.79 KBgauravvvv
#23 Screenshot 2021-04-11 at 11.35.57.png112.5 KBgauravvvv
#22 3153265-after-mobile.gif4.99 MBabhijith s
#22 3153265-after-desktop.gif1.6 MBabhijith s
#22 3153265-before.gif4.43 MBabhijith s
#21 3153265_21_skip_link_focus.patch1.07 KBhinal05
#20 3153265_20_skip_link_focus.patch1.09 KBhinal05
#20 3153265_20_mobile_after.gif871.42 KBhinal05
#20 3153265_20_after.gif593.26 KBhinal05
#17 Home___Drupal_and_asdfsdf___Drupal.png56.73 KBmherchel
#16 after.png152.22 KBkomalk
#16 3153265-16.patch945 byteskomalk
#16 3153265-16.patch945 byteskomalk
#11 mobile.mp4323.28 KBambuj_gupta
#10 overlay.mp4489.01 KBambuj_gupta
#5 after_patch.png187.16 KBambuj_gupta
#5 admin.png230.44 KBambuj_gupta
#3 after_patch.gif1.54 MBkiran.kadam911
#3 skip-link-focus-3153265-3.patch741 byteskiran.kadam911

Comments

mherchel created an issue. See original summary.

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: Unassigned » kiran.kadam911
Status: Active » Needs work
kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: kiran.kadam911 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new skip-link-focus-3153265-3.patch741 bytes
new after_patch.gif1.54 MB

Kindly review the attached patch.

Thanks!

ambuj_gupta’s picture

ambuj_gupta commented
Assigned: Unassigned » ambuj_gupta
ambuj_gupta’s picture

ambuj_gupta commented
StatusFileSize
new admin.png230.44 KB
new after_patch.png187.16 KB

Testing Steps:
1. Apply the patch.
2. Go to site and clear the cache.
3. Visit any page. And press tab and verify the position of Skip to the Main Content link.
3. Verified it for anonymous and authenticated users

Testing Results:
Tested after applying the patch created by @kiran.kadam911. and It's working as expected.

ambuj_gupta’s picture

ambuj_gupta commented
Assigned: ambuj_gupta » Unassigned
Status: Needs review » Reviewed & tested by the community
andrewmacpherson’s picture

andrewmacpherson commented
Issue summary: View changes

Adding link to video recording of the review in accessibility office hours.

andrewmacpherson’s picture

andrewmacpherson commented

It would be good to get screenshot/gif of the behaviour at narrow viewports (say 360px)

andrewmacpherson’s picture

andrewmacpherson commented
Parent issue: » #3153260: Standardize Olivero's focus states across all non-form controls
ambuj_gupta’s picture

ambuj_gupta commented
StatusFileSize
new overlay.mp4489.01 KB

Video recording of review attached.

ambuj_gupta’s picture

ambuj_gupta commented
StatusFileSize
new mobile.mp4323.28 KB

Are we going to display Skip To Main Link on mobile as well? As per my understanding, we don't need Skip Main Links on Devices because there we don't have keyboard Navigation.
I think we need to hide it for Devices.

andrewmacpherson’s picture

andrewmacpherson commented

Thanks for the video demos @ambuj_gupta. Both of the videos in #10 and #11 are good.

In #11, showing a narrow (mobile) viewport, the skip link obscures the site name while the skip link is focused. This is acceptable per WCAG success criterion 1.4.13 Content on Hover or Focus.

Are we going to display Skip To Main Link on mobile as well? As per my understanding, we don't need Skip Main Links on Devices because there we don't have keyboard Navigation.

YES - we need to make this skip link available on all devices.

It's incorrect to say that phones, tablets, etc. don't have keyboard navigation:

  • Many phones and tablets don't have built-in physical keyboards, but some do. The first Android device on sale (HTC G1) had a physical keyboard, and they keep cropping up in new models too. Most recently, some Blackberry Android devices have built-in physical keyboards. I've used one to operate the Talkback screen reader!
  • Regardless of whether the device has a physical keyboard built in, all the major mobile operating systems support the use of external keyboards via Bluetooth. Android also supports USB keyboards. There are hundreds of compact Bluetooth keyboards on the market, and their use is popular. A Google image search for "iPhone Bluetooth keyboard" and "iPad Bluetooth keyboard" reveals different products and situations where they are used.
  • When we talk about a website being "keyboard" accessible, we don't necessarily mean that someone is using a typical QWERTY sort of keyboard. It's also important because there's a huge variety of assistive technology (hardware and software) which can be set up to send keyboard commands. This includes things such as programmable keypads, games controllers, "macro" recorders, scripting tools, and other approaches.
kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Issue tags: +Bug Smash Initiative
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Title: Skip link focus should not create layout shift » Olivero: Skip link focus should not create layout shift
Project: Olivero » Drupal core
Version: 8.x-1.x-dev » 9.1.x-dev
Component: Code » Olivero theme
Status: Reviewed & tested by the community » Needs work
komalk’s picture

komalk commented
Assigned: Unassigned » komalk
komalk’s picture

komalk commented
Assigned: komalk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3153265-16.patch945 bytes
new 3153265-16.patch945 bytes
new after.png152.22 KB
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Priority: Normal » Minor
Status: Needs review » Needs work
StatusFileSize
new Home___Drupal_and_asdfsdf___Drupal.png56.73 KB

The skip link isn't visible when the admin toolbar is present. We need to account for that (in all of its permutations).

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Related issues: +#2958478: Toolbar height calculation is faulty in multiple cases

Blocker https://www.drupal.org/project/drupal/issues/2958478 to resolve issue pointed in previous comment

hinal05’s picture

hinal05 commented
Assigned: Unassigned » hinal05
hinal05’s picture

hinal05 commented
Assigned: hinal05 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3153265_20_after.gif593.26 KB
new 3153265_20_mobile_after.gif871.42 KB
new 3153265_20_skip_link_focus.patch1.09 KB

Please review the attached patch.

hinal05’s picture

hinal05 commented
StatusFileSize
new 3153265_21_skip_link_focus.patch1.07 KB

Please refer updated patch.

abhijith s’s picture

abhijith s commented
StatusFileSize
new 3153265-before.gif4.43 MB
new 3153265-after-desktop.gif1.6 MB
new 3153265-after-mobile.gif4.99 MB

Applied patch #21 and it works fine.Adding screenshots below.

Before patch:
before

After patch:
after1

Mobile
after2

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new Screenshot 2021-04-11 at 11.35.57.png112.5 KB
new Screenshot 2021-04-11 at 11.38.44.png90.79 KB

Patch #21, Working fine and fixed the layout shift issue.
Adding after patch screenshot for reference.

Moving to RTBC.

gauravvvv’s picture

gauravvvv commented
Status: Needs review » Reviewed & tested by the community
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Needs work

This looks good except for the empty line that's removed. 

+++ b/core/themes/olivero/css/components/skip-link.pcss.css
@@ -18,8 +18,14 @@
-

No need to remove this empty line.

kishor_kolekar’s picture

kishor_kolekar commented
Issue summary: View changes
StatusFileSize
new 3153265_26_skip_link_focus.patch987 bytes
new interdiff-21-26.txt394 bytes

Hey, I have tried to cover the points mentioned in #25
Please review

kishor_kolekar’s picture

kishor_kolekar commented
Status: Needs work » Needs review
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Version: 9.1.x-dev » 9.2.x-dev
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
StatusFileSize
new 3153265_26_skip_link_focus.patch987 bytes

Re-uploading the exact same patch from #26, so we can test this against 9.2.x (Drupal CI wouldn't let me add a test through the 'add test' link)

sakthivel m’s picture

sakthivel m commented

Thanks patch @mherchel, @kishor_kolekar
patch
#26 patch applied. The patch is working fine

bhumikavarshney’s picture

bhumikavarshney commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new BEFORE-PATCH.png163.79 KB
new after-patch.png75.36 KB

HI @mherchel,
Patch #29, Working fine.
Fixed the layout shift issue.
Adding after patch screenshot for reference.

Moving to RTBC.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lauriii’s picture

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

position: absolute makes the focus ring visible. The while focus ring doesn't work well in this use case. Since the focus ring is not visible before, we should probably make it invisible here.

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Status: Needs work » Needs review
StatusFileSize
new 3153265-34.patch1023 bytes
new interdiff_29-34.txt574 bytes

Providing updated & re-rolled 9.3.x patch as per #33 removing outline on focus.

Kindly review.

Thanks!

chetanbharambe’s picture

chetanbharambe commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new After Patch 3153265.png339.72 KB
new Before Patch 3153265.png168.43 KB

Verified and tested patch #34.
Patch applied successfully and looks good to me.

Testing Steps:
# Press tab button
# User is able to see "Skip to main content"

Expected Results:
# User should not see white focus ring under skip to main content

Actual Results:
# Currently user is able to see the white focus line under skip to main content.

Can be a move to RTBC.

  • lauriii committed 8f711d9 on 9.3.x 
    Issue #3153265 by kiran.kadam911, hinal05, komalk, mherchel,...

  • lauriii committed eb07dc5 on 9.2.x 
    Issue #3153265 by kiran.kadam911, hinal05, komalk, mherchel,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 8f711d9 and pushed to 9.3.x and 9.2.x. Thanks!

gauravvvv’s picture

gauravvvv commented

Status: Fixed » Closed (fixed)

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