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

[a11y] Route Announcements #20428

Merged
merged 31 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
eafdb89
Add two client-navigation accessibility improvements:
kyleboss Dec 23, 2020
323a952
Merge branch 'canary' into client-nav-a11y
kyleboss Dec 23, 2020
e22817b
Merge branch 'canary' into client-nav-a11y
kyleboss Dec 28, 2020
9a48bca
Merge branch 'canary' into client-nav-a11y
kyleboss Dec 28, 2020
333ab3d
Merge branch 'canary' into client-nav-a11y
kyleboss Dec 28, 2020
7156183
Merge branch 'canary' into client-nav-a11y
kyleboss Dec 28, 2020
991cd67
Merge branch 'canary' of git://github.com/vercel/next.js into client-…
kyleboss Dec 28, 2020
ac60dcc
Merge branch 'canary' of git://github.com/vercel/next.js into client-…
kyleboss Dec 30, 2020
1c4bf60
Merge remote-tracking branch 'upstream/canary' into client-nav-a11y
Timer Jan 21, 2021
8cc9a83
Fix hydration mismatch
Timer Jan 21, 2021
c15507b
Relocate focus handling to router
Timer Jan 21, 2021
5c2dbe1
tweak build output numbers
Timer Jan 21, 2021
0080959
Update visually hidden styles
Timer Jan 21, 2021
8adf039
Fix import
Timer Jan 21, 2021
845063c
Merge branch 'canary' into client-nav-a11y
kyleboss Mar 9, 2021
9db8424
Remove logic for focusing on main or h1 if one exists. It's
kyleboss Mar 9, 2021
39ccbde
Apply suggestions from code review
shuding Mar 10, 2021
54dcfa3
simplify portal; fix the case where h1 is empty
shuding Mar 10, 2021
85ef996
do not announce on initial load
shuding Mar 10, 2021
b437b2e
Remove focus improvements. We will add these in a future PR.
kyleboss Mar 14, 2021
8884236
Merge branch 'client-nav-a11y' of https://github.com/kyleboss/next.js…
kyleboss Mar 14, 2021
902426a
I do not know why this was deleted?
kyleboss Mar 14, 2021
5293465
Trying to bring router back to the way it was?
kyleboss Mar 14, 2021
2b15725
Another attempt at bringing router back to normal
kyleboss Mar 14, 2021
8c28dd9
Merge remote-tracking branch 'upstream/canary' into client-nav-a11y
kyleboss Mar 14, 2021
0ce69dc
Another attempt at fixing router
kyleboss Mar 14, 2021
596ae18
fix tests
shuding Mar 15, 2021
9b21d10
fix tests
shuding Mar 15, 2021
54b5721
fix tests
shuding Mar 15, 2021
b70aa27
fix test
shuding Mar 15, 2021
8fc903e
Merge branch 'canary' into client-nav-a11y
kodiakhq[bot] Mar 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/next/client/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import {
NEXT_DATA,
ST,
} from '../next-server/lib/utils'
import { Portal } from './portal'
import initHeadManager from './head-manager'
import PageLoader, { StyleSheetTuple } from './page-loader'
import measureWebVitals from './performance-relayer'
import { RouteAnnouncer } from './route-announcer'
import { createRouter, makePublicRouterInstance } from './router'

/// <reference types="react-dom/experimental" />
Expand Down Expand Up @@ -784,6 +786,9 @@ function doRender(input: RenderRouteInfo): Promise<any> {
<Head callback={onHeadCommit} />
<AppContainer>
<App {...appProps} />
<Portal type="next-route-announcer">
<RouteAnnouncer />
</Portal>
</AppContainer>
</Root>
)
Expand Down
6 changes: 0 additions & 6 deletions packages/next/client/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,6 @@ function linkClicked(
shallow,
locale,
scroll,
}).then((success: boolean) => {
if (!success) return
if (scroll) {
// FIXME: proper route announcing at Router level, not Link:
document.body.focus()
}
})
}

Expand Down
9 changes: 9 additions & 0 deletions packages/next/client/portal/LICENSE
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.
24 changes: 24 additions & 0 deletions packages/next/client/portal/index.tsx
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
}
67 changes: 67 additions & 0 deletions packages/next/client/route-announcer.tsx
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
8 changes: 4 additions & 4 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,17 @@ describe('Build Output', () => {
expect(parseFloat(indexSize) - 266).toBeLessThanOrEqual(0)
expect(indexSize.endsWith('B')).toBe(true)

// should be no bigger than 63.9 kb
expect(parseFloat(indexFirstLoad)).toBeCloseTo(64.1, 1)
// should be no bigger than 64.6 kb
expect(parseFloat(indexFirstLoad)).toBeCloseTo(64.6, 1)
expect(indexFirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(err404Size) - 3.7).toBeLessThanOrEqual(0)
expect(err404Size.endsWith('kB')).toBe(true)

expect(parseFloat(err404FirstLoad)).toBeCloseTo(67.1, 0)
expect(parseFloat(err404FirstLoad)).toBeCloseTo(67.8, 0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll)).toBeCloseTo(63.9, 1)
expect(parseFloat(sharedByAll)).toBeCloseTo(64.4, 1)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down
1 change: 1 addition & 0 deletions test/integration/client-navigation-a11y/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {}
19 changes: 19 additions & 0 deletions test/integration/client-navigation-a11y/pages/index.js
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 test/integration/client-navigation-a11y/pages/page-with-h1.js
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 test/integration/client-navigation-a11y/pages/page-with-title.js
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>
)
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 test/integration/client-navigation-a11y/test/index.test.js
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()
})
})
})
})
2 changes: 1 addition & 1 deletion test/integration/fallback-modules/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('Build Output', () => {
expect(parseFloat(indexSize)).toBeGreaterThanOrEqual(2)
expect(indexSize.endsWith('kB')).toBe(true)

expect(parseFloat(indexFirstLoad)).toBeLessThanOrEqual(67.5)
expect(parseFloat(indexFirstLoad)).toBeLessThanOrEqual(67.8)
expect(parseFloat(indexFirstLoad)).toBeGreaterThanOrEqual(60)
expect(indexFirstLoad.endsWith('kB')).toBe(true)
})
Expand Down
2 changes: 1 addition & 1 deletion test/integration/size-limit/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ describe('Production response size', () => {
const delta = responseSizesBytes / 1024

// Expected difference: < 0.5
expect(delta).toBeCloseTo(285.3, 0)
expect(delta).toBeCloseTo(286.7, 0)
})
})