Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manage focus, and use aria-live to announce navigation #307

Closed
Rich-Harris opened this issue Jan 6, 2021 · 11 comments
Closed

manage focus, and use aria-live to announce navigation #307

Rich-Harris opened this issue Jan 6, 2021 · 11 comments

Comments

@Rich-Harris
Copy link
Member

sveltejs/sapper#1083, basically.

There are two separate issues to consider in order to make client-side routing accessible:

1. Managing focus

In a server-rendered app, any time you navigate to a page, the <body> is focused. In Sapper and SvelteKit apps, we blur the current activeElement upon navigation, but this doesn't actually reset the focus, which is what we need — it just removes it temporarily. As soon as you press the tab key (or shift-tab), focus moves to the element after (or before) whichever element was focused prior to the navigation, assuming it still exists in the DOM. This is not desirable.

Went down a bit of a rabbit hole trying to understand current recommendations:

  • Marcy Sutton advises focusing a 'skip back to navigation' link (i.e. a skip nav link in reverse)
  • Ryan Florence thinks the app developer should have control over where focus lands, but that it should be an element inside the innermost route that has changed
  • Nick Colley and Daniel Nixon prefer finding some primary focus target, e.g. the <h1>
  • David Luhr suggests resetting focus to the top of the DOM

I'm sure there are other suggestions too. Each comes from an accessibility expert and has a solid argument, but ultimately they are mutually exclusive. Any would be better than the status quo though.

My inclination in the short term is to simply focus <body> (i.e. David Luhr's suggestion) — it's easy to implement, and matches the behaviour of server-rendered apps. In the future perhaps we could investigate alternatives, particularly ones that put power in app developers' hands rather than making one-size-fits-all decisions for them.

The short term solution ought to be fairly straightforward — we just change this...

if (document.activeElement instanceof HTMLElement) {
document.activeElement.blur();
}

...to this:

document.body.setAttribute('tabindex', '-1');
document.body.focus();

(Better still, only set the tabindex once, when the router initialises.)

2. Announcing the page change

Per the recommendation in https://www.gatsbyjs.com/blog/2019-07-11-user-testing-accessible-client-routing/, we should add an ARIA live region that announces new pages. That looks something like this:

<div
  id="svelte-announcer"
  class="visually-hidden"
  aria-live="assertive"
  aria-atomic="true"
>Navigated to {title}</div>

This could live inside the auto-generated root.svelte component.

It makes sense for title to simply reflect document.title, I think, rather than introducing some complicated way to customise it, since a page already needs an informational document title for SEO and accessibility purposes.

@Rich-Harris Rich-Harris added this to the public beta milestone Jan 6, 2021
@Rich-Harris
Copy link
Member Author

(Not pictured: a solution to i18n, when constructing the Navigated to {title} text)

Rich-Harris pushed a commit that referenced this issue Jan 7, 2021
* reset focus properly on navigation (#307)

* fix
Rich-Harris added a commit that referenced this issue Jan 7, 2021
Rich-Harris pushed a commit that referenced this issue Jan 7, 2021
* rename tests

* announce page changes (#307)

* try to reduce flakiness
@Rich-Harris
Copy link
Member Author

closed via #309 and #311

@dummdidumm
Copy link
Member

For future consideration (haven't really thought this through): Could this be a component that can be imported via $app/a11y or something? People could then use it in their layout like this:

<script>
   import Announcer from "...";
</script>

<Announcer let:title>
   Hey, I've navigated to {title}
</Announcer>

@Rich-Harris
Copy link
Member Author

I like that solution, it's a good way to make it customisable without configitis. The trade-off is you're much likelier to omit (or duplicate!) it, would need to think about how to avoid that.

$app/a11y could also be useful for things like skip nav links

@dummdidumm
Copy link
Member

On "how to avoid": I guess this is the tradeoff here. To diminish this, we could add it to the template that is bootstrapped when using create-svelte.

@philrenaud
Copy link

Wanted to drop a note: the main application sveltekit app I'm working with uses a full-height, full-width css grid element as its primary "canvas", and the presence of a tabindex on the body removes all keyboard-based scrolling behaviour (arrow keys, PgDn, etc.) that was previously working.

I'm using the following in our _layout.svelte to counteract this for now, but it feels pretty hacky:

  onMount(async () => {
      document.body.removeAttribute('tabindex');

@dummdidumm
Copy link
Member

When using goto there's now an option to not reset focus. The same doesn't exist as a directive for a tags yet though

@philrenaud
Copy link

@dummdidumm I appreciate the option, but we would need this for effectively all routes throughout the app. I wonder if there's some way to do it at a global config level?

@dummdidumm
Copy link
Member

There's not right now, Rich mentions this in the initial post. I think it's best if you open a new feature request, outlining the current problem in detail as well as proposing some solutions, like the config option.

@Badlapje
Copy link

For future consideration (haven't really thought this through): Could this be a component that can be imported via $app/a11y or something? People could then use it in their layout like this:

<script>
   import Announcer from "...";
</script>

<Announcer let:title>
   Hey, I've navigated to {title}
</Announcer>

I would like to second this. This is something which for example Drupal provides as well.

On a general note: I'm very happily surprised at the state of a11y support being provided by Svelte. Keep up the good work!

@Badlapje
Copy link

As an added suggestion: can we add an a11y tag to the issue tags? I'd be happy to help out where i can with anything a11y related. It's easier to search for those issues if there's a tag for it.

Lms24 added a commit to getsentry/sentry-javascript that referenced this issue Aug 18, 2022
Add a SvelteKit detection mechanism to our SDK and inserts a SvelteKit entry into `event.modules` if we detect that the frontend in which the SDK is used, was rendered by SvelteKit.

To check this, we check for the existence of a div with the id `svelte-announcer` that's automatically injected into the rendered HTML by SvelteKit. It is used to improve accessibility (see sveltejs/kit#307) but we can leverage it as a SvelteKit detection criterion. 

Btw, we're not the first ones who came up with this approach, as it turns out: https://twitter.com/jhewt/status/1359632380483493889 

Introduce a new utils function, `getDomElement` in which we consolidate the usage of `document.querySelector` in the SDK. So in addition to using this new function for obtaining the div described above, we now also call it in `BrowserTracing` to get `<meta>` tags.

Add some tests to test the new behaviour and the helper functions. We might want to consider writing an integration test for this feature but this first requires a Svelte SDK integration test infrastructure. 

ref: #5573
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants