Skip to content

Commit d045ae4

Browse files
hectahertzfrancinelucca
andauthoredJan 30, 2025··
Reintroduce Pagination algorithm enhancements (#5590)
* Revert "Revert "Pagination algorithm enhancements (#5504)" (#5560)" This reverts commit b8284ce. * Remove 'rel' attribute for disabled pagination * Revert "Remove 'rel' attribute for disabled pagination" This reverts commit 8c0c656. * Set aria-disabled=true * fix(Pagination): correct maxVisiblePages calculation * test(Pagination): correct test --------- Co-authored-by: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Co-authored-by: Marie Lucca <francinelucca@github.com>
1 parent 2b05aad commit d045ae4

23 files changed

+321
-138
lines changed
 

‎.changeset/serious-melons-own.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/react": patch
3+
---
4+
5+
Pagination: Optimize the page rendering algorithm and prevent layout shifts.
Loading
Loading

‎packages/react/src/Pagination/Pagination.module.css

+2-10
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,8 @@
103103
box-shadow: inset 0 0 0 3px var(--fgColor-onEmphasis);
104104
}
105105

106-
.Page[aria-disabled]:first-child,
107-
.Page[aria-disabled]:hover:first-child {
108-
/* stylelint-disable-next-line primer/spacing */
109-
margin: 0 2px;
110-
/* stylelint-disable-next-line primer/spacing */
111-
margin-right: 6px;
112-
}
113-
114-
.Page[aria-disabled],
115-
.Page[aria-disabled]:hover,
106+
.Page[aria-hidden],
107+
.Page[aria-hidden]:hover,
116108
.Page[role='presentation'],
117109
.Page[role='presentation']:hover {
118110
color: var(--fgColor-disabled);

‎packages/react/src/Pagination/Pagination.tsx

+2-11
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,8 @@ const Page = toggleStyledComponent(
9191
box-shadow: inset 0 0 0 3px ${get('colors.fg.onEmphasis')};
9292
}
9393
94-
&[aria-disabled],
95-
&[aria-disabled]:hover {
96-
margin: 0 2px;
97-
98-
&:first-child {
99-
margin-right: 6px;
100-
}
101-
}
102-
103-
&[aria-disabled],
104-
&[aria-disabled]:hover,
94+
&[aria-hidden],
95+
&[aria-hidden]:hover,
10596
&[role='presentation'],
10697
&[role='presentation']:hover {
10798
color: ${get('colors.primer.fg.disabled')}; // check

‎packages/react/src/Pagination/model.tsx

+112-111
Original file line numberDiff line numberDiff line change
@@ -5,125 +5,126 @@ export function buildPaginationModel(
55
marginPageCount: number,
66
surroundingPageCount: number,
77
) {
8-
const pages = []
8+
const prev: PageType = {type: 'PREV', num: currentPage - 1, disabled: currentPage === 1}
9+
const next: PageType = {type: 'NEXT', num: currentPage + 1, disabled: currentPage === pageCount}
10+
if (!showPages) {
11+
return [prev, next]
12+
}
913

10-
if (showPages) {
11-
const pageNums: Array<number> = []
12-
const addPage = (n: number) => {
13-
if (n >= 1 && n <= pageCount) {
14-
pageNums.push(n)
15-
}
16-
}
14+
if (pageCount <= 0) {
15+
return [prev, {...next, disabled: true}]
16+
}
1717

18-
// Start by defining the window of pages to show around the current page.
19-
// If the window goes off either edge, shift it until it fits.
20-
let extentLeft = currentPage - surroundingPageCount
21-
let extentRight = currentPage + surroundingPageCount
22-
if (extentLeft < 1 && extentRight > pageCount) {
23-
// Our window is larger than the entire range,
24-
// so simply display every page.
25-
extentLeft = 1
26-
extentRight = pageCount
27-
} else if (extentLeft < 1) {
28-
while (extentLeft < 1) {
29-
extentLeft++
30-
extentRight++
31-
}
32-
} else if (extentRight > pageCount) {
33-
while (extentRight > pageCount) {
34-
extentLeft--
35-
extentRight--
36-
}
37-
}
18+
const pages: PageType[] = []
3819

39-
// Next, include the pages in the margins.
40-
// If a margin page is already covered in the window,
41-
// extend the window to the other direction.
42-
for (let i = 1; i <= marginPageCount; i++) {
43-
const leftPage = i
44-
const rightPage = pageCount - (i - 1)
45-
if (leftPage >= extentLeft) {
46-
extentRight++
47-
} else {
48-
addPage(leftPage)
49-
}
50-
if (rightPage <= extentRight) {
51-
extentLeft--
52-
} else {
53-
addPage(rightPage)
54-
}
55-
}
20+
// number of pages shown on each side of the current page
21+
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
22+
// standardGap: 3
23+
const standardGap = surroundingPageCount + marginPageCount
5624

57-
for (let i = extentLeft; i <= extentRight; i++) {
58-
addPage(i)
59-
}
25+
// the maximum number of pages that can be shown at a given time (account for current page, left and right ellipsis)
26+
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
27+
// maxVisiblePages: 9
28+
const maxVisiblePages = standardGap + standardGap + 3
6029

61-
const sorted = pageNums
62-
.slice()
63-
.sort((a, b) => a - b)
64-
.filter((item, idx, ary) => !idx || item !== ary[idx - 1])
65-
for (let idx = 0; idx < sorted.length; idx++) {
66-
const num = sorted[idx]
67-
const selected = num === currentPage
68-
const last = sorted[idx - 1]
69-
const next = sorted[idx + 1]
70-
const lastDelta = num - last
71-
const nextDelta = num - next
72-
const precedesBreak = nextDelta !== -1
73-
74-
if (idx === 0) {
75-
if (num !== 1) {
76-
// If the first page isn't page one,
77-
// we need to add a break
78-
pages.push({
79-
type: 'BREAK',
80-
num: 1,
81-
})
82-
}
83-
pages.push({
84-
type: 'NUM',
85-
num,
86-
selected,
87-
precedesBreak,
88-
})
89-
} else {
90-
if (lastDelta === 1) {
91-
pages.push({
92-
type: 'NUM',
93-
num,
94-
selected,
95-
precedesBreak,
96-
})
97-
} else {
98-
// We skipped some, so add a break
99-
pages.push({
100-
type: 'BREAK',
101-
num: num - 1,
102-
})
103-
pages.push({
104-
type: 'NUM',
105-
num,
106-
selected,
107-
precedesBreak: false,
108-
})
109-
}
110-
}
111-
}
30+
// if the number of pages is less than the maximum number of pages that can be shown just return all of them
31+
if (pageCount <= maxVisiblePages) {
32+
addPages(1, pageCount, false)
33+
return [prev, ...pages, next]
34+
}
35+
36+
// startGap is the number of pages hidden by the start ellipsis
37+
// startOffset is the number of pages to offset at the start to compensate
38+
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
39+
// startGap: 5
40+
// startOffset: 0
41+
// when the margin and the surrounding windows overlap.
42+
// [1, _2_, 3, 4, 5, 6, ..., 15]
43+
// startGap = 0
44+
// startOffset: -3 <--
45+
let startGap = 0
46+
let startOffset = 0
11247

113-
const lastPage = pages[pages.length - 1]
114-
if (lastPage.type === 'NUM' && lastPage.num !== pageCount) {
115-
// The last page we rendered wasn't the actual last page,
116-
// so we need an additional break
48+
// When there is overlap
49+
if (currentPage - standardGap - 1 <= 1) {
50+
startOffset = currentPage - standardGap - 2
51+
} else {
52+
startGap = currentPage - standardGap - 1
53+
}
54+
55+
// These are equivalent to startGap and startOffset but at the end of the list
56+
let endGap = 0
57+
let endOffset = 0
58+
59+
// When there is overlap
60+
if (pageCount - currentPage - standardGap <= 1) {
61+
endOffset = pageCount - currentPage - standardGap - 1
62+
} else {
63+
endGap = pageCount - currentPage - standardGap
64+
}
65+
66+
const hasStartEllipsis = startGap > 0
67+
const hasEndEllipsis = endGap > 0
68+
69+
// add pages "before" the start ellipsis (if any)
70+
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
71+
// marginPageCount: 1
72+
// addPages(1, 1, true)
73+
addPages(1, marginPageCount, hasStartEllipsis)
74+
75+
if (hasStartEllipsis) {
76+
addEllipsis(marginPageCount)
77+
}
78+
79+
// add middle pages
80+
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
81+
// marginPageCount: 1
82+
// surroundingPageCount: 2
83+
// startGap: 5
84+
// startOffset: 0
85+
// endGap: 3
86+
// endOffset: 0
87+
// addPages(7, 11, true)
88+
addPages(
89+
marginPageCount + startGap + endOffset + 1,
90+
pageCount - startOffset - endGap - marginPageCount,
91+
hasEndEllipsis,
92+
)
93+
94+
if (hasEndEllipsis) {
95+
addEllipsis(pageCount - startOffset - endGap - marginPageCount)
96+
}
97+
98+
// add pages "after" the start ellipsis (if any)
99+
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
100+
// marginPageCount: 1
101+
// surroundingPageCount: 2
102+
// startGap: 5
103+
// startOffset: 0
104+
// endGap: 3
105+
// endOffset: 0
106+
// addPages(15, 15)
107+
addPages(pageCount - marginPageCount + 1, pageCount)
108+
109+
return [prev, ...pages, next]
110+
111+
function addEllipsis(previousPage: number): void {
112+
pages.push({
113+
type: 'BREAK',
114+
num: previousPage + 1,
115+
})
116+
}
117+
118+
function addPages(start: number, end: number, precedesBreak: boolean = false): void {
119+
for (let i = start; i <= end; i++) {
117120
pages.push({
118-
type: 'BREAK',
119-
num: pageCount,
121+
type: 'NUM',
122+
num: i,
123+
selected: i === currentPage,
124+
precedesBreak: i === end && precedesBreak,
120125
})
121126
}
122127
}
123-
124-
const prev = {type: 'PREV', num: currentPage - 1, disabled: currentPage === 1}
125-
const next = {type: 'NEXT', num: currentPage + 1, disabled: currentPage === pageCount}
126-
return [prev, ...pages, next]
127128
}
128129

129130
type PageType = {
@@ -148,7 +149,7 @@ export function buildComponentData(
148149
key = 'page-prev'
149150
content = 'Previous'
150151
if (page.disabled) {
151-
Object.assign(props, {'aria-hidden': 'true'})
152+
Object.assign(props, {rel: 'prev', 'aria-hidden': 'true', 'aria-disabled': 'true'})
152153
} else {
153154
Object.assign(props, {
154155
rel: 'prev',
@@ -163,7 +164,7 @@ export function buildComponentData(
163164
key = 'page-next'
164165
content = 'Next'
165166
if (page.disabled) {
166-
Object.assign(props, {'aria-hidden': 'true'})
167+
Object.assign(props, {rel: 'next', 'aria-hidden': 'true', 'aria-disabled': 'true'})
167168
} else {
168169
Object.assign(props, {
169170
rel: 'next',

‎packages/react/src/__tests__/Pagination/PaginationModel.test.tsx

+200-6
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,166 @@ function last(array: Array<any>, count = 1) {
1414
}
1515

1616
describe('Pagination model', () => {
17+
it('correctly handles negative pages', () => {
18+
const model = buildPaginationModel(-10, 1, true, 1, 2)
19+
expect(first(model).type).toEqual('PREV')
20+
expect(first(model).disabled).toBe(true)
21+
expect(last(model).type).toEqual('NEXT')
22+
expect(last(model).disabled).toBe(true)
23+
expect(model.length).toBe(2)
24+
})
25+
26+
it('correctly handles zero pages', () => {
27+
const model = buildPaginationModel(0, 1, true, 1, 2)
28+
expect(first(model).type).toEqual('PREV')
29+
expect(first(model).disabled).toBe(true)
30+
expect(last(model).type).toEqual('NEXT')
31+
expect(last(model).disabled).toBe(true)
32+
expect(model.length).toBe(2)
33+
})
34+
35+
it('correctly handles 1 page', () => {
36+
const model = buildPaginationModel(1, 1, true, 1, 2)
37+
expect(first(model).type).toEqual('PREV')
38+
expect(first(model).disabled).toBe(true)
39+
expect(last(model).type).toEqual('NEXT')
40+
expect(last(model).disabled).toBe(true)
41+
expect(model.length).toBe(3)
42+
})
43+
44+
it('correctly handles zero margin pages', () => {
45+
const model = buildPaginationModel(6, 2, true, 0, 2)
46+
47+
const expected = [
48+
{
49+
type: 'PREV',
50+
num: 1,
51+
disabled: false,
52+
},
53+
{
54+
type: 'NUM',
55+
num: 1,
56+
selected: false,
57+
precedesBreak: false,
58+
},
59+
{
60+
type: 'NUM',
61+
num: 2,
62+
selected: true,
63+
precedesBreak: false,
64+
},
65+
{
66+
type: 'NUM',
67+
num: 3,
68+
selected: false,
69+
precedesBreak: false,
70+
},
71+
{
72+
type: 'NUM',
73+
num: 4,
74+
selected: false,
75+
precedesBreak: false,
76+
},
77+
{
78+
type: 'NUM',
79+
num: 5,
80+
selected: false,
81+
precedesBreak: false,
82+
},
83+
{
84+
type: 'NUM',
85+
num: 6,
86+
selected: false,
87+
precedesBreak: false,
88+
},
89+
{
90+
type: 'NEXT',
91+
num: 3,
92+
disabled: false,
93+
},
94+
]
95+
96+
expect(model).toMatchObject(expected)
97+
})
98+
99+
it('correctly handles zero surrounding pages', () => {
100+
const model = buildPaginationModel(7, 4, true, 1, 0)
101+
102+
const expected = [
103+
{
104+
type: 'PREV',
105+
num: 3,
106+
disabled: false,
107+
},
108+
{
109+
type: 'NUM',
110+
num: 1,
111+
selected: false,
112+
precedesBreak: true,
113+
},
114+
{
115+
type: 'BREAK',
116+
num: 2,
117+
},
118+
{
119+
type: 'NUM',
120+
num: 4,
121+
selected: true,
122+
precedesBreak: true,
123+
},
124+
{
125+
type: 'BREAK',
126+
num: 5,
127+
},
128+
{
129+
type: 'NUM',
130+
num: 7,
131+
selected: false,
132+
precedesBreak: false,
133+
},
134+
{
135+
type: 'NEXT',
136+
num: 5,
137+
disabled: false,
138+
},
139+
]
140+
141+
expect(model).toMatchObject(expected)
142+
})
143+
144+
it('correctly handles zero margin and surrounding pages', () => {
145+
const model = buildPaginationModel(50, 3, true, 0, 0)
146+
147+
const expected = [
148+
{
149+
type: 'PREV',
150+
num: 2,
151+
disabled: false,
152+
},
153+
{
154+
type: 'BREAK',
155+
num: 1,
156+
},
157+
{
158+
type: 'NUM',
159+
num: 3,
160+
selected: true,
161+
precedesBreak: true,
162+
},
163+
{
164+
type: 'BREAK',
165+
num: 4,
166+
},
167+
{
168+
type: 'NEXT',
169+
num: 4,
170+
disabled: false,
171+
},
172+
]
173+
174+
expect(model).toMatchObject(expected)
175+
})
176+
17177
it('sets disabled on prev links', () => {
18178
const model1 = buildPaginationModel(10, 1, true, 1, 2)
19179
expect(first(model1).type).toEqual('PREV')
@@ -94,26 +254,60 @@ describe('Pagination model', () => {
94254
{type: 'NUM', num: 2, selected: true},
95255
{type: 'NUM', num: 3},
96256
// normally with a surround of 1, only 1 and 3 would be shown
97-
// however, since 1 was already shown, we extend to 4
98-
{type: 'NUM', num: 4, precedesBreak: true},
257+
// however, since we don't overlap, the window is extended to 5
258+
{type: 'NUM', num: 4},
259+
{type: 'NUM', num: 5, precedesBreak: true},
99260
{type: 'BREAK'},
100261
]
101-
expect(first(model, 6)).toMatchObject(expected)
262+
expect(first(model, 7)).toMatchObject(expected)
102263
})
103264

104265
it('adds items to the left if it hits bounds to the right', () => {
105266
const model = buildPaginationModel(15, 14, true, 1, 1)
106267
const expected = [
107268
// normally with a surround of 1, only 13 and 15 would be shown
108-
// however, since 15 was already shown, we extend to 12
269+
// however, since we don't overlap, the window is extended to 11
109270
{type: 'BREAK'},
271+
{type: 'NUM', num: 11},
110272
{type: 'NUM', num: 12},
111273
{type: 'NUM', num: 13},
112274
{type: 'NUM', num: 14, selected: true},
113275
{type: 'NUM', num: 15},
114276
{type: 'NEXT', num: 15},
115277
]
116-
expect(last(model, 6)).toMatchObject(expected)
278+
expect(last(model, 7)).toMatchObject(expected)
279+
})
280+
281+
it('adds a page when there would be only one page hidden by the left ellipsis', () => {
282+
const model = buildPaginationModel(15, 5, true, 1, 2)
283+
const expected = [
284+
{type: 'PREV', num: 4},
285+
{type: 'NUM', num: 1},
286+
{type: 'NUM', num: 2},
287+
{type: 'NUM', num: 3},
288+
{type: 'NUM', num: 4},
289+
{type: 'NUM', num: 5, selected: true},
290+
{type: 'NUM', num: 6},
291+
{type: 'NUM', num: 7, precedesBreak: true},
292+
{type: 'BREAK'},
293+
]
294+
expect(first(model, 9)).toMatchObject(expected)
295+
})
296+
297+
it('adds a page when there would be only one page hidden by the right ellipsis', () => {
298+
const model = buildPaginationModel(15, 11, true, 1, 2)
299+
const expected = [
300+
{type: 'BREAK'},
301+
{type: 'NUM', num: 9},
302+
{type: 'NUM', num: 10},
303+
{type: 'NUM', num: 11, selected: true},
304+
{type: 'NUM', num: 12},
305+
{type: 'NUM', num: 13},
306+
{type: 'NUM', num: 14},
307+
{type: 'NUM', num: 15},
308+
{type: 'NEXT', num: 12},
309+
]
310+
expect(last(model, 9)).toMatchObject(expected)
117311
})
118312

119313
it('correctly creates breaks next to the next/prev links when margin is 0', () => {
@@ -124,7 +318,7 @@ describe('Pagination model', () => {
124318
{type: 'NUM', num: 4},
125319
{type: 'NUM', num: 5, selected: true},
126320
{type: 'NUM', num: 6, precedesBreak: true},
127-
{type: 'BREAK', num: 10},
321+
{type: 'BREAK', num: 7},
128322
{type: 'NEXT'},
129323
]
130324
expect(model).toMatchObject(expected)

0 commit comments

Comments
 (0)
Please sign in to comment.