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

Pagination algorithm enhancements #5504

Merged
merged 23 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
5dbc09a
Simplify Pagination algorithm with an early return
hectahertz Jan 7, 2025
e86e873
Pagination: Keep a consistent width on next and previous
hectahertz Jan 8, 2025
93a0032
Simplify Pagination algorithm and keep number of pages shown consistent
hectahertz Jan 8, 2025
321642a
Add changeset
hectahertz Jan 8, 2025
c1d1901
Replicate style changes to Pagination.module.css
hectahertz Jan 10, 2025
8c89b44
wip: algo refactor
francinelucca Jan 10, 2025
15870fe
refactor(Pagination): refine algorithm
francinelucca Jan 11, 2025
c60d968
fix: lint
francinelucca Jan 13, 2025
f68002c
pagination: small tweaks
francinelucca Jan 13, 2025
853c6da
fix(Pagination): refine logic, add tests
francinelucca Jan 14, 2025
2ff4af4
Merge branch 'main' into hectahertz/pagination-algorithm-enhancements
francinelucca Jan 14, 2025
3536ee1
chore: remove outdated comments
francinelucca Jan 14, 2025
cece1ba
Merge branch 'hectahertz/pagination-algorithm-enhancements' of github…
francinelucca Jan 14, 2025
da86850
test(vrt): update snapshots
francinelucca Jan 14, 2025
1afe2c0
Merge branch 'main' into hectahertz/pagination-algorithm-enhancements
francinelucca Jan 14, 2025
b38a5a4
Merge branch 'hectahertz/pagination-algorithm-enhancements' into refa…
francinelucca Jan 14, 2025
a44f9a0
test(vrt): update snapshots
francinelucca Jan 14, 2025
0865121
Merge branch 'hectahertz/pagination-algorithm-enhancements' into refa…
francinelucca Jan 15, 2025
1f8adbe
chore(Pagination): refactoor
francinelucca Jan 15, 2025
61baf88
Merge branch 'refactor/pagination' of github.com:primer/react into re…
francinelucca Jan 15, 2025
c8706df
fix(Pagination): remove unused type
francinelucca Jan 15, 2025
55edda7
Merge branch 'main' into hectahertz/pagination-algorithm-enhancements
francinelucca Jan 15, 2025
e723e40
Merge branch 'main' into hectahertz/pagination-algorithm-enhancements
francinelucca Jan 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/serious-melons-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Pagination: Optimize the page rendering algorithm and prevent layout shifts.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 2 additions & 10 deletions packages/react/src/Pagination/Pagination.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,8 @@
box-shadow: inset 0 0 0 3px var(--fgColor-onEmphasis);
}

.Page[aria-disabled]:first-child,
.Page[aria-disabled]:hover:first-child {
/* stylelint-disable-next-line primer/spacing */
margin: 0 2px;
/* stylelint-disable-next-line primer/spacing */
margin-right: 6px;
}

