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

…31147) ## Improvement This pull-request should address #24021, improving the page change announcement for assistive technologies by giving priority to `document.title` over `h1`. Interestingly, it would also improve a potential performance bottleneck by skipping calls to `innerText` on the main `h1` raised in [this comment](#20428 (comment)).
vercel · Nov 8, 2021 · 3ceb9c5 · 3ceb9c5
1 parent feed67e
commit 3ceb9c5
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 59 deletions.
diff --git a/packages/next/client/route-announcer.tsx b/packages/next/client/route-announcer.tsx
@@ -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(
     () => {
@@ -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]

diff --git a/test/integration/client-navigation-a11y/pages/index.js b/test/integration/client-navigation-a11y/pages/index.js
@@ -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

diff --git a/test/integration/client-navigation-a11y/pages/page-with-h1-and-title.js b/test/integration/client-navigation-a11y/pages/page-with-h1-and-title.js
@@ -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>
+)
diff --git a/test/integration/client-navigation-a11y/pages/page-with-h1.js b/test/integration/client-navigation-a11y/pages/page-with-h1.js
@@ -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>

diff --git a/test/integration/client-navigation-a11y/test/index.test.js b/test/integration/client-navigation-a11y/test/index.test.js
@@ -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()
@@ -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(
@@ -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()
       })
     })