Twenty Seventeen: Focused controls may be hidden by the top menu.

[afercia]

When a page is scrolled, the top nav menu become fixed. Depending on the page content, some focused links or buttons may be actually "hidden" behind the nav menu thus potentially creating confusion for keyboard users since the focus indication would actually "disappear" for a while.

See in the screenshot below, where I've made the nav menu semi-transparent just to make the issue clear:

Hat tip Sam Sidler at WCMIL Contributor's Day.

[Fencer04]

I'd be wiling to work on this but what do we think an agreeable solution is?

[davidakennedy]

Thanks @Fencer04! I'm open to ideas for this one as I haven't thought about it too deeply yet. Hopefully, it's something that doesn't require a ton of additional code.

Twenty Fourteen did a bit of this with its skip to content link. See: https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentyfourteen/js/functions.js#L63

[Fencer04]

@davidakennedy that makes sense, I'll review that type of solution and see what I can come up with.

[Fencer04]

Added code to check if something with focus was in the region behind the fixed navigation.

[Fencer04]

I posted a patch that I think is on the right track. I put it in global.js because I wasn't sure where else it should go. It uses the change focus event on all elements. I'm not sure if that is a bad thing or not. I'll need to update it with some comments but wanted to see if someone could review/test it to see if I'm on the right track.

[davidakennedy]

38476.2.patch​ builds on the work of @Fencer04. It:

• Makes sure the scrollTo plugin loads on every page since it can be called at any time now.
• Moves the work into the $( document ).ready( function() part of the file so we know jQuery is loaded, etc.
• Targets links only in main body of page so selector is more performant.
• Also, only calls function if the nav is sticky.

Nice way to kick things off @Fencer04! I still feel like this needs more work. It doesn't work on all cases for me. I'll try to make a screencast tomorrow. But encourage people to test this patch and iterate on it. @afercia Feel free to test too!

[Fencer04]

@davidakennedy this appears to be working for me, what circumstances caused issues on your end?

[afercia]

Tested a bit and the current patch works only if the navigation has the site-navigation-fixed CSS class on DOM ready, which is true only when the page is already scrolled and then refreshed.
The check for fixed nav should happen on every focus event so it should moved inside the anonymous function. Also, not sure there's the need for e.preventDefault();.

[Fencer04]

@afercia That makes sense, I'll review the current patch and submit an update by Monday afternoon (EST).

[Fencer04]

@afercia I tested this in IE11 and the latest version of Chrome and don't seem to have the issue you described. After I visit a page and the fixed nav isn't in place if I scroll down and then tab to items that are hidden by the nav I am brought to them.

Can someone else test to make sure I'm not missing something?

[afercia]

@Fencer04 what I see in 38476.2.patch​ is:

// Fires on document ready
$( document ).ready( function() {
	...

	if ( $navigation.hasClass( 'site-navigation-fixed' ) ) {
		$( '#content a' ).focus( function( e ) {
			etc...

so on DOM ready, the top navigation doesn't have a CSS class site-navigation-fixed yet, thus the focus event doesn't get attached.

[Fencer04]

Update to bring the code back to the anonymous function and check for the fixed navigation class on each focus.

[Fencer04]

@afercia Sorry for the confusion. My old patch code was still in place while I was testing the new code. After correcting that I saw the issue you described. I uploaded a new patch that moves the code back to anonymous and checks for the fixed navigation class on each focus change.

One question: I included the changes to functions.php from patch number 2 in my patch. Is that the correct procedure or should you only include the changes made in this version?

[davidakennedy]

Account for Admin bar and footer links.

[davidakennedy]

Use get_theme_file_uri() for enqueueing.

[davidakennedy]

With 38476.5.patch​, I've accounted for the admin bar and used get_theme_file_uri() just in case the script needs to be overridden.

[karmatosed]

In 39183:

Twenty Seventeen: Fixes focused controls hidden by top menu.

When a page is scrolled, the top nav menu became fixed. This resolves that.

Props afercia, Fencer04, davidakennedy
Fixes #38476

[afercia]

Sorry for reopening but I'd like to hear more feedback on this and have some more testing. I've tested ;) but more eyes would be appreciated. I'd recommend to test with Firefox (because Chrome tends to scroll to the middle of the page).

I think the 600ms interval should be way shorter, maybe 0. The thing is, when you keep the tab key pressed, and thus focus will move very quickly through controls, the interval is too long and will cause the page to "jump". That's because the browser has already moved focus on the following controls but the JS scrolling is still running.

The goal here shouldn't be to have a nice "smooth scrolling" effect, instead the fix should try to emulate as much as possible the native browsers behaviour.

Also, worth considering this works just for links and ignores form fields, buttons, or other focusable controls that may be hidden under the top nav. To test, just put the search widget in your sidebar. Wondering if the fix should work for any focusable control.

[Fencer04]

@afercia I had originally used * as the selector for the focus. Do you think changing it to "#content *, #colophon *" would do the trick? This was anything that can get focus would trigger the code?

[afercia]

@Fencer04 to be honest I'm more concerned about the 600 ms interval :) that should really be reduced to 0. About the elements to target, maybe I'd consider to add all form controls but I'd defer the decision to all the ones who know Twenty Seventeen better than me :)

[davidakennedy]

Expand possible controls that can receive focus and remove smooth scrolling.

[davidakennedy]

@afercia Great points! I didn't think of those. :( But I'm totally for removing the smooth scrolling and expanding the selectors. Please see my latest patch and give it a test.

[afercia]

@davidakennedy I'd consider to:

• target only what's inside the #content, the footer can probably be left out
• filter the visible elements (e.g. to exclude hidden inputs)
• the asterisk on tabindex and contenteditable seems redundant to me
• performance: any suggestions to improve very welcome

Something like this maybe:

$( '#content' ).find( terrible pile of selectors here ).filter( ':visible' )

[davidakennedy]

Improve selector list for performance.

[davidakennedy]

Thanks @afercia for the feedback! In 38476.7.patch​, I've made some changes to narrow the selectors. Do we need to filter for hidden elements? I thought they couldn't receive focus already? Also, .filter can be a performance hog, so I'm hesitant to use it.

this appears to be working for me, what circumstances caused issues on your end?

@Fencer04 I realized I didn't answer your question from above. It was not accounting for the admin bar height. So it worked, just not as well when logged in. Thanks for your continued work on this! It's been super helpful.

[afercia]

Replying to davidakennedy:

Do we need to filter for hidden elements? I thought they couldn't receive focus already?

They would have the focus event attached anyways, thinking it would be better to avoid that. And yes, they are out of the tab order, but they can receive focus programmatically.

Also, .filter can be a performance hog, so I'm hesitant to use it.

Yep I know 😔

[davidakennedy]

Added .filter to filter visible items.

[davidakennedy]

@afercia In 38476.8.patch​, I added :filter.

[davidakennedy]

Add context selector for better performance.

[davidakennedy]

In 38476.9.patch​, I added context to the selector for better performance based on some feedback:

​https://wordpress.slack.com/archives/accessibility/p1479133361000403

[karmatosed]

In 39225:

Twenty Seventeen: Resolves focused controls hidden by the top menu.

When a page is scrolled, the top nav menu become fixed. Depending on the page content this caused issue where focused links or buttons may be hidden behind the nav menu.

Props afercia, davidakennedy, Fencer04
Fixes #38476