Accessible Routing #19290

madalynrose · 2019-11-05T22:35:44Z

Description

MVP for announcing route changes in Gatsby

Relies on live announcer region
Grabs new location page name from either: first h1, document.title, hardcoded to "new page"

Tested and works with:

Chrome + VoiceOver
NVDA + IE11
Narrator + Edge
Safari + VoiceOver
NVDA + Firefox ~~(bug where it announces multiple times)~~
Safari + VoiceOver (iOS)
Chrome + TalkBack (Android) announces twice reliably (not sure if this is fixable)

TODO:

add localization for announced string
create place for configuration (e.g. element id for page name, assertiveness, element id for announcer) to live

pieh · 2019-11-05T23:19:16Z

create place for configuration (e.g. element id for page name, assertiveness, element id for announcer) to live

I wonder if instead of "element id for page name", we could design API bit differently so there is less disconnect when users work on their pages.

Example idea that I'm not sure if technically could work - what if user could use <Announce> component from gatsby to mark what should be announced. Example of page/template:

import React from "react"
import { Announce } from "gatsby"

export default () => (
  <>
    <Announce>
      <h1>Title</h1>
      <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc faucibus eleifend libero in sollicitudin. Nulla facilisi. Pellentesque ultricies ultricies mauris. Etiam commodo tortor bibendum tortor varius mattis. Fusce sollicitudin rhoncus metus, ac vehicula elit hendrerit vel. Cras aliquet, mauris et maximus mollis, nulla est iaculis massa, vitae viverra nulla nunc et dolor. Donec fermentum arcu eu quam viverra malesuada. Suspendisse potenti. Suspendisse fringilla eget mauris facilisis egestas. Duis erat risus, dictum vitae mi nec, scelerisque tristique velit. Ut sed metus est.</p>
    </Announce>
    <p>Maecenas lacinia justo purus, nec semper nunc cursus at. Vivamus pellentesque nec risus vel varius. Fusce hendrerit, dolor non dapibus feugiat, urna justo bibendum leo, ut facilisis velit ipsum et felis. Maecenas fringilla vehicula vestibulum. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Donec volutpat accumsan sem ut molestie. Sed porta tristique est, sit amet posuere nibh mollis id. Donec orci sem, efficitur a enim ut, suscipit vehicula arcu. Vestibulum hendrerit justo eu accumsan semper. Curabitur viverra ipsum sit amet dui auctor, ut posuere orci pharetra. Maecenas sit amet placerat urna, in fringilla dui. Nunc fermentum ornare velit, vitae tempor libero aliquam id.</p>
  </>
)

Which would announce everything within our <Announce> component when navigated to this page?

madalynrose · 2019-11-05T23:40:35Z

I wonder if instead of "element id for page name", we could design API bit differently so there is less disconnect when users work on their pages.

Example idea that I'm not sure if technically could work - what if user could use <Announce> component from gatsby to mark what should be announced. Example of page/template:

Which would announce everything within our <Announce> component when navigated to this page?

This is an interesting idea! I worry that adding a component might make the it more cumbersome for developers and also might make it too easy to include too much content and overwhelm screen reader users. Definitely something to think more on.

muescha · 2019-11-06T04:26:46Z

packages/gatsby/cache-dir/production-app.js

            </RouteUpdates>
          )}
        </EnsureResources>
      )
    }
  }

