From d851b63b7f257c1ec711a017cd4407b3027d1f3c Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 14 Mar 2023 17:00:08 -0400 Subject: [PATCH 1/4] implements latest a11y feedback for improving Pagination rm forgotten console log --- src/Pagination/model.tsx | 28 +++++++++++++------ .../Pagination/PaginationModel.test.tsx | 10 +++---- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/Pagination/model.tsx b/src/Pagination/model.tsx index 1ba6f323b18..d1bb9d467e0 100644 --- a/src/Pagination/model.tsx +++ b/src/Pagination/model.tsx @@ -65,6 +65,12 @@ export function buildPaginationModel( for (let idx = 0; idx < sorted.length; idx++) { const num = sorted[idx] const selected = num === currentPage + const last = sorted[idx - 1] + const next = sorted[idx + 1] + const lastDelta = num - last + const nextDelta = num - next + const preceedsBreak = nextDelta !== -1 + if (idx === 0) { if (num !== 1) { // If the first page isn't page one, @@ -78,15 +84,15 @@ export function buildPaginationModel( type: 'NUM', num, selected, + preceedsBreak, }) } else { - const last = sorted[idx - 1] - const delta = num - last - if (delta === 1) { + if (lastDelta === 1) { pages.push({ type: 'NUM', num, selected, + preceedsBreak, }) } else { // We skipped some, so add a break @@ -98,6 +104,7 @@ export function buildPaginationModel( type: 'NUM', num, selected, + preceedsBreak: false, }) } } @@ -124,6 +131,7 @@ type PageType = { num: number disabled?: boolean selected?: boolean + preceedsBreak?: boolean } export function buildComponentData( @@ -169,11 +177,15 @@ export function buildComponentData( case 'NUM': { key = `page-${page.num}` content = String(page.num) - if (page.selected) { - Object.assign(props, {as: 'em', 'aria-current': 'page'}) - } else { - Object.assign(props, {href: hrefBuilder(page.num), 'aria-label': `Page ${page.num}`, onClick}) - } + Object.assign(props, { + href: hrefBuilder(page.num), + // We append "..." to the aria-label for pages that preceed a break because screen readers will + // change the tone the text is read in. + // This is a slightly nicer experience than skipping a bunch of numbers unexpectedly. + 'aria-label': `Page ${page.num}${page.preceedsBreak ? '...' : ''}`, + onClick, + 'aria-current': page.selected ? 'page' : undefined, + }) break } case 'BREAK': { diff --git a/src/__tests__/Pagination/PaginationModel.test.tsx b/src/__tests__/Pagination/PaginationModel.test.tsx index a119873f346..537a1eacc65 100644 --- a/src/__tests__/Pagination/PaginationModel.test.tsx +++ b/src/__tests__/Pagination/PaginationModel.test.tsx @@ -60,7 +60,7 @@ describe('Pagination model', () => { const slice = last(model, 5) const expected = [ - {type: 'NUM'}, + {type: 'NUM', preceedsBreak: true}, {type: 'BREAK'}, {type: 'NUM', num: 9}, {type: 'NUM', num: 10}, @@ -74,11 +74,11 @@ describe('Pagination model', () => { const model = buildPaginationModel(10, 5, true, 1, 1) const expected = [ {type: 'PREV', num: 4}, - {type: 'NUM', num: 1}, + {type: 'NUM', num: 1, preceedsBreak: true}, {type: 'BREAK'}, {type: 'NUM', num: 4}, {type: 'NUM', num: 5, selected: true}, - {type: 'NUM', num: 6}, + {type: 'NUM', num: 6, preceedsBreak: true}, {type: 'BREAK'}, {type: 'NUM', num: 10}, {type: 'NEXT', num: 6}, @@ -95,7 +95,7 @@ describe('Pagination model', () => { {type: 'NUM', num: 3}, // normally with a surround of 1, only 1 and 3 would be shown // however, since 1 was already shown, we extend to 4 - {type: 'NUM', num: 4}, + {type: 'NUM', num: 4, preceedsBreak: true}, {type: 'BREAK'}, ] expect(first(model, 6)).toMatchObject(expected) @@ -123,7 +123,7 @@ describe('Pagination model', () => { {type: 'BREAK', num: 1}, {type: 'NUM', num: 4}, {type: 'NUM', num: 5, selected: true}, - {type: 'NUM', num: 6}, + {type: 'NUM', num: 6, preceedsBreak: true}, {type: 'BREAK', num: 10}, {type: 'NEXT'}, ] From e28561b4afc782e205bf8234ac7caecaa5e1def8 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 14 Mar 2023 17:04:32 -0400 Subject: [PATCH 2/4] adds changeset --- .changeset/lemon-pigs-mix.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lemon-pigs-mix.md diff --git a/.changeset/lemon-pigs-mix.md b/.changeset/lemon-pigs-mix.md new file mode 100644 index 00000000000..616ae88d36e --- /dev/null +++ b/.changeset/lemon-pigs-mix.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Adds new a11y improvements to Pagination. From 91b648214317fb5fdbff0dd0d1a84dc8c2381552 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 15 Mar 2023 16:37:24 -0400 Subject: [PATCH 3/4] Update src/Pagination/model.tsx Co-authored-by: Josh Black --- src/Pagination/model.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Pagination/model.tsx b/src/Pagination/model.tsx index d1bb9d467e0..ebc782eee25 100644 --- a/src/Pagination/model.tsx +++ b/src/Pagination/model.tsx @@ -69,7 +69,7 @@ export function buildPaginationModel( const next = sorted[idx + 1] const lastDelta = num - last const nextDelta = num - next - const preceedsBreak = nextDelta !== -1 + const precedesBreak = nextDelta !== -1 if (idx === 0) { if (num !== 1) { From 909aacbc222ea8a89fccf9dda80733e9c02c0295 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 15 Mar 2023 16:43:03 -0400 Subject: [PATCH 4/4] fixes spelling, updates snaps --- src/Pagination/model.tsx | 10 +++++----- src/__tests__/Pagination/PaginationModel.test.tsx | 10 +++++----- .../Pagination/__snapshots__/Pagination.test.tsx.snap | 9 ++++++--- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Pagination/model.tsx b/src/Pagination/model.tsx index ebc782eee25..914ab52d24f 100644 --- a/src/Pagination/model.tsx +++ b/src/Pagination/model.tsx @@ -84,7 +84,7 @@ export function buildPaginationModel( type: 'NUM', num, selected, - preceedsBreak, + precedesBreak, }) } else { if (lastDelta === 1) { @@ -92,7 +92,7 @@ export function buildPaginationModel( type: 'NUM', num, selected, - preceedsBreak, + precedesBreak, }) } else { // We skipped some, so add a break @@ -104,7 +104,7 @@ export function buildPaginationModel( type: 'NUM', num, selected, - preceedsBreak: false, + precedesBreak: false, }) } } @@ -131,7 +131,7 @@ type PageType = { num: number disabled?: boolean selected?: boolean - preceedsBreak?: boolean + precedesBreak?: boolean } export function buildComponentData( @@ -182,7 +182,7 @@ export function buildComponentData( // We append "..." to the aria-label for pages that preceed a break because screen readers will // change the tone the text is read in. // This is a slightly nicer experience than skipping a bunch of numbers unexpectedly. - 'aria-label': `Page ${page.num}${page.preceedsBreak ? '...' : ''}`, + 'aria-label': `Page ${page.num}${page.precedesBreak ? '...' : ''}`, onClick, 'aria-current': page.selected ? 'page' : undefined, }) diff --git a/src/__tests__/Pagination/PaginationModel.test.tsx b/src/__tests__/Pagination/PaginationModel.test.tsx index 537a1eacc65..3a3ba2e6273 100644 --- a/src/__tests__/Pagination/PaginationModel.test.tsx +++ b/src/__tests__/Pagination/PaginationModel.test.tsx @@ -60,7 +60,7 @@ describe('Pagination model', () => { const slice = last(model, 5) const expected = [ - {type: 'NUM', preceedsBreak: true}, + {type: 'NUM', precedesBreak: true}, {type: 'BREAK'}, {type: 'NUM', num: 9}, {type: 'NUM', num: 10}, @@ -74,11 +74,11 @@ describe('Pagination model', () => { const model = buildPaginationModel(10, 5, true, 1, 1) const expected = [ {type: 'PREV', num: 4}, - {type: 'NUM', num: 1, preceedsBreak: true}, + {type: 'NUM', num: 1, precedesBreak: true}, {type: 'BREAK'}, {type: 'NUM', num: 4}, {type: 'NUM', num: 5, selected: true}, - {type: 'NUM', num: 6, preceedsBreak: true}, + {type: 'NUM', num: 6, precedesBreak: true}, {type: 'BREAK'}, {type: 'NUM', num: 10}, {type: 'NEXT', num: 6}, @@ -95,7 +95,7 @@ describe('Pagination model', () => { {type: 'NUM', num: 3}, // normally with a surround of 1, only 1 and 3 would be shown // however, since 1 was already shown, we extend to 4 - {type: 'NUM', num: 4, preceedsBreak: true}, + {type: 'NUM', num: 4, precedesBreak: true}, {type: 'BREAK'}, ] expect(first(model, 6)).toMatchObject(expected) @@ -123,7 +123,7 @@ describe('Pagination model', () => { {type: 'BREAK', num: 1}, {type: 'NUM', num: 4}, {type: 'NUM', num: 5, selected: true}, - {type: 'NUM', num: 6, preceedsBreak: true}, + {type: 'NUM', num: 6, precedesBreak: true}, {type: 'BREAK', num: 10}, {type: 'NEXT'}, ] diff --git a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap index 7197e07d306..889609579e2 100644 --- a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap +++ b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap @@ -129,12 +129,15 @@ exports[`Pagination renders consistently 1`] = ` > Previous - 1 - +