Skip links are not the first focused element on internal navigation

[aitchiss]

Describe the bug

Under #1153 we added skip links to all pages in the app. A skip link should be the first element focused on a page when the user presses the Tab key - however, this is only the case in a Forem if you land directly on a URL, and not if you navigate internally, e.g. by a link click.

There are several strategies available to managing focus on route change, but I would suggested the approach Marcy Sutton has written about following user testing with Fable: What we learned from user testing of accessible client-side routing techniques with Fable Tech Labs

The recommendation in that article is to send focus immediately to the skip link on the new page. We will need to experiment with :focus-visible to ensure that for users who navigated by mouse click, the skip link isn't visible. If this isn't possible, I think an acceptable fallback would be to focus an empty element directly before the skip link, so that once on the new page, the skip link is the first item focused on Tab press.

To Reproduce

• Load the home page, and press Tab. You should see the skip link appear
• Activate the skip link by clicking or pressing Enter
• Select a post using the Tab key and press enter to view it
• On the new page, press Tab again
• Notice that the skip link is not the first item focused, and if you want to get to the main post content you have to press Tab multiple more times

Expected behavior

• I select an article by keyboard on the home page
• On the new page the skip link is visible and focused
• If I repeat these actions but use the mouse, the skip link is not visible when I arrive on the new page

[github-actions]

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

[Link2Twenty]

I think the easiest solution, though I'm sure you've already thought of this, is to modify the header to be a focusable element.

forem/app/views/layouts/_top_bar.html.erb

Line 2 in 90da635

<header class="crayons-header print-hidden">

<header tabindex="-1" class="crayons-header crayons-focus-reset print-hidden">
   content...
</header>

.crayons-focus-reset:focus {
  outline: none;
}

Then when you load in a new page you set the focus to .crayons-focus-reset with something like

document.querySelector('.crayons-focus-reset').focus();

[aitchiss]

I'm keen to run with the results from the research linked above, and focus the skip link after the route change, but I agree if there proves to be any difficulty doing that, focusing the header could be the next best thing 👍

[metamoni]

Hello! 👋

I'd like to work on this, if that's alright.

[nickytonline]

Sounds good @metamoni! Assigning this to you.

[aitchiss]

@metamoni yes!! excited to have you here 😄 ✨

[metamoni]

@aitchiss I've played with quite a bit this over the weekend and here's what I've done:

• set the focus to the Skip Link in the _top_bar partial:

 document.querySelector('.skip-content-link').focus()

• updated the styles in header.scss to use :focus-visible (replacing the :focus and :focus-within selectors):

  &:focus:not(:focus-visible) {
    visibility: hidden;
  }
  
  &:focus-visible {
    pointer-events: auto;
    transform: translate(-50%, 0);
    outline: none;
    border: 2px solid var(--header-button-focus-color);
  }

Here's what that looks like:
https://user-images.githubusercontent.com/22390758/118394256-7e1baa00-b63b-11eb-9a6e-22e0de5b5990.mov

It mostly works the way you wanted, except the Skip Link gets focused automatically when the page is loaded/reloaded. Any idea how to handle this from here?

The way I see it, to fix this we'd probably need something in place for when the window loads, i.e. focus on an empty element (or the header) and only go to the Skip Link when the user presses Tab. Which would bring us back to the solution proposed by @Link2Twenty. Let me know what you think.

[aitchiss]

@metamoni I'm wondering if we could get around this by adding your code to base.js.erb's changePage function, instead of inside the _top_bar partial? That function doesn't get triggered on a fresh page load so potentially we could get the behaviour where:

• When you first arrive on a Forem (by actually entering a URL in the browser, page reload, etc), the body behaves as 'normal', with the skip link being the first focused on tab press
• When you navigate internally, we focus the skip link like the user research suggests

The only thing I'm wondering is if this will play nicely with browser back press navigation 🙃

[metamoni]

I'll investigate 🕵️‍♀️

[nickytonline]

@metamoni I'm wondering if we could get around this by adding your code to base.js.erb's changePage function, instead of inside the _top_bar partial? That function doesn't get triggered on a fresh page load so potentially we could get the behaviour where:

• When you first arrive on a Forem (by actually entering a URL in the browser, page reload, etc), the body behaves as 'normal', with the skip link being the first focused on tab press

• When you navigate internally, we focus the skip link like the user research suggests

The only thing I'm wondering is if this will play nicely with browser back press navigation 🙃

@aitchiss, the changePage should work, but like you mention, I'm not sure that would work for the browser back button, but wouldn't that be a problem in general?

[aitchiss]

@aitchiss, the changePage should work, but like you mention, I'm not sure that would work for the browser back button, but wouldn't that be a problem in general?

Yeah you're right - it just occurred to me thinking about changePage 😄 I guess what we are trying to fix at the moment under this issue is internal navigation, clicking on links through the app. We probably don't need to complicate things by thinking about browser back navigation right now.

[metamoni]

Your suggestion worked, @aitchiss. Finally got around to opening a PR for this. Thank you for your help and patience 🙂

[aitchiss]

Closed via #13828 ❤️