+  const RouteAnnouncer = () => {
+    const [announcement, setAnnouncement] = useState(``)
+    useEffect(() => {


should useEffect run on every update or it can be only for example when location is changed?

I think ultimately we would need to use old lifecycle methods anyway (and not hooks) because we declare support for react@^16.4.2 (which is few versions before hooks were introduced):

gatsby/packages/gatsby/package.json

Lines 183 to 186 in 5d78258

"peerDependencies": {

"react": "^16.4.2",

"react-dom": "^16.4.2"

},

That's why we don't use any hooks in gatsby runtime right now (which is a bummer).

While this is in work in progress, I don't want to focus on implementation too much yet and first focus on high level ideas/questions:

use live-region to do announcements

use short content for page change announcements (title, h1, etc) instead of full new page content

how users will be able to customize the behaviour (and what should they be able to customize)

what depends to push react dependency to current react version?

muescha · 2019-11-06T04:37:55Z

Should the name also get it from some aria-label first if they exists on the given elements?

KyleAMathews · 2019-11-06T08:41:37Z

I like the idea of a built-in component as that'd give fine-grained control to those that want it. @madalynrose we could constrain what people pass in so they can't add too long of messages e.g. < 200 characters or whatever.

KyleAMathews · 2019-11-06T08:44:49Z

This is exciting!

Only question I had is will this work with react-helmet as that only sets new titles after the initial render of the new page so it could get tricky coordinating with it.

packages/gatsby/cache-dir/production-app.js

vtenfys · 2019-11-06T10:32:14Z

packages/gatsby/cache-dir/root.js

@@ -52,15 +52,19 @@ const RouteHandler = props => (
  </BaseContext.Provider>
 )

+let locationListener


What is the purpose of this? I can't see how it's used at the moment

vtenfys · 2019-11-06T10:40:42Z

packages/gatsby/cache-dir/navigation.js

-const onRouteUpdate = (location, prevLocation) => {
+const onRouteUpdate = (location, prevLocation, announceLocation) => {
  if (!maybeRedirect(location.pathname)) {
    apiRunner(`onRouteUpdate`, { location, prevLocation })
-
+    if (announceLocation) {
+      announceLocation()
+    }


Should this logic be moved to the RouteUpdates component?

My initial thought was that it may be easier to maintain if moved there, since its props will always update when the location changes, meaning if we use this API in other places then we wouldn't have to duplicate any logic

Though on second thoughts, it looks like the onRouteUpdate API is only called from the RouteUpdates component, so maybe it wouldn't make a difference either way?

vtenfys · 2019-11-06T10:42:14Z

This is awesome!! Left some comments + I second @KyleAMathews' question about React Helmet

pieh · 2019-11-06T12:41:35Z

This is an interesting idea! I worry that adding a component might make the it more cumbersome for developers

We would still fallback to document.title or h1 if the component is not there. We also potentially could just support both id and dedicated component (tho supporting two APIs would probably be cumbersome, not to mention that potentially confusing to developers). In any case this is just suggestion, I don't have strong opinion on API design for this

and also might make it too easy to include too much content and overwhelm screen reader users.

This is also true if we would look for specific id in new page to customize what is announced. People could replace <Announce> with <div id="announce-area"> to get same result. Benefit of using dedicated component is that it's easier to implement some validation (like warning users if there is too much content to be announced)

Only question I had is will this work with react-helmet as that only sets new titles after the initial render of the new page so it could get tricky coordinating with it.

I think the additional requestAnimationFrame there is exactly because of react-helmet doing "delayed" update, so it should work with react-helmet as-is.

I was also wondering on how to best provide way for customization (other than pages defining what should be announced) and maybe we could package all of this as a plugin (as we rely on onRouteUpdate anyway, so plugin can do that as well), include that plugin by default and allow users to override plugin options (so very similar to what we do for gatsby-plugin-page-creator):

https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/bootstrap/load-plugins/load.js#L216-L240

Additional benefit is that users could also just add this plugin to older Gatsby version if they don't want to (or can't) update gatsby core version (I imagine that's a stretch here, and we probably won't see this happening)

Lastly - we should release canary version of gatsby with it, as well as publish demo site using those changes for people to try out

Co-Authored-By: David Bailey <4248177+davidbailey00@users.noreply.github.com>

muescha · 2019-11-06T21:22:30Z

~~Before it is implemented we should ask impaired people what they most expect from this addition (to avoid they get annoyed by to much and unnecessary text)~~

~~maybe they can help us again: https://www.gatsbyjs.org/blog/2019-07-11-user-testing-accessible-client-routing/~~

I see this PR try to fix this:

In Gatsby–which uses React.js for rendering–we’re currently setting focus to an element wrapping the entire application with a custom implementation of @reach/router. But similar to some of the Reach demos, page content isn’t announced consistently in Safari and Voiceover or NVDA and Firefox–two critical combinations for users of assistive technology. We’re actively working to improve this automatic accessibility support as we explore more component- and research-driven solutions.

…-mvp

…f hooks, use pathName instead of vague string as default announcement

…1y-routing-mvp

freiksenet · 2020-01-08T07:38:15Z

packages/gatsby/cache-dir/production-app.js

@@ -30,6 +30,7 @@ window.___loader = publicLoader
 navigationInit()

 apiRunnerAsync(`onClientEntry`).then(() => {
+  console.log(`HELLO`)


Stray console log.

Why HELLO to you too

freiksenet · 2020-01-17T08:51:14Z

packages/gatsby/cache-dir/production-app.js

@@ -94,6 +94,69 @@ apiRunnerAsync(`onClientEntry`).then(() => {
    }
  }

+  // class RouteAnnouncer extends React.Component {


I think this can be removed now.

good catch!

freiksenet

I think this is ready to get shipped!

blainekasten · 2020-01-17T15:28:14Z

packages/gatsby/cache-dir/navigation.js

+      ></div>
+    )
+  }
+}


So this component doesnt render any dynamic data, I wonder if we could add a

shouldComponentUpdate() { return false; }

is still getting used to using refs like this

does this.announcementRef.current.innerText not count as dynamic data being rendered?

nope. On initial render, the ref will be passed to React, who will attach it to the node. It happens asynchronous and does not affect component lifecycles.

Changing the ref will not cause any re-renders ever!

Okay I'm trying to leverage my new-ness to React as something useful and ask my potentially trivial questions out in the open 🙈

Our changes happen to the announcement ref happen within the componentDidUpdate lifecycle method. Wouldn't returning false inside shouldComponentUpdate prevent it from being invoked?

Great questions! And you know what, I'm wrong. 😆 Since you are doing it in DidUpdate, that occurs after a render. So stopping update stops that cycle. Let's just ship this, I think there are some changes that would better align with React, but this works. So let's ship and we can see if there are better iterations later :)

blainekasten · 2020-01-17T15:41:03Z

packages/gatsby/cache-dir/navigation.js

+  constructor(props) {
+    super(props)
+    this.announcementRef = React.createRef()
+  }


ES6 tip (feel free to do this if you want). But you can ditch the constructor here so we're just instantiating a class variable. Do it like this instead:

class Router { announcementRef = React.createRef() }

It'll achieve the same result! (if this doesn't work ignore, it does require a specific babel setup to make it work and maybe we don't have that)

I think I'm going to leave this for a later iteration. I'm ready to get this thing OUT!

blainekasten · 2020-01-17T15:55:29Z

packages/gatsby/cache-dir/navigation.js

+        pageName = pageHeadings[0].textContent
+      }
+      const newAnnouncement = `Navigated to ${pageName}`
+      const oldAnnouncement = this.announcementRef.current.innerText


Ah, one thing I just thought of. We should add a bailout check if the ref isn't attached yet. It feels unlikely to occur, but technically ref's type is {current: HTMLElement | null}. It's possible this could come up later on with suspense. All you need to do is at the top of this rAF, put a if (!this.announcementRef.current) return

@blainekasten Suspense won’t change anything here.

It’s impossible for the ref to be null during componentDidUpdate, though note that because of the rAF call it is possible (though probably unlikely) for the component to unmount before this code runs, so a check is appropriate.

@blainekasten I think this check is still missing in the code. Just talked to someone on Discord getting Cannot read property 'innerText' of null on this line. Don't know if that's the cause of his problem, or a side-effect.

I'm getting this error while trying to debug problems with Ms Edge (14). Happens when I try to navigate to a new page.

blainekasten

This is awesome! I'm in approval!!

wKovacs64 · 2020-01-18T04:05:00Z

Ironically, this is causing Gatsby to no longer pass a11y tests (via cypress-axe) in one of my projects:

Should it be wrapped in something or go elsewhere, perhaps?

madalynrose · 2020-01-18T04:19:41Z

Thanks for letting us know, @wKovacs64! Could you create an issue and add more about your setup (or point to your repo)? I'll dive into this first thing on Tuesday.

JustFly1984 · 2020-01-18T04:57:16Z

packages/gatsby/cache-dir/navigation.js

+    return (
+      <div
+        id="gatsby-announcer"
+        style={{


please extract object to const outside of class scope, or React.useMemo hook

please add eslint-plugin-react-perf, to escape this kind of performance implications.

this is a very minor perf issue. Creating one object on re-renders (which only happen on route change). It's probably easier to just implement a shouldComponentUpdate: () => false

Easier - does not mean best way. Counting that react moving to functions, instead of components, and original advise do not use shouldComponentUpdate, this style object already a symptom that there could be much more performance issues in gatsby.js project itself. A lot of very minor issues adds up pretty darn fast

Former React team manager here – I can guarantee this will never be a performance problem and there is nothing wrong with writing the style object like this. I’d recommend you stop worrying about microoptimizing code.

JustFly1984 · 2020-01-18T04:58:02Z

packages/gatsby/cache-dir/navigation.js

+      if (pageHeadings && pageHeadings.length) {
+        pageName = pageHeadings[0].textContent
+      }
+      const newAnnouncement = `Navigated to ${pageName}`


how about internationalization support?

wKovacs64 · 2020-01-18T16:58:33Z

Thanks for letting us know, @wKovacs64! Could you create an issue and add more about your setup (or point to your repo)? I'll dive into this first thing on Tuesday.

Opened #20697, thanks.

luukdv · 2020-01-19T09:51:25Z

Just noticed the #gatsby-announcer element causes a vertical scrollbar on 100vh pages, even when empty. Looks like a default styling of height: 1px; causes the issue, which is present when the element doesn't have content.

Any way to turn this off for apps that don't use routing? Or not render/apply those styles when the element is empty?

madalynrose · 2020-01-19T20:38:12Z

@luukdv Thanks for letting us know! Could you open an issue for this and add more about the browser/platform you're seeing this on, possibly with a link to a page where you can reproduce the issue? I'll be focusing on bug fixes first thing on Tuesday.

@marcysutton

# Route Announcements ## Summary This PR improves the accessibility of NextJS's client-side navigation by announcing route changes to screen readers. ## Context When a user who is sighted clicks on a link, they can see the content change. It's an affirmation that what the user intended to do by clicking a link actually worked! Users navigating the page via a screen-reader will not get this feedback on NextJS sites (This is an issue on many SPA-like architectures). https://user-images.githubusercontent.com/4213649/103017382-63b02b00-44f8-11eb-9940-fb530d2d3018.mov ## Solution Whenever there is a route change, the new `<RouteAnnouncer />` will look for a name to give the new page and then announce it! The name is found by first looking for an `h1`, falling back to `document.title`, and lastly to `pathname`. `<RouteAnnouncer />` is a visually hidden component placed within the `<AppContainer />`. ## Demo https://user-images.githubusercontent.com/4213649/103017401-6ad73900-44f8-11eb-8050-b3e9a7e0c3f2.mov ## Inspiration First and foremost, this PR was inspired by @marcysutton's studies and writing, [What we learned from user testing of accessible client-side routing techniques with Fable Tech Labs ](https://www.gatsbyjs.com/blog/2019-07-11-user-testing-accessible-client-routing/) as well as @madalynrose's [Accessible Routing](gatsbyjs/gatsby#19290) PR for Gatsby. There were also learnings gleaned from the conversations within #7681. ### Related Issues & PRs - Resolves #7681 - Relates to #19963

add announcer live region for routing

164eeba

madalynrose requested review from pieh and marcysutton November 5, 2019 22:37

muescha reviewed Nov 6, 2019

View reviewed changes

vtenfys reviewed Nov 6, 2019

View reviewed changes

packages/gatsby/cache-dir/production-app.js Outdated Show resolved Hide resolved

vtenfys reviewed Nov 6, 2019

View reviewed changes

Update packages/gatsby/cache-dir/production-app.js

1b8221f

Co-Authored-By: David Bailey <4248177+davidbailey00@users.noreply.github.com>

m-allanson mentioned this pull request Nov 21, 2019

a11y issues: page nav doesn't trigger assistive tech #5581

Closed

eps1lon mentioned this pull request Nov 21, 2019

Consider deprecating all special live region roles except alert w3c/aria#1104

Open

madalynrose added 5 commits December 11, 2019 15:19

Merge branch 'master' of github.com:gatsbyjs/gatsby into a11y-routing…

c525e4a

…-mvp

merged in master

10024e1

refactor announcement updates to happen in lifecycle method instead o…

752682e

…f hooks, use pathName instead of vague string as default announcement

remove unused imports

6f52df1

Merge branch 'a11y-routing-mvp' of github.com:gatsbyjs/gatsby into a1…

81cfa5a

…1y-routing-mvp

m-allanson assigned madalynrose, freiksenet and blainekasten Jan 7, 2020

madalynrose added 2 commits January 7, 2020 14:08

move RouteAnnouncer inside of RouteUpdates in navigation.js

842830f

remove console.logs

810b9e1

freiksenet reviewed Jan 8, 2020

View reviewed changes

freiksenet marked this pull request as ready for review January 17, 2020 08:50

freiksenet requested a review from a team as a code owner January 17, 2020 08:50

freiksenet reviewed Jan 17, 2020

View reviewed changes

remove commented out code

ba53b68

blainekasten reviewed Jan 17, 2020

View reviewed changes

add accidentally deleted code back in, remove changes in root.js

5b15471

blainekasten reviewed Jan 17, 2020

View reviewed changes

blainekasten approved these changes Jan 17, 2020

View reviewed changes

madalynrose merged commit 701222f into master Jan 17, 2020

madalynrose deleted the a11y-routing-mvp branch January 17, 2020 18:36

JustFly1984 reviewed Jan 18, 2020

View reviewed changes

wKovacs64 mentioned this pull request Jan 18, 2020

[2.18.25] Accessible Routing triggers region a11y violation in axe #20697

Closed

hidde mentioned this pull request Jan 21, 2020

Reset focus after navigation EmilTholin/svelte-routing#25

Open

madalynrose mentioned this pull request Jan 21, 2020

ARIA live region triggers region rule violation dequelabs/axe-core#1992

Closed

muescha mentioned this pull request Jan 21, 2020

fix(chore): extract style to const #20754

Closed

This was referenced Jan 22, 2020

Localization for client-side route announcements #20801

Closed

Blog 2020-02-05 Accessible Routing #21018

Merged

madalynrose mentioned this pull request Jan 30, 2020

Add additional configuration options for client-side routing announcements #21059

Closed

madalynrose mentioned this pull request Aug 11, 2020

Gatsby a11y helpers #26376

Closed

3 tasks

kyleboss mentioned this pull request Dec 23, 2020

[a11y] Route Announcements vercel/next.js#20428

Merged

	"peerDependencies": {
	"react": "^16.4.2",
	"react-dom": "^16.4.2"
	},

Accessible Routing #19290

Accessible Routing #19290

Conversation

madalynrose commented Nov 5, 2019 • edited Loading

Description

Tested and works with:

pieh commented Nov 5, 2019 • edited Loading

madalynrose commented Nov 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muescha commented Nov 6, 2019

KyleAMathews commented Nov 6, 2019

KyleAMathews commented Nov 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vtenfys commented Nov 6, 2019

pieh commented Nov 6, 2019

muescha commented Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freiksenet left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madalynrose Jan 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blainekasten left a comment

Choose a reason for hiding this comment

wKovacs64 commented Jan 18, 2020

madalynrose commented Jan 18, 2020 • edited Loading

JustFly1984 Jan 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wKovacs64 commented Jan 18, 2020

luukdv commented Jan 19, 2020

madalynrose commented Jan 19, 2020

madalynrose commented Nov 5, 2019 •

edited

Loading

pieh commented Nov 5, 2019 •

edited

Loading

muescha commented Nov 6, 2019 •

edited

Loading

freiksenet left a comment •

edited

Loading

madalynrose Jan 17, 2020 •

edited

Loading

madalynrose commented Jan 18, 2020 •

edited

Loading

JustFly1984 Jan 18, 2020 •

edited

Loading