diff --git a/packages/dify-ui/src/pagination/__tests__/index.spec.tsx b/packages/dify-ui/src/pagination/__tests__/index.spec.tsx index 4b99651d1d2..85bf98e97c2 100644 --- a/packages/dify-ui/src/pagination/__tests__/index.spec.tsx +++ b/packages/dify-ui/src/pagination/__tests__/index.spec.tsx @@ -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(
@@ -35,6 +43,8 @@ async function renderPagination({ page={page} totalPages={totalPages} onPageChange={onPageChange} + siblingCount={siblingCount} + boundaryCount={boundaryCount} data-testid="pagination" > @@ -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( + , + ) + + 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() diff --git a/packages/dify-ui/src/pagination/index.stories.tsx b/packages/dify-ui/src/pagination/index.stories.tsx index 9e2b5d808ac..02df3d87bcf 100644 --- a/packages/dify-ui/src/pagination/index.stories.tsx +++ b/packages/dify-ui/src/pagination/index.stories.tsx @@ -40,13 +40,12 @@ function PaginationDemo(props: React.ComponentProps) { ) } -function DesignSpecDemo() { +function RangeStatesDemo() { return (
- - - - + + +
) } @@ -91,13 +90,12 @@ export const Playground: Story = { }, } -export const DesignSpec: Story = { - render: () => , +export const RangeStates: Story = { + render: () => , 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.', }, }, }, diff --git a/packages/dify-ui/src/pagination/index.tsx b/packages/dify-ui/src/pagination/index.tsx index 3d80574e85b..3daa203a71f 100644 --- a/packages/dify-ui/src/pagination/index.tsx +++ b/packages/dify-ui/src/pagination/index.tsx @@ -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((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, '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( @@ -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 ( = pagination.totalPages || pagination.disabled + const disabled = props.disabled || pagination.page >= pagination.totalPages return ( ({ onPageChange, ...props }: PaginationProps) { - const normalizedTotalPages = Math.max(Math.trunc(totalPages), 0) + const normalizedTotalPages = normalizeTotalPages(totalPages) const normalizedPage = clampPage(page, normalizedTotalPages) const editPageNumber = labels?.editPageNumber?.(normalizedPage, normalizedTotalPages) diff --git a/web/app/components/datasets/documents/detail/completed/__tests__/index.spec.tsx b/web/app/components/datasets/documents/detail/completed/__tests__/index.spec.tsx index 88ef64d8b25..64cd81a6f63 100644 --- a/web/app/components/datasets/documents/detail/completed/__tests__/index.spec.tsx +++ b/web/app/components/datasets/documents/detail/completed/__tests__/index.spec.tsx @@ -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(, { 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(, { 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(, { wrapper: createWrapper() }) - expect(screen.getByRole('navigation', { name: 'Pagination' })).toHaveAttribute( - 'data-totalpages', - '5', - ) + expect(getPageSummaryButton(1, 5)).toBeInTheDocument() }) // Covers search input change