-
Notifications
You must be signed in to change notification settings - Fork 27.1k
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
[a11y] Route Announcements #23086
Merged
Merged
[a11y] Route Announcements #23086
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
The MIT License (MIT) | ||
|
||
Copyright (c) 2018-present, React Training LLC | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: | ||
|
||
The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import * as React from 'react' | ||
import { createPortal } from 'react-dom' | ||
|
||
type PortalProps = { | ||
children: React.ReactNode | ||
type: string | ||
} | ||
|
||
export const Portal: React.FC<PortalProps> = ({ children, type }) => { | ||
let portalNode = React.useRef<HTMLElement | null>(null) | ||
let [, forceUpdate] = React.useState<{}>() | ||
React.useEffect(() => { | ||
portalNode.current = document.createElement(type) | ||
document.body.appendChild(portalNode.current) | ||
forceUpdate({}) | ||
return () => { | ||
if (portalNode.current) { | ||
document.body.removeChild(portalNode.current) | ||
} | ||
} | ||
}, [type]) | ||
|
||
return portalNode.current ? createPortal(children, portalNode.current) : null | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import React, { useEffect, useState, useRef } from 'react' | ||
import { useRouter } from './router' | ||
|
||
export function RouteAnnouncer() { | ||
const { asPath } = useRouter() | ||
const [routeAnnouncement, setRouteAnnouncement] = useState('') | ||
|
||
// Only announce the path change, but not for the first load because screen reader will do that automatically. | ||
const initialPathLoaded = useRef(false) | ||
|
||
// Every time the path changes, announce the route change. The announcement will be prioritized by h1, then title | ||
// (from metadata), and finally if those don't exist, then the pathName that is in the URL. This methodology is | ||
// inspired by Marcy Sutton's accessible client routing user testing. More information can be found here: | ||
// https://www.gatsbyjs.com/blog/2019-07-11-user-testing-accessible-client-routing/ | ||
useEffect( | ||
() => { | ||
if (!initialPathLoaded.current) { | ||
initialPathLoaded.current = true | ||
return | ||
} | ||
|
||
let newRouteAnnouncement | ||
const pageHeader = document.querySelector('h1') | ||
|
||
if (pageHeader) { | ||
newRouteAnnouncement = pageHeader.innerText || pageHeader.textContent | ||
} | ||
if (!newRouteAnnouncement) { | ||
if (document.title) { | ||
newRouteAnnouncement = document.title | ||
} else { | ||
newRouteAnnouncement = asPath | ||
} | ||
} | ||
|
||
setRouteAnnouncement(newRouteAnnouncement) | ||
}, | ||
// TODO: switch to pathname + query object of dynamic route requirements | ||
[asPath] | ||
) | ||
|
||
return ( | ||
<p | ||
aria-live="assertive" // Make the announcement immediately. | ||
id="__next-route-announcer__" | ||
role="alert" | ||
style={{ | ||
border: 0, | ||
clip: 'rect(0 0 0 0)', | ||
height: '1px', | ||
margin: '-1px', | ||
overflow: 'hidden', | ||
padding: 0, | ||
position: 'absolute', | ||
width: '1px', | ||
|
||
// https://medium.com/@jessebeach/beware-smushed-off-screen-accessible-text-5952a4c2cbfe | ||
whiteSpace: 'nowrap', | ||
wordWrap: 'normal', | ||
}} | ||
> | ||
{routeAnnouncement} | ||
</p> | ||
) | ||
} | ||
|
||
export default RouteAnnouncer |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import Link from 'next/link' | ||
|
||
export default () => ( | ||
<div id="page-container"> | ||
<Link href="/page-with-h1"> | ||
<a id="page-with-h1-link">Go to a page with an h1</a> | ||
</Link> | ||
<Link href="/page-with-title"> | ||
<a id="page-with-title-link"> | ||
Go to a page without an h1, but with a title | ||
</a> | ||
</Link> | ||
<Link href="/page-without-h1-or-title"> | ||
<a id="page-without-h1-or-title-link"> | ||
Go to a page without an h1 or a title | ||
</a> | ||
</Link> | ||
</div> | ||
) |
11 changes: 11 additions & 0 deletions
11
test/integration/client-navigation-a11y/pages/page-with-h1.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import Head from 'next/head' | ||
|
||
export default () => ( | ||
<div id="page-with-h1"> | ||
<Head> | ||
<title>Another Page's Title</title> | ||
</Head> | ||
<h1>My heading</h1> | ||
<div>Extraneous stuff</div> | ||
</div> | ||
) |
11 changes: 11 additions & 0 deletions
11
test/integration/client-navigation-a11y/pages/page-with-title.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import Head from 'next/head' | ||
|
||
export default () => ( | ||
<div id="page-with-title"> | ||
<Head> | ||
<title>Another Page's Title</title> | ||
</Head> | ||
<div>My heading</div> | ||
<div>Extraneous stuff</div> | ||
</div> | ||
) |
6 changes: 6 additions & 0 deletions
6
test/integration/client-navigation-a11y/pages/page-without-h1-or-title.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export default () => ( | ||
<div id="page-without-h1-or-title"> | ||
<div>My heading</div> | ||
<div>Extraneous stuff</div> | ||
</div> | ||
) |
101 changes: 101 additions & 0 deletions
101
test/integration/client-navigation-a11y/test/index.test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/* eslint-env jest */ | ||
|
||
import { | ||
findPort, | ||
killApp, | ||
launchApp, | ||
renderViaHTTP, | ||
} from '../../../lib/next-test-utils' | ||
import webdriver from 'next-webdriver' | ||
import { join } from 'path' | ||
|
||
const context = {} | ||
jest.setTimeout(1000 * 60 * 5) | ||
|
||
describe('Client Navigation accessibility', () => { | ||
beforeAll(async () => { | ||
context.appPort = await findPort() | ||
context.server = await launchApp(join(__dirname, '../'), context.appPort, { | ||
env: { __NEXT_TEST_WITH_DEVTOOL: 1 }, | ||
}) | ||
|
||
const prerender = [ | ||
'/page-with-h1, /page-with-title, /page-without-h1-or-title', | ||
] | ||
|
||
await Promise.all( | ||
prerender.map((route) => renderViaHTTP(context.appPort, route)) | ||
) | ||
}) | ||
|
||
afterAll(() => killApp(context.server)) | ||
|
||
describe('<RouteAnnouncer />', () => { | ||
it('has aria-live="assertive" and role="alert"', async () => { | ||
const browser = await webdriver(context.appPort, '/') | ||
const routeAnnouncer = await browser.waitForElementByCss( | ||
'#__next-route-announcer__' | ||
) | ||
const ariaLiveValue = await routeAnnouncer.getAttribute('aria-live') | ||
const roleValue = await routeAnnouncer.getAttribute('role') | ||
|
||
expect(ariaLiveValue).toBe('assertive') | ||
expect(roleValue).toBe('alert') | ||
await browser.close() | ||
}) | ||
describe('There is an h1 tag', () => { | ||
it('has the same innerText value as the h1 tag', async () => { | ||
const browser = await webdriver(context.appPort, '/') | ||
const h1Value = await browser | ||
.waitForElementByCss('#page-with-h1-link') | ||
.click() | ||
.waitForElementByCss('#page-with-h1') | ||
.elementByCss('h1') | ||
.text() | ||
|
||
const routeAnnouncerValue = await browser | ||
.waitForElementByCss('#__next-route-announcer__') | ||
.text() | ||
|
||
expect(h1Value).toBe(routeAnnouncerValue) | ||
await browser.close() | ||
}) | ||
}) | ||
describe('There is a document.title, but no h1 tag', () => { | ||
it('has the innerText equal to the value of document.title', async () => { | ||
const browser = await webdriver(context.appPort, '/') | ||
await browser | ||
.waitForElementByCss('#page-with-title-link') | ||
.click() | ||
.waitForElementByCss('#page-with-title') | ||
|
||
const title = await browser.eval('document.title') | ||
|
||
const routeAnnouncerValue = await browser | ||
.waitForElementByCss('#__next-route-announcer__') | ||
.text() | ||
|
||
expect(title).toBe(routeAnnouncerValue) | ||
await browser.close() | ||
}) | ||
}) | ||
describe('There is neither an h1 or a title tag', () => { | ||
it('has the innerText equal to the value of the pathname', async () => { | ||
const browser = await webdriver(context.appPort, '/') | ||
await browser | ||
.waitForElementByCss('#page-without-h1-or-title-link') | ||
.click() | ||
.waitForElementByCss('#page-without-h1-or-title') | ||
|
||
const pathname = '/page-without-h1-or-title' | ||
|
||
const routeAnnouncerValue = await browser | ||
.waitForElementByCss('#__next-route-announcer__') | ||
.text() | ||
|
||
expect(pathname).toBe(routeAnnouncerValue) | ||
await browser.close() | ||
}) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking back at it, I think the logic might be a little backwards here.
In my opinion, we should use the
document.title
first if available, since this is what would happen in a non-SPA scenario upon page change. And if we do not have a page title, we should use theh1
(if any, of course). Right now it will announce the content of theh1
, despite having a perfectly cleandocument.title
, which I think is not the best experience.Taking the home page of the Gorillas website as an example: the page title is “Home | Gorillas” while the
h1
is “Groceries delivered in 10 minutes”. Navigating to the home page should announce “Home | Gorillas”, not the baseline (or catch phrase, or whatever we want to name it). And while in many cases, theh1
and thedocument.title
would be in sync, I believe this is not always the case. In a case like this, we also don’t want theh1
to just contain “Home.”Pinging @kyleboss who authored the original PR for opinions. :)
Edit: I’m happy opening an issue/PR if you think this is a valid concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking the liberty to ping @shuding and @olpeh as well for comments. ✨