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

Give priority to document.title over h1 when announcing page change #31147

Merged
merged 3 commits into from Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 13 additions & 17 deletions packages/next/client/route-announcer.tsx
Expand Up @@ -5,12 +5,15 @@ export function RouteAnnouncer() {
const { asPath } = useRouter()
const [routeAnnouncement, setRouteAnnouncement] = React.useState('')

// Only announce the path change, but not for the first load because screen reader will do that automatically.
// Only announce the path change, but not for the first load because screen
// reader will do that automatically.
const initialPathLoaded = React.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:
// Every time the path changes, announce the new page’s title following this
// priority: first the document title (from head), otherwise the first h1, or
// if none of these exist, then the pathname from 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/
React.useEffect(
() => {
Expand All @@ -19,21 +22,14 @@ export function RouteAnnouncer() {
return
}

let newRouteAnnouncement
const pageHeader = document.querySelector('h1')
if (document.title) {
setRouteAnnouncement(document.title)
} else {
const pageHeader = document.querySelector('h1')
const content = pageHeader?.innerText ?? pageHeader?.textContent

if (pageHeader) {
newRouteAnnouncement = pageHeader.innerText || pageHeader.textContent
setRouteAnnouncement(content || asPath)
}
if (!newRouteAnnouncement) {
if (document.title) {
newRouteAnnouncement = document.title
} else {
newRouteAnnouncement = asPath
}
}

setRouteAnnouncement(newRouteAnnouncement)
},
// TODO: switch to pathname + query object of dynamic route requirements
[asPath]
Expand Down
10 changes: 9 additions & 1 deletion test/integration/client-navigation-a11y/pages/index.js
Expand Up @@ -2,14 +2,22 @@ import Link from 'next/link'

export default () => (
<div id="page-container">
<Link href="/page-with-h1-and-title">
<a id="page-with-h1-and-title-link">
Go to a page with a title and an h1
</a>
</Link>

<Link href="/page-with-h1">
<a id="page-with-h1-link">Go to a page with an h1</a>
<a id="page-with-h1-link">Go to a page without a title but 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
Expand Down
@@ -0,0 +1,11 @@
import Head from 'next/head'

export default () => (
<div id="page-with-h1-and-title">
<Head>
<title>Another Page's Title</title>
</Head>
<h1>My heading</h1>
<div>Extraneous stuff</div>
</div>
)
@@ -1,10 +1,5 @@
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>
Expand Down
84 changes: 48 additions & 36 deletions test/integration/client-navigation-a11y/test/index.test.js
Expand Up @@ -11,6 +11,20 @@ import { join } from 'path'

const context = {}

const navigateTo = async (browser, selector) =>
await browser
.waitForElementByCss('#' + selector + '-link')
.click()
.waitForElementByCss('#' + selector)

const getAnnouncedTitle = async (browser) =>
await browser.waitForElementByCss('#__next-route-announcer__').text()

const getDocumentTitle = async (browser) => await browser.eval('document.title')

const getMainHeadingTitle = async (browser) =>
await browser.elementByCss('h1').text()

describe('Client Navigation accessibility', () => {
beforeAll(async () => {
context.appPort = await findPort()
Expand All @@ -19,7 +33,7 @@ describe('Client Navigation accessibility', () => {
})

const prerender = [
'/page-with-h1, /page-with-title, /page-without-h1-or-title',
'/page-with-h1-and-title, /page-with-h1, /page-with-title, /page-without-h1-or-title',
]

await Promise.all(
Expand All @@ -42,57 +56,55 @@ describe('Client Navigation accessibility', () => {
expect(roleValue).toBe('alert')
await browser.close()
})
describe('There is an h1 tag', () => {
it('has the same innerText value as the h1 tag', async () => {

describe('There is a title but no h1 tag', () => {
it('has the innerText equal to the value of document.title', 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 navigateTo(browser, 'page-with-title')

const routeAnnouncerValue = await getAnnouncedTitle(browser)
const title = await getDocumentTitle(browser)

expect(routeAnnouncerValue).toBe(title)
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 () => {

describe('There is no title but a h1 tag', () => {
it('has the innerText equal to the value of h1', async () => {
const browser = await webdriver(context.appPort, '/')
await browser
.waitForElementByCss('#page-with-title-link')
.click()
.waitForElementByCss('#page-with-title')
await navigateTo(browser, 'page-with-h1')

const title = await browser.eval('document.title')
const routeAnnouncerValue = await getAnnouncedTitle(browser)
const h1Value = await getMainHeadingTitle(browser)

const routeAnnouncerValue = await browser
.waitForElementByCss('#__next-route-announcer__')
.text()
expect(routeAnnouncerValue).toBe(h1Value)
await browser.close()
})
})

expect(title).toBe(routeAnnouncerValue)
describe('There is a title and a h1 tag', () => {
it('has the innerText equal to the value of h1', async () => {
const browser = await webdriver(context.appPort, '/')
await navigateTo(browser, 'page-with-h1-and-title')

const routeAnnouncerValue = await getAnnouncedTitle(browser)
const title = await getDocumentTitle(browser)

expect(routeAnnouncerValue).toBe(title)
await browser.close()
})
})
describe('There is neither an h1 or a title tag', () => {

describe('There is no title and no h1 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')
await navigateTo(browser, 'page-without-h1-or-title')

const routeAnnouncerValue = await getAnnouncedTitle(browser)
const pathname = '/page-without-h1-or-title'

const routeAnnouncerValue = await browser
.waitForElementByCss('#__next-route-announcer__')
.text()

expect(pathname).toBe(routeAnnouncerValue)
expect(routeAnnouncerValue).toBe(pathname)
await browser.close()
})
})
Expand Down