fix(dify-ui): refine pagination navigation semantics (#40723)

This commit is contained in:
yyh 2026-08-13 07:54:31 +00:00 committed by GitHub
parent 84e348c52b
commit 7a6f380f2c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 132 additions and 130 deletions

View File

@ -16,18 +16,26 @@ import {
const asHTMLElement = (element: HTMLElement | SVGElement) => element as HTMLElement
function getRenderedPageNumbers(container: HTMLElement) {
return Array.from(container.querySelectorAll('ol button'), (button) => Number(button.textContent))
}
async function renderPagination({
page = 2,
totalPages = 200,
onPageChange = vi.fn(),
pageSize = 25,
onPageSizeChange = vi.fn(),
siblingCount,
boundaryCount,
}: {
page?: number
totalPages?: number
onPageChange?: (page: number) => void
pageSize?: number
onPageSizeChange?: (pageSize: number) => void
siblingCount?: number
boundaryCount?: number
} = {}) {
const screen = await render(
<div className="w-236">
@ -35,6 +43,8 @@ async function renderPagination({
page={page}
totalPages={totalPages}
onPageChange={onPageChange}
siblingCount={siblingCount}
boundaryCount={boundaryCount}
data-testid="pagination"
>
<PaginationContent data-testid="content">
@ -65,9 +75,7 @@ describe('Pagination primitive', () => {
it('renders the pagination structure with semantic navigation', async () => {
const { screen } = await renderPagination()
await expect
.element(screen.getByRole('navigation', { name: 'Pagination' }))
.toHaveAttribute('data-page', '2')
await expect.element(screen.getByRole('navigation', { name: 'Pagination' })).toBeInTheDocument()
await expect.element(screen.getByRole('button', { name: 'Previous page' })).toBeInTheDocument()
await expect.element(screen.getByRole('button', { name: 'Next page' })).toBeInTheDocument()
await expect
@ -76,19 +84,53 @@ describe('Pagination primitive', () => {
await expect
.element(screen.getByRole('button', { name: 'Page 2, current page' }))
.toHaveAttribute('aria-current', 'page')
await expect
.element(screen.getByRole('button', { name: 'Page 2, current page' }))
.not.toHaveAttribute('aria-pressed')
await expect.element(screen.getByText('…')).toBeInTheDocument()
})
it('uses one-based page changes for previous, next, and page buttons', async () => {
const { screen, onPageChange } = await renderPagination({ page: 4 })
const { screen, onPageChange } = await renderPagination({ page: 100 })
await screen.getByRole('button', { name: 'Previous page' }).click()
await screen.getByRole('button', { name: 'Next page' }).click()
await screen.getByRole('button', { name: 'Go to page 6' }).click()
await screen.getByRole('button', { name: 'Go to page 1', exact: true }).click()
expect(onPageChange).toHaveBeenNthCalledWith(1, 3)
expect(onPageChange).toHaveBeenNthCalledWith(2, 5)
expect(onPageChange).toHaveBeenNthCalledWith(3, 6)
expect(onPageChange).toHaveBeenNthCalledWith(1, 99)
expect(onPageChange).toHaveBeenNthCalledWith(2, 101)
expect(onPageChange).toHaveBeenNthCalledWith(3, 1)
})
it('does not report a page change when the current page is activated', async () => {
const { screen, onPageChange } = await renderPagination({ page: 4 })
await screen.getByRole('button', { name: 'Page 4, current page' }).click()
expect(onPageChange).not.toHaveBeenCalled()
})
it.each([
['near the start', 2, [1, 2, 3, 4, 5, 200], 1],
['in the middle', 100, [1, 99, 100, 101, 200], 2],
['near the end', 199, [1, 196, 197, 198, 199, 200], 1],
])('keeps a compact page window %s', async (_position, page, expectedPages, expectedEllipses) => {
const { screen } = await renderPagination({ page })
expect(getRenderedPageNumbers(screen.container)).toEqual(expectedPages)
expect(screen.container.querySelectorAll('ol [aria-hidden="true"]')).toHaveLength(
expectedEllipses,
)
})
it('uses sibling and boundary counts as the page range contract', async () => {
const { screen } = await renderPagination({
page: 100,
siblingCount: 0,
boundaryCount: 2,
})
expect(getRenderedPageNumbers(screen.container)).toEqual([1, 2, 100, 199, 200])
})
it('disables previous at the first page', async () => {
@ -106,14 +148,19 @@ describe('Pagination primitive', () => {
it('clamps invalid root page values without exposing invalid state', async () => {
const { screen } = await renderPagination({ page: 999, totalPages: 10 })
await expect
.element(screen.getByRole('navigation', { name: 'Pagination' }))
.toHaveAttribute('data-page', '10')
await expect
.element(screen.getByRole('button', { name: 'Page 10, current page' }))
.toHaveAttribute('aria-current', 'page')
})
it('treats a non-finite page count as an empty pagination state', async () => {
const screen = await render(
<Pagination page={1} totalPages={Number.NaN} onPageChange={vi.fn()} />,
)
expect(screen.container.querySelector('nav')).not.toBeInTheDocument()
})
it('switches the page summary into a selected labelled number field', async () => {
const { screen } = await renderPagination()
@ -265,9 +312,7 @@ describe('Pagination primitive', () => {
it('omits compound page jump and page list content for empty pagination state', async () => {
const { screen } = await renderPagination({ page: 1, totalPages: 0 })
await expect
.element(screen.getByRole('navigation', { name: 'Pagination' }))
.toHaveAttribute('data-page', '1')
await expect.element(screen.getByRole('navigation', { name: 'Pagination' })).toBeInTheDocument()
expect(
screen.container.querySelector('button[aria-label*="current page 1 of 0"]'),
).not.toBeInTheDocument()

View File

@ -40,13 +40,12 @@ function PaginationDemo(props: React.ComponentProps<typeof PaginationExample>) {
)
}
function DesignSpecDemo() {
function RangeStatesDemo() {
return (
<div className="flex w-236 max-w-full flex-col gap-6 bg-components-panel-bg px-16 py-10">
<PaginationExample label="Default pagination" />
<PaginationExample label="Hover pagination" initialPage={2} initialPageSize={25} />
<PaginationExample label="Focused pagination" initialPage={2} initialPageSize={25} />
<PaginationExample label="Page size pagination" initialPage={2} initialPageSize={25} />
<PaginationExample label="Pagination near the start" initialPage={2} />
<PaginationExample label="Pagination in the middle" initialPage={100} />
<PaginationExample label="Pagination near the end" initialPage={199} />
</div>
)
}
@ -91,13 +90,12 @@ export const Playground: Story = {
},
}
export const DesignSpec: Story = {
render: () => <DesignSpecDemo />,
export const RangeStates: Story = {
render: () => <RangeStatesDemo />,
parameters: {
docs: {
description: {
story:
'Pagination rows with default, hover-like, focused, page-size, and skeleton examples.',
story: 'Pagination windows near the beginning, middle, and end of a long result set.',
},
},
},

View File

@ -16,7 +16,6 @@ type PaginationContextValue = {
page: number
totalPages: number
hasPages: boolean
disabled: boolean
onPageChange: (page: number) => void
items: PageItem[]
}
@ -37,6 +36,12 @@ function clampPage(page: number, totalPages: number) {
return Math.min(Math.max(Math.trunc(page), 1), Math.max(totalPages, 1))
}
function normalizeTotalPages(totalPages: number) {
if (!Number.isFinite(totalPages)) return 0
return Math.max(Math.trunc(totalPages), 0)
}
function range(start: number, end: number) {
if (end < start) return []
@ -48,7 +53,6 @@ type GetPageItemsOptions = {
totalPages: number
siblingCount: number
boundaryCount: number
visiblePageCount: number
}
function getPageItems({
@ -56,70 +60,62 @@ function getPageItems({
totalPages,
siblingCount,
boundaryCount,
visiblePageCount,
}: GetPageItemsOptions): PageItem[] {
if (totalPages <= 0) return []
const normalizedPage = clampPage(page, totalPages)
const normalizedBoundaryCount = Math.max(Math.trunc(boundaryCount), 1)
const normalizedSiblingCount = Math.max(Math.trunc(siblingCount), 0)
const windowSize = Math.max(Math.trunc(visiblePageCount), normalizedSiblingCount * 2 + 1)
const normalizedBoundaryCount = Number.isFinite(boundaryCount)
? Math.max(Math.trunc(boundaryCount), 0)
: 1
const normalizedSiblingCount = Number.isFinite(siblingCount)
? Math.max(Math.trunc(siblingCount), 0)
: 1
const visibleItemCount = normalizedBoundaryCount * 2 + normalizedSiblingCount * 2 + 3
if (totalPages <= windowSize + normalizedBoundaryCount) return range(1, totalPages)
if (totalPages <= visibleItemCount) return range(1, totalPages)
const nearStartEnd = windowSize
const nearEndStart = totalPages - windowSize + 1
const middleStart = Math.max(normalizedBoundaryCount + 1, normalizedPage - normalizedSiblingCount)
const middleEnd = Math.min(
totalPages - normalizedBoundaryCount,
normalizedPage + normalizedSiblingCount,
const startPages = range(1, Math.min(normalizedBoundaryCount, totalPages))
const endPages = range(
Math.max(totalPages - normalizedBoundaryCount + 1, normalizedBoundaryCount + 1),
totalPages,
)
const firstEndPage = endPages.at(0)
const siblingStart = Math.max(
Math.min(
normalizedPage - normalizedSiblingCount,
totalPages - normalizedBoundaryCount - normalizedSiblingCount * 2 - 1,
),
normalizedBoundaryCount + 2,
)
const siblingEnd = Math.min(
Math.max(
normalizedPage + normalizedSiblingCount,
normalizedBoundaryCount + normalizedSiblingCount * 2 + 2,
),
firstEndPage !== undefined ? firstEndPage - 2 : totalPages - 1,
)
const items: PageItem[] = [...startPages]
const windowPages =
normalizedPage <= nearStartEnd - normalizedSiblingCount
? range(1, nearStartEnd)
: normalizedPage >= nearEndStart + normalizedSiblingCount
? range(nearEndStart, totalPages)
: range(middleStart, middleEnd)
if (siblingStart > normalizedBoundaryCount + 2) items.push('ellipsis-start')
else if (normalizedBoundaryCount + 1 < totalPages - normalizedBoundaryCount)
items.push(normalizedBoundaryCount + 1)
const pageSet = new Set([
...range(1, normalizedBoundaryCount),
...windowPages,
...range(totalPages - normalizedBoundaryCount + 1, totalPages),
])
const pages = Array.from(pageSet)
.filter((item) => item >= 1 && item <= totalPages)
.sort((a, b) => a - b)
items.push(...range(siblingStart, siblingEnd))
return pages.reduce<PageItem[]>((items, item, index) => {
const previous = pages[index - 1]
if (siblingEnd < totalPages - normalizedBoundaryCount - 1) items.push('ellipsis-end')
else if (totalPages - normalizedBoundaryCount > normalizedBoundaryCount)
items.push(totalPages - normalizedBoundaryCount)
if (previous && item - previous === 2) items.push(previous + 1)
else if (previous && item - previous > 2)
items.push(item < normalizedPage ? 'ellipsis-start' : 'ellipsis-end')
items.push(item)
return items
}, [])
items.push(...endPages)
return items
}
type PaginationRootState = {
page: number
totalPages: number
hasPages: boolean
disabled: boolean
}
type PaginationRootProps = Omit<
useRender.ComponentProps<'nav', PaginationRootState>,
'onChange'
> & {
type PaginationRootProps = Omit<useRender.ComponentProps<'nav'>, 'onChange'> & {
page: number
totalPages: number
onPageChange: (page: number) => void
siblingCount?: number
boundaryCount?: number
visiblePageCount?: number
}
function PaginationRoot({
@ -128,16 +124,14 @@ function PaginationRoot({
onPageChange,
siblingCount = 1,
boundaryCount = 1,
visiblePageCount = 8,
render,
children,
className,
...props
}: PaginationRootProps) {
const normalizedTotalPages = Math.max(Math.trunc(totalPages), 0)
const normalizedTotalPages = normalizeTotalPages(totalPages)
const normalizedPage = clampPage(page, normalizedTotalPages)
const hasPages = normalizedTotalPages > 0
const disabled = normalizedTotalPages <= 1
const items = React.useMemo(
() =>
getPageItems({
@ -145,9 +139,8 @@ function PaginationRoot({
totalPages: normalizedTotalPages,
siblingCount,
boundaryCount,
visiblePageCount,
}),
[boundaryCount, normalizedPage, normalizedTotalPages, siblingCount, visiblePageCount],
[boundaryCount, normalizedPage, normalizedTotalPages, siblingCount],
)
const context = React.useMemo<PaginationContextValue>(
@ -155,11 +148,14 @@ function PaginationRoot({
page: normalizedPage,
totalPages: normalizedTotalPages,
hasPages,
disabled,
onPageChange: (nextPage) => onPageChange(clampPage(nextPage, normalizedTotalPages)),
onPageChange: (nextPage) => {
const normalizedNextPage = clampPage(nextPage, normalizedTotalPages)
if (normalizedNextPage !== normalizedPage) onPageChange(normalizedNextPage)
},
items,
}),
[disabled, hasPages, items, normalizedPage, normalizedTotalPages, onPageChange],
[hasPages, items, normalizedPage, normalizedTotalPages, onPageChange],
)
const defaultProps: useRender.ElementProps<'nav'> = {
@ -174,12 +170,6 @@ function PaginationRoot({
return useRender({
defaultTagName: 'nav',
render,
state: {
page: normalizedPage,
totalPages: normalizedTotalPages,
hasPages,
disabled,
},
props: mergeProps<'nav'>(defaultProps, props),
})
}
@ -246,7 +236,7 @@ function PaginationPrevious({
if (!pagination.hasPages) return null
const disabled = props.disabled || pagination.page <= 1 || pagination.disabled
const disabled = props.disabled || pagination.page <= 1
return (
<BaseButton
@ -276,7 +266,7 @@ function PaginationNext({
if (!pagination.hasPages) return null
const disabled = props.disabled || pagination.page >= pagination.totalPages || pagination.disabled
const disabled = props.disabled || pagination.page >= pagination.totalPages
return (
<BaseButton
@ -574,7 +564,7 @@ function Pagination<Value extends number = number>({
onPageChange,
...props
}: PaginationProps<Value>) {
const normalizedTotalPages = Math.max(Math.trunc(totalPages), 0)
const normalizedTotalPages = normalizeTotalPages(totalPages)
const normalizedPage = clampPage(page, normalizedTotalPages)
const editPageNumber = labels?.editPageNumber?.(normalizedPage, normalizedTotalPages)

View File

@ -305,6 +305,11 @@ const createWrapper = () => {
)
}
const getPageSummaryButton = (page: number, totalPages: number) =>
screen.getByRole('button', {
name: `common.pagination.editPageNumber:{"page":${page},"totalPages":${totalPages}}`,
})
describe('SegmentListContext', () => {
describe('Default Values', () => {
it('should have correct default context values', () => {
@ -497,12 +502,10 @@ describe('Completed Component', () => {
describe('Pagination', () => {
it('should start on the first page', () => {
mockSegmentListData.total = 30
render(<Completed {...defaultProps} />, { wrapper: createWrapper() })
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-page',
'1',
)
expect(getPageSummaryButton(1, 3)).toBeInTheDocument()
})
it('should update page when pagination changes', async () => {
@ -513,10 +516,7 @@ describe('Completed Component', () => {
fireEvent.click(nextPageButton)
await waitFor(() => {
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-page',
'2',
)
expect(getPageSummaryButton(2, 3)).toBeInTheDocument()
})
})
@ -1082,19 +1082,13 @@ describe('Inline callback and hook initialization coverage', () => {
fireEvent.click(screen.getByRole('button', { name: 'common.pagination.next' }))
await waitFor(() => {
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-page',
'2',
)
expect(getPageSummaryButton(2, 3)).toBeInTheDocument()
})
fireEvent.click(screen.getByTestId('status-enabled'))
await waitFor(() => {
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-page',
'1',
)
expect(getPageSummaryButton(1, 3)).toBeInTheDocument()
})
})
@ -1207,28 +1201,6 @@ describe('Inline callback and hook initialization coverage', () => {
})
})
// Covers line 133-135: handlePageChange
it('should handle multiple page changes', async () => {
mockSegmentListData.total = 30
render(<Completed {...defaultProps} />, { wrapper: createWrapper() })
fireEvent.click(screen.getByRole('button', { name: 'common.pagination.next' }))
await waitFor(() => {
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-page',
'2',
)
})
fireEvent.click(screen.getByRole('button', { name: 'common.pagination.next' }))
await waitFor(() => {
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-page',
'3',
)
})
})
it('should compute pagination pages from child chunk data in full-doc mode', () => {
mockDocForm.current = ChunkingModeEnum.parentChild
mockParentMode.current = 'full-doc'
@ -1236,10 +1208,7 @@ describe('Inline callback and hook initialization coverage', () => {
render(<Completed {...defaultProps} />, { wrapper: createWrapper() })
expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute(
'data-totalpages',
'5',
)
expect(getPageSummaryButton(1, 5)).toBeInTheDocument()
})
// Covers search input change