From afdb89fa01f4d949c10dee5449d696da90cb2801 Mon Sep 17 00:00:00 2001 From: Kitty Giraudel Date: Mon, 8 Nov 2021 14:08:58 +0100 Subject: [PATCH 1/2] Give priority to document.title over h1 when announcing page change --- packages/next/client/route-announcer.tsx | 30 +++---- .../client-navigation-a11y/pages/index.js | 10 ++- .../pages/page-with-h1-and-title.js | 11 +++ .../pages/page-with-h1.js | 3 - .../client-navigation-a11y/test/index.test.js | 84 +++++++++++-------- 5 files changed, 81 insertions(+), 57 deletions(-) create mode 100644 test/integration/client-navigation-a11y/pages/page-with-h1-and-title.js diff --git a/packages/next/client/route-announcer.tsx b/packages/next/client/route-announcer.tsx index a778fe5dac448cf..2c5a772b5fcb81b 100644 --- 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 index b616bbe516a5da1..04ed7407e7d2199 100644 --- 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 () => (
+ + + Go to a page with a title and an h1 + + + - Go to a page with an h1 + Go to a page without a title but with an h1 + Go to a page without an h1, but with a title + 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 new file mode 100644 index 000000000000000..f746c7b0fe567f2 --- /dev/null +++ b/test/integration/client-navigation-a11y/pages/page-with-h1-and-title.js @@ -0,0 +1,11 @@ +import Head from 'next/head' + +export default () => ( +
+ + Another Page's Title + +

My heading

+
Extraneous stuff
+
+) diff --git a/test/integration/client-navigation-a11y/pages/page-with-h1.js b/test/integration/client-navigation-a11y/pages/page-with-h1.js index b3624846bc32aa9..ba7f9cfb1b89af4 100644 --- a/test/integration/client-navigation-a11y/pages/page-with-h1.js +++ b/test/integration/client-navigation-a11y/pages/page-with-h1.js @@ -2,9 +2,6 @@ import Head from 'next/head' export default () => (
- - Another Page's Title -

My heading

Extraneous stuff
diff --git a/test/integration/client-navigation-a11y/test/index.test.js b/test/integration/client-navigation-a11y/test/index.test.js index 302b1d13abc97b4..99c347726aa4d1d 100644 --- 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() }) }) From 91623ee4ecdbdce714e56b4445100975ce0b9674 Mon Sep 17 00:00:00 2001 From: Kitty Giraudel Date: Mon, 8 Nov 2021 14:08:58 +0100 Subject: [PATCH 2/2] Give priority to document.title over h1 when announcing page change --- test/integration/client-navigation-a11y/pages/page-with-h1.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/integration/client-navigation-a11y/pages/page-with-h1.js b/test/integration/client-navigation-a11y/pages/page-with-h1.js index ba7f9cfb1b89af4..d531c0df5b1ea38 100644 --- a/test/integration/client-navigation-a11y/pages/page-with-h1.js +++ b/test/integration/client-navigation-a11y/pages/page-with-h1.js @@ -1,5 +1,3 @@ -import Head from 'next/head' - export default () => (

My heading