.Page[aria-disabled],
.Page[aria-disabled]:hover,
.Page[aria-hidden],
.Page[aria-hidden]:hover,
.Page[role='presentation'],
.Page[role='presentation']:hover {
color: var(--fgColor-disabled);
Expand Down
13 changes: 2 additions & 11 deletions packages/react/src/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,8 @@ const Page = toggleStyledComponent(
box-shadow: inset 0 0 0 3px ${get('colors.fg.onEmphasis')};
}

&[aria-disabled],
&[aria-disabled]:hover {
margin: 0 2px;

&:first-child {
margin-right: 6px;
}
}

&[aria-disabled],
&[aria-disabled]:hover,
&[aria-hidden],
&[aria-hidden]:hover,
hectahertz marked this conversation as resolved.
Show resolved Hide resolved
&[role='presentation'],
&[role='presentation']:hover {
color: ${get('colors.primer.fg.disabled')}; // check
Expand Down
223 changes: 112 additions & 111 deletions packages/react/src/Pagination/model.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,125 +5,126 @@ export function buildPaginationModel(
marginPageCount: number,
surroundingPageCount: number,
) {
const pages = []
const prev: PageType = {type: 'PREV', num: currentPage - 1, disabled: currentPage === 1}
const next: PageType = {type: 'NEXT', num: currentPage + 1, disabled: currentPage === pageCount}
if (!showPages) {
return [prev, next]
}

if (showPages) {
const pageNums: Array<number> = []
const addPage = (n: number) => {
if (n >= 1 && n <= pageCount) {
pageNums.push(n)
}
}
if (pageCount <= 0) {
return [prev, {...next, disabled: true}]
}

// Start by defining the window of pages to show around the current page.
// If the window goes off either edge, shift it until it fits.
let extentLeft = currentPage - surroundingPageCount
let extentRight = currentPage + surroundingPageCount
if (extentLeft < 1 && extentRight > pageCount) {
// Our window is larger than the entire range,
// so simply display every page.
extentLeft = 1
extentRight = pageCount
} else if (extentLeft < 1) {
while (extentLeft < 1) {
extentLeft++
extentRight++
}
} else if (extentRight > pageCount) {
while (extentRight > pageCount) {
extentLeft--
extentRight--
}
}
const pages: PageType[] = []

// Next, include the pages in the margins.
// If a margin page is already covered in the window,
// extend the window to the other direction.
for (let i = 1; i <= marginPageCount; i++) {
const leftPage = i
const rightPage = pageCount - (i - 1)
if (leftPage >= extentLeft) {
extentRight++
} else {
addPage(leftPage)
}
if (rightPage <= extentRight) {
extentLeft--
} else {
addPage(rightPage)
}
}
// number of pages shown on each side of the current page
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
// standardGap: 3
const standardGap = surroundingPageCount + marginPageCount

for (let i = extentLeft; i <= extentRight; i++) {
addPage(i)
}
// the maximum number of pages that can be shown at a given time
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
// maxVisiblePages: 7
const maxVisiblePages = standardGap + standardGap + 1

const sorted = pageNums
.slice()
.sort((a, b) => a - b)
.filter((item, idx, ary) => !idx || item !== ary[idx - 1])
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 precedesBreak = nextDelta !== -1

if (idx === 0) {
if (num !== 1) {
// If the first page isn't page one,
// we need to add a break
pages.push({
type: 'BREAK',
num: 1,
})
}
pages.push({
type: 'NUM',
num,
selected,
precedesBreak,
})
} else {
if (lastDelta === 1) {
pages.push({
type: 'NUM',
num,
selected,
precedesBreak,
})
} else {
// We skipped some, so add a break
pages.push({
type: 'BREAK',
num: num - 1,
})
pages.push({
type: 'NUM',
num,
selected,
precedesBreak: false,
})
}
}
}
// if the number of pages is less than the maximum number of pages that can be shown just return all of them
if (pageCount <= maxVisiblePages) {
addPages(1, pageCount, false)
return [prev, ...pages, next]
}

// startGap is the number of pages hidden by the start ellipsis
// startOffset is the number of pages to offset at the start to compensate
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
// startGap: 5
// startOffset: 0
// when the margin and the surrounding windows overlap.
// [1, _2_, 3, 4, 5, 6, ..., 15]
// startGap = 0
// startOffset: -3 <--
let startGap = 0
let startOffset = 0

const lastPage = pages[pages.length - 1]
if (lastPage.type === 'NUM' && lastPage.num !== pageCount) {
// The last page we rendered wasn't the actual last page,
// so we need an additional break
// When there is overlap
if (currentPage - standardGap - 1 <= 1) {
startOffset = currentPage - standardGap - 2
} else {
startGap = currentPage - standardGap - 1
}

// These are equivalent to startGap and startOffset but at the end of the list
let endGap = 0
let endOffset = 0

// When there is overlap
if (pageCount - currentPage - standardGap <= 1) {
endOffset = pageCount - currentPage - standardGap - 1
} else {
endGap = pageCount - currentPage - standardGap
}

const hasStartEllipsis = startGap > 0
const hasEndEllipsis = endGap > 0

// add pages "before" the start ellipsis (if any)
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
// marginPageCount: 1
// addPages(1, 1, true)
addPages(1, marginPageCount, hasStartEllipsis)

if (hasStartEllipsis) {
addEllipsis(marginPageCount)
}

// add middle pages
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
// marginPageCount: 1
// surroundingPageCount: 2
// startGap: 5
// startOffset: 0
// endGap: 3
// endOffset: 0
// addPages(7, 11, true)
addPages(
marginPageCount + startGap + endOffset + 1,
pageCount - startOffset - endGap - marginPageCount,
hasEndEllipsis,
)

if (hasEndEllipsis) {
addEllipsis(pageCount - startOffset - endGap - marginPageCount)
}

// add pages "after" the start ellipsis (if any)
// [1, ..., 7, 8, _9_, 10, 11, ..., 15]
// marginPageCount: 1
// surroundingPageCount: 2
// startGap: 5
// startOffset: 0
// endGap: 3
// endOffset: 0
// addPages(15, 15)
addPages(pageCount - marginPageCount + 1, pageCount)

return [prev, ...pages, next]

function addEllipsis(previousPage: number): void {
pages.push({
type: 'BREAK',
num: previousPage + 1,
})
}

function addPages(start: number, end: number, precedesBreak: boolean = false): void {
for (let i = start; i <= end; i++) {
pages.push({
type: 'BREAK',
num: pageCount,
type: 'NUM',
num: i,
selected: i === currentPage,
precedesBreak: i === end && precedesBreak,
})
}
}

const prev = {type: 'PREV', num: currentPage - 1, disabled: currentPage === 1}
const next = {type: 'NEXT', num: currentPage + 1, disabled: currentPage === pageCount}
return [prev, ...pages, next]
}

type PageType = {
Expand All @@ -148,7 +149,7 @@ export function buildComponentData(
key = 'page-prev'
content = 'Previous'
if (page.disabled) {
Object.assign(props, {'aria-hidden': 'true'})
Object.assign(props, {rel: 'prev', 'aria-hidden': 'true'})
} else {
Object.assign(props, {
rel: 'prev',
Expand All @@ -163,7 +164,7 @@ export function buildComponentData(
key = 'page-next'
content = 'Next'
if (page.disabled) {
Object.assign(props, {'aria-hidden': 'true'})
Object.assign(props, {rel: 'next', 'aria-hidden': 'true'})
} else {
Object.assign(props, {
rel: 'next',
Expand Down
Loading
Loading