From 099e06df675d8164ef87fe8b3aa5e33d792360d4 Mon Sep 17 00:00:00 2001 From: yoyo837 Date: Wed, 14 Oct 2026 22:33:05 +0800 Subject: [PATCH 1/7] fix: move pagination control semantics off list items --- README.md | 16 + assets/index.less | 3 + package.json | 1 + src/Pager.tsx | 17 +- src/Pagination.tsx | 37 +- src/PaginationItem.tsx | 90 + tests/__snapshots__/demo.test.tsx.snap | 2107 ++++++++-------------- tests/__snapshots__/simple.test.tsx.snap | 10 +- tests/accessibility.test.tsx | 191 ++ tests/index.test.tsx | 120 +- tests/itemRender.test.tsx | 38 +- tests/jumper.test.tsx | 2 +- 12 files changed, 1215 insertions(+), 1417 deletions(-) create mode 100644 src/PaginationItem.tsx create mode 100644 tests/accessibility.test.tsx diff --git a/README.md b/README.md index f058da0a..5749bcc6 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,22 @@ Then open `http://localhost:8000`. | onChange | `(page: number, pageSize: number, info?: { recommendPage?: number }) => void` | - | Triggered when page or page size changes. The `info.recommendPage` is only provided when pageSize changes, indicating the recommended page number based on the new page size. | | onShowSizeChange | `(current: number, size: number) => void` | - | Triggered when page size changes. | +## Custom item accessibility + +Default page and jump anchors expose button semantics, and previous/next actions +use native buttons. The outer `li` keeps its list-item semantics. Focus, labels, +current-page state, and activation belong to the inner control. + +When `itemRender` or an icon returns a native `button` or `a`, Pagination reuses +that control instead of wrapping it. Native buttons use browser Enter/Space +activation; links with `href` retain link behavior. Other custom content keeps +its existing delegated interaction for compatibility. Custom components should +provide their own accessible control semantics; Pagination cannot assume that +they forward DOM props or safely wrap them in another interactive element. + +Custom styles that target the outer item's focus should also account for focus +on its child, for example with `:focus-within` or a child `:focus-visible` rule. + ## Development ```bash diff --git a/assets/index.less b/assets/index.less index 64ceac6d..f1c57c2a 100644 --- a/assets/index.less +++ b/assets/index.less @@ -83,6 +83,7 @@ } &:focus, + &:focus-within, &:hover { border-color: @primary-color; transition: all 0.3s; @@ -101,11 +102,13 @@ } &:focus, + &:focus-within, &:hover { border-color: #40a9ff; } &:focus a, + &:focus-within a, &:hover a { color: #40a9ff; } diff --git a/package.json b/package.json index 2768693b..fd7c42c3 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "@types/node": "^26.0.1", "@types/react": "^19.2.17", "@types/react-dom": "^19.2.3", + "axe-core": "^4.14.0", "cross-env": "^10.1.0", "dumi": "^2.4.38", "eslint": "^10.6.0", diff --git a/src/Pager.tsx b/src/Pager.tsx index 5a512afd..b7b3b3c5 100644 --- a/src/Pager.tsx +++ b/src/Pager.tsx @@ -2,6 +2,7 @@ import { clsx } from 'clsx'; import React from 'react'; import type { PaginationProps } from './interface'; +import PaginationItem from './PaginationItem'; export interface PagerProps extends Pick { rootPrefixCls: string; @@ -11,6 +12,7 @@ export interface PagerProps extends Pick { className?: string; style?: React.CSSProperties; showTitle: boolean; + disabled?: boolean; onClick?: (page: number) => void; onKeyPress?: ( e: React.KeyboardEvent, @@ -28,6 +30,7 @@ const Pager: React.FC = (props) => { className, style, showTitle, + disabled, onClick, onKeyPress, itemRender, @@ -52,29 +55,23 @@ const Pager: React.FC = (props) => { onKeyPress(e, onClick, page); }; - const pager = itemRender( - page, - 'page', - , - ); + const pager = itemRender(page, 'page', {page}); const pagerLabel = `${pageLabel} ${page}`.trim(); return pager ? ( -
  • {pager} -
    • + ) : null; }; diff --git a/src/Pagination.tsx b/src/Pagination.tsx index 2ca06bd8..b88499f9 100644 --- a/src/Pagination.tsx +++ b/src/Pagination.tsx @@ -13,6 +13,7 @@ import zhCN from './locale/zh_CN'; import Options from './Options'; import type { PagerProps } from './Pager'; import Pager from './Pager'; +import PaginationItem from './PaginationItem'; const defaultItemRender: PaginationProps['itemRender'] = (_, __, element) => element; @@ -126,8 +127,6 @@ const Pagination: React.FC = (props) => { ) : (
  • @@ -1936,7 +1713,6 @@ exports[`Example itemRender 1`] = ` aria-disabled="false" aria-label="下一页" class="rc-pagination-next" - role="button" tabindex="0" >
    @@ -1960,134 +1736,114 @@ exports[`Example jumper 1`] = ` >
  • @@ -2839,7 +2547,6 @@ exports[`Example lessPages 1`] = `
  • @@ -2881,7 +2586,6 @@ exports[`Example lessPages 1`] = ` aria-disabled="false" aria-label="下一页" class="rc-pagination-next" - role="button" tabindex="0" >
  • @@ -2981,7 +2677,6 @@ exports[`Example lessPages 1`] = `
  • @@ -3023,7 +2716,6 @@ exports[`Example lessPages 1`] = ` aria-disabled="false" aria-label="下一页" class="rc-pagination-next" - role="button" tabindex="0" >
    • @@ -3845,12 +3452,11 @@ exports[`Example showTitle 1`] = ` >
    • @@ -3961,12 +3554,11 @@ exports[`Example showTotal 1`] = `
  • + ), + nextIcon: ( + + ), + jumpPrevIcon: ( + + Previous jump icon + + ), + jumpNextIcon: ( + + Next jump icon + + ), +}; + +describe('accessible pagination controls', () => { + it.each([ + { total: 1000, defaultCurrent: 50 }, + { total: 1000, defaultCurrent: 50, ...antdIcons }, + { total: 1000, defaultCurrent: 1 }, + { total: 1000, defaultCurrent: 100 }, + { total: 1000, defaultCurrent: 50, disabled: true, ...antdIcons }, + { total: 1000, defaultCurrent: 1, simple: true }, + { total: 1000, defaultCurrent: 100, simple: { readOnly: true } }, + { total: 0 }, + { + total: 1000, + defaultCurrent: 50, + itemRender: (_, __, element) => element, + }, + { + total: 1000, + defaultCurrent: 50, + itemRender: (page) => , + }, + { + total: 1000, + defaultCurrent: 50, + itemRender: (page) => {page}, + }, + ])('passes axe for %j', async (props) => { + const { container } = render( {}} />); + await checkAccessibility(container); + container.querySelectorAll('li').forEach((item) => { + expect(item).not.toHaveAttribute('role', 'button'); + expect(item).not.toHaveAttribute('tabindex'); + }); + expect(container.querySelector('button button')).toBeNull(); + }); + + it.each(['prev', 'next', 'jump-prev', 'jump-next', 'item-51'])( + 'activates %s exactly once for each mouse/Enter/Space action', + async (type) => { + const onChange = jest.fn(); + const { container } = render( + , + ); + const control = container.querySelector( + `.rc-pagination-${type} > :is(button, a)`, + ); + control.focus(); + await userEvent.keyboard('{Enter}'); + expect(onChange).toHaveBeenCalledTimes(1); + await userEvent.keyboard(' '); + expect(onChange).toHaveBeenCalledTimes(type === 'item-51' ? 1 : 2); + // Selecting the current page is intentionally a no-op. + if (type !== 'item-51') { + await userEvent.click(control); + expect(onChange).toHaveBeenCalledTimes(3); + } + }, + ); + + it('has one focus target per item and exposes its current/disabled state', async () => { + const { container } = render( {}} />); + await userEvent.tab(); + expect(container.querySelector('.rc-pagination-item-1 a')).toHaveFocus(); + await userEvent.tab(); + expect(container.querySelector('.rc-pagination-item-2 a')).toHaveFocus(); + expect(container.querySelector('.rc-pagination-item-1 a')).toHaveAttribute( + 'aria-current', + 'page', + ); + expect( + container.querySelector('.rc-pagination-prev button'), + ).toBeDisabled(); + }); + + it('prevents navigation from a disabled custom link', () => { + const onChange = jest.fn(); + const { getByRole } = render( + {page}} + />, + ); + expect(fireEvent.click(getByRole('link', { name: '页 2' }))).toBe(false); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('preserves delegated keyboard behavior for opaque custom content', () => { + const onChange = jest.fn(); + const { container } = render( + {page}} + />, + ); + const item = container.querySelector('.rc-pagination-item-2'); + expect(item).toHaveAttribute('tabindex', '0'); + fireEvent.keyDown(item, { key: 'Enter' }); + expect(onChange).toHaveBeenCalledTimes(1); + }); + it('composes custom click handlers without duplicate activation', async () => { + const onClick = jest.fn(); + const onChange = jest.fn(); + const { container } = render( + } + />, + ); + const control = container.querySelector( + '.rc-pagination-item-2 button', + ); + control.focus(); + await userEvent.keyboard('{Enter}'); + expect(onClick).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('respects custom controls that stop propagation or are disabled', async () => { + const onChange = jest.fn(); + const { container } = render( + ( + + )} + />, + ); + await userEvent.click( + container.querySelector('.rc-pagination-item-2 button'), + ); + expect(onChange).not.toHaveBeenCalled(); + expect( + container.querySelector('.rc-pagination-item-3 button'), + ).toBeDisabled(); + }); +}); diff --git a/tests/index.test.tsx b/tests/index.test.tsx index 7cba57e2..127f46eb 100644 --- a/tests/index.test.tsx +++ b/tests/index.test.tsx @@ -1,6 +1,7 @@ import type { RenderResult } from '@testing-library/react'; import { render, fireEvent } from '@testing-library/react'; import React from 'react'; +import userEvent from '@testing-library/user-event'; import Pagination from '../src'; import { resetWarned } from '@rc-component/util'; import { sizeChangerRender } from './commonUtil'; @@ -21,7 +22,7 @@ describe('Default Pagination', () => { it('onChange should be forbidden when total is default', () => { const pagers = $$('.rc-pagination-item'); - fireEvent.click(pagers[0]); + fireEvent.click(pagers[0].querySelector('button, a') || pagers[0]); expect(onChange).not.toHaveBeenCalled(); }); }); @@ -66,10 +67,9 @@ describe('Uncontrolled Pagination', () => { wrapper.container.querySelector('.rc-pagination-item-active'), ).toHaveTextContent('1'); expect($$('.rc-pagination-item')[0]).toHaveTextContent('1'); - expect($$('.rc-pagination-item')[0]).toHaveAttribute( - 'aria-current', - 'page', - ); + expect( + $$('.rc-pagination-item')[0].querySelector('button, a'), + ).toHaveAttribute('aria-current', 'page'); expect($$('.rc-pagination-item')[0]).toHaveAttribute('title', '1'); }); @@ -98,14 +98,18 @@ describe('Uncontrolled Pagination', () => { }); it('should expose role and label for navigation items', () => { - const prevButton = wrapper.container.querySelector('.rc-pagination-prev'); - const nextButton = wrapper.container.querySelector('.rc-pagination-next'); + const prevButton = wrapper.container.querySelector( + '.rc-pagination-prev button', + ); + const nextButton = wrapper.container.querySelector( + '.rc-pagination-next button', + ); const currentPage = wrapper.container.querySelector( - '.rc-pagination-item-active', + '.rc-pagination-item-active a', ); - expect(prevButton).toHaveAttribute('role', 'button'); + expect(prevButton.tagName).toBe('BUTTON'); expect(prevButton).toHaveAttribute('aria-label'); - expect(nextButton).toHaveAttribute('role', 'button'); + expect(nextButton.tagName).toBe('BUTTON'); expect(nextButton).toHaveAttribute('aria-label'); expect(currentPage).toHaveAttribute('role', 'button'); expect(currentPage).toHaveAttribute('aria-current', 'page'); @@ -116,7 +120,7 @@ describe('Uncontrolled Pagination', () => { expect(pagers).toHaveLength(3); const page2 = pagers[1]; expect(page2).toHaveClass('rc-pagination-item-2'); - fireEvent.click(page2); + fireEvent.click(page2.querySelector('button, a') || page2); expect( wrapper.container.querySelector('.rc-pagination-item-active'), ).toHaveTextContent('2'); @@ -126,7 +130,7 @@ describe('Uncontrolled Pagination', () => { it('should response next page', () => { const nextButton = wrapper.container.querySelector('.rc-pagination-next'); - fireEvent.click(nextButton); + fireEvent.click(nextButton.querySelector('button, a') || nextButton); expect( wrapper.container.querySelector('.rc-pagination-item-active'), ).toHaveTextContent('2'); @@ -197,9 +201,9 @@ describe('Uncontrolled Pagination', () => { ); expect(totalText).toHaveTextContent('1 - 10 of 25 items'); const nextButton = wrapper.container.querySelector('.rc-pagination-next'); - fireEvent.click(nextButton); + fireEvent.click(nextButton.querySelector('button, a') || nextButton); expect(totalText).toHaveTextContent('11 - 20 of 25 items'); - fireEvent.click(nextButton); + fireEvent.click(nextButton.querySelector('button, a') || nextButton); expect(totalText).toHaveTextContent('21 - 25 of 25 items'); }); @@ -213,9 +217,10 @@ describe('Uncontrolled Pagination', () => { warnSpy.mockRestore(); }); - it('should response keyboard event', () => { + it('should response keyboard event', async () => { const pagers = $$('.rc-pagination-item'); - fireEvent.keyDown(pagers[2], { key: 'Enter', keyCode: 13, which: 13 }); + pagers[2].querySelector('button, a').focus(); + await userEvent.keyboard('{Enter}'); expect( wrapper.container.querySelector('.rc-pagination-item-active'), ).toHaveTextContent('3'); @@ -250,7 +255,7 @@ describe('Controlled Pagination', () => { it('should not response mouse click', () => { const nextButton = wrapper.container.querySelector('.rc-pagination-next'); - fireEvent.click(nextButton); + fireEvent.click(nextButton.querySelector('button, a') || nextButton); expect( wrapper.container.querySelector('.rc-pagination-item-active'), ).toHaveTextContent('2'); @@ -611,7 +616,7 @@ describe('should emit onChange when total is string', () => { it('onChange should be called when click page', () => { const pagers = wrapper.container.querySelectorAll('.rc-pagination-item-3'); - fireEvent.click(pagers[0]); + fireEvent.click(pagers[0].querySelector('button, a') || pagers[0]); expect(onChange).toHaveBeenCalledWith(3, 10); }); }); @@ -619,7 +624,12 @@ describe('should emit onChange when total is string', () => { describe('keyboard support', () => { let wrapper: RenderResult; const onChange = jest.fn(); - const $ = (selector: string) => wrapper.container.querySelector(selector); + const $ = (selector: string) => + wrapper.container.querySelector(selector + ' > :is(button, a)'); + async function activate(control: Element, key: string) { + (control as HTMLElement).focus(); + await userEvent.keyboard(key); + } beforeEach(() => { wrapper = render( @@ -632,105 +642,101 @@ describe('keyboard support', () => { onChange.mockReset(); }); - it('should work for prev page', () => { + it('should work for prev page', async () => { const prevButton = $('li.rc-pagination-prev'); expect(prevButton).toBeTruthy(); - fireEvent.click(prevButton); - fireEvent.click(prevButton); + fireEvent.click(prevButton.querySelector('button, a') || prevButton); + fireEvent.click(prevButton.querySelector('button, a') || prevButton); - fireEvent.keyDown(prevButton, { key: 'Enter', keyCode: 13, which: 13 }); - fireEvent.keyDown(prevButton, { key: 'Enter', keyCode: 13, which: 13 }); + await activate(prevButton, '{Enter}'); + await activate(prevButton, '{Enter}'); expect(onChange).toHaveBeenLastCalledWith(46, 10); }); - it('should work for prev page with space key', () => { + it('should work for prev page with space key', async () => { const prevButton = $('li.rc-pagination-prev'); expect(prevButton).toBeTruthy(); - fireEvent.keyDown(prevButton, { key: ' ', keyCode: 32, which: 32 }); - fireEvent.keyDown(prevButton, { key: 'Spacebar', keyCode: 32, which: 32 }); + await activate(prevButton, ' '); + await activate(prevButton, ' '); expect(onChange).toHaveBeenLastCalledWith(48, 10); }); - it('should work for next page', () => { + it('should work for next page', async () => { const nextButton = $('li.rc-pagination-next'); expect(nextButton).toBeTruthy(); - fireEvent.keyDown(nextButton, { key: 'Enter', keyCode: 13, which: 13 }); - fireEvent.keyDown(nextButton, { key: 'Enter', keyCode: 13, which: 13 }); + await activate(nextButton, '{Enter}'); + await activate(nextButton, '{Enter}'); - fireEvent.click(nextButton); - fireEvent.click(nextButton); + fireEvent.click(nextButton.querySelector('button, a') || nextButton); + fireEvent.click(nextButton.querySelector('button, a') || nextButton); expect(onChange).toHaveBeenLastCalledWith(54, 10); }); - it('should work for next page with space key', () => { + it('should work for next page with space key', async () => { const nextButton = $('li.rc-pagination-next'); expect(nextButton).toBeTruthy(); - fireEvent.keyDown(nextButton, { key: ' ', keyCode: 32, which: 32 }); - fireEvent.keyDown(nextButton, { key: 'Spacebar', keyCode: 32, which: 32 }); + await activate(nextButton, ' '); + await activate(nextButton, ' '); expect(onChange).toHaveBeenLastCalledWith(52, 10); }); - it('should work for next page with enter keyCode only', () => { + it('should work for next page with native Enter activation', async () => { const nextButton = $('li.rc-pagination-next'); expect(nextButton).toBeTruthy(); - fireEvent.keyDown(nextButton, { keyCode: 13, which: 13 }); + await activate(nextButton, '{Enter}'); expect(onChange).toHaveBeenLastCalledWith(51, 10); }); - it('should work for jump prev page', () => { + it('should work for jump prev page', async () => { const jumpPrevButton = $('li.rc-pagination-jump-prev'); expect(jumpPrevButton).toBeTruthy(); - fireEvent.keyDown(jumpPrevButton, { key: 'Enter', keyCode: 13, which: 13 }); - fireEvent.click(jumpPrevButton); + await activate(jumpPrevButton, '{Enter}'); + fireEvent.click( + jumpPrevButton.querySelector('button, a') || jumpPrevButton, + ); expect(onChange).toHaveBeenLastCalledWith(40, 10); }); - it('should work for jump prev page with space key', () => { + it('should work for jump prev page with space key', async () => { const jumpPrevButton = $('li.rc-pagination-jump-prev'); expect(jumpPrevButton).toBeTruthy(); - fireEvent.keyDown(jumpPrevButton, { key: ' ', keyCode: 32, which: 32 }); - fireEvent.keyDown(jumpPrevButton, { - key: 'Spacebar', - keyCode: 32, - which: 32, - }); + await activate(jumpPrevButton, ' '); + await activate(jumpPrevButton, ' '); expect(onChange).toHaveBeenLastCalledWith(40, 10); }); - it('should work for jump next page', () => { + it('should work for jump next page', async () => { const jumpNextButton = $('li.rc-pagination-jump-next'); expect(jumpNextButton).toBeTruthy(); - fireEvent.click(jumpNextButton); - fireEvent.keyDown(jumpNextButton, { key: 'Enter', keyCode: 13, which: 13 }); + fireEvent.click( + jumpNextButton.querySelector('button, a') || jumpNextButton, + ); + await activate(jumpNextButton, '{Enter}'); expect(onChange).toHaveBeenLastCalledWith(60, 10); }); - it('should work for jump next page with space key', () => { + it('should work for jump next page with space key', async () => { const jumpNextButton = $('li.rc-pagination-jump-next'); expect(jumpNextButton).toBeTruthy(); - fireEvent.keyDown(jumpNextButton, { key: ' ', keyCode: 32, which: 32 }); - fireEvent.keyDown(jumpNextButton, { - key: 'Spacebar', - keyCode: 32, - which: 32, - }); + await activate(jumpNextButton, ' '); + await activate(jumpNextButton, ' '); expect(onChange).toHaveBeenLastCalledWith(60, 10); }); diff --git a/tests/itemRender.test.tsx b/tests/itemRender.test.tsx index 50c426f2..b9884b80 100644 --- a/tests/itemRender.test.tsx +++ b/tests/itemRender.test.tsx @@ -26,21 +26,31 @@ describe('itemRender', () => { const jumpNext = $$('.rc-pagination-jump-next'); const active = $$('.rc-pagination-item-active'); - expect(prev.innerHTML).toBe( - `${currentPage - 1}`, + [prev, next, jumpPrev, jumpNext, active].forEach((item) => { + const link = item.querySelector('a'); + expect(link).toHaveAttribute('href'); + expect(link).toHaveAttribute('aria-label'); + expect(link).toHaveAttribute('tabindex', '0'); + expect(item).not.toHaveAttribute('tabindex'); + expect(item).not.toHaveAttribute('role'); + }); + expect(prev.querySelector('a')).toHaveAttribute( + 'href', + `#${currentPage - 1}`, ); - expect(next.innerHTML).toBe( - `${currentPage + 1}`, + expect(next.querySelector('a')).toHaveAttribute( + 'href', + `#${currentPage + 1}`, ); - expect(jumpPrev.innerHTML).toBe( - `${currentPage - 5}`, + expect(jumpPrev.querySelector('a')).toHaveAttribute( + 'href', + `#${currentPage - 5}`, ); - expect(jumpNext.innerHTML).toBe( - `${currentPage + 5}`, - ); - expect(active.innerHTML).toBe( - `${currentPage}`, + expect(jumpNext.querySelector('a')).toHaveAttribute( + 'href', + `#${currentPage + 5}`, ); + expect(active.querySelector('a')).toHaveAttribute('aria-current', 'page'); }); it('should support empty custom itemRender', () => { @@ -105,7 +115,9 @@ describe('itemRender', () => { ); const prev = container.querySelector('.rc-pagination-prev'); const next = container.querySelector('.rc-pagination-next'); - expect(prev.innerHTML).toBe('0'); - expect(next.innerHTML).toBe('2'); + expect(prev.querySelector('a')).toHaveAttribute('aria-disabled', 'true'); + expect(prev.querySelector('a')).toHaveAttribute('tabindex', '-1'); + expect(prev.querySelector('a')).not.toHaveAttribute('disabled'); + expect(next.querySelector('a')).toHaveAttribute('aria-disabled', 'false'); }); }); diff --git a/tests/jumper.test.tsx b/tests/jumper.test.tsx index f87dbcb7..71825bbc 100644 --- a/tests/jumper.test.tsx +++ b/tests/jumper.test.tsx @@ -186,7 +186,7 @@ describe('simple quick jumper', () => { // https://github.com/ant-design/ant-design/issues/10080 it('should not quick jump to previous page when input invalid char', () => { const nextButton = wrapper.container.querySelector('.rc-pagination-next'); - fireEvent.click(nextButton); + fireEvent.click(nextButton.querySelector('button, a')); const input = wrapper.container.querySelector('input'); fireEvent.change(input, { target: { value: '&' } }); fireEvent.keyUp(input, { key: 'Enter', keyCode: 13, which: 13 }); From 4144185ad91e5fe9f5d06453ec3f3b8b213f1e42 Mon Sep 17 00:00:00 2001 From: yoyo837 Date: Wed, 14 Oct 2026 22:46:32 +0800 Subject: [PATCH 2/7] fix: preserve quick-jump behavior when focusing inner controls --- src/Options.tsx | 5 ++-- tests/accessibility.test.tsx | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/Options.tsx b/src/Options.tsx index aa570dc0..45bcb2c6 100644 --- a/src/Options.tsx +++ b/src/Options.tsx @@ -73,8 +73,9 @@ const Options: React.FC = (props) => { setGoInputText(''); if ( e.relatedTarget && - (e.relatedTarget.className.includes(`${rootPrefixCls}-item-link`) || - e.relatedTarget.className.includes(`${rootPrefixCls}-item`)) + e.relatedTarget.closest( + `.${rootPrefixCls}-item-link, .${rootPrefixCls}-item`, + ) ) { return; } diff --git a/tests/accessibility.test.tsx b/tests/accessibility.test.tsx index f43e68be..66cca984 100644 --- a/tests/accessibility.test.tsx +++ b/tests/accessibility.test.tsx @@ -188,4 +188,52 @@ describe('accessible pagination controls', () => { container.querySelector('.rc-pagination-item-3 button'), ).toBeDisabled(); }); + it.each(['prev', 'next', 'jump-prev', 'jump-next'])( + 'retains Enter/Space handling for opaque %s content', + (type) => { + const onChange = jest.fn(); + const { container } = render( + {page}} + />, + ); + const item = container.querySelector(`.rc-pagination-${type}`); + fireEvent.keyDown(item, { key: 'Enter' }); + expect(onChange).toHaveBeenCalledTimes(1); + fireEvent.keyDown(item, { key: ' ' }); + expect(onChange).toHaveBeenCalledTimes(2); + }, + ); + + it('allows a custom anchor to cancel delegated keyboard activation', () => { + const onChange = jest.fn(); + const onKeyDown = jest.fn((event: React.KeyboardEvent) => + event.preventDefault(), + ); + const { container } = render( + {page}} + />, + ); + fireEvent.keyDown(container.querySelector('.rc-pagination-item-2 a'), { + key: 'Enter', + }); + expect(onKeyDown).toHaveBeenCalledTimes(1); + expect(onChange).not.toHaveBeenCalled(); + }); + it('does not submit the quick-jump input when focusing a page control', async () => { + const onChange = jest.fn(); + const { getByRole } = render( + , + ); + await userEvent.type(getByRole('textbox'), '3'); + await userEvent.click(getByRole('button', { name: '页 2' })); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenLastCalledWith(2, 10); + }); }); From 362213c3d4ffc329ce71175897f9887be4cd3494 Mon Sep 17 00:00:00 2001 From: yoyo837 <6134547+yoyo837@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:51:05 +0800 Subject: [PATCH 3/7] refactor: merge control props onto itemRender element directly Drop the button/a type sniffing in PaginationItem: control semantics (role, labels, tab order, click and key handling) are always merged onto the rendered element with cloneElement, and the list item only keeps them when no valid element is provided. Native buttons and links keep their own Enter/Space activation, detected from the event target to avoid double activation. Disabled state is exposed via aria-disabled and tab order instead of the native disabled attribute, and the redundant aria-disabled="false" output is gone. --- README.md | 13 +- src/PaginationItem.tsx | 74 +++-- tests/__snapshots__/demo.test.tsx.snap | 338 +++++++++-------------- tests/__snapshots__/simple.test.tsx.snap | 8 +- tests/accessibility.test.tsx | 18 +- tests/index.test.tsx | 9 +- tests/itemRender.test.tsx | 2 +- tests/simple.test.tsx | 5 +- 8 files changed, 201 insertions(+), 266 deletions(-) diff --git a/README.md b/README.md index 5749bcc6..befbcddd 100644 --- a/README.md +++ b/README.md @@ -110,12 +110,13 @@ Default page and jump anchors expose button semantics, and previous/next actions use native buttons. The outer `li` keeps its list-item semantics. Focus, labels, current-page state, and activation belong to the inner control. -When `itemRender` or an icon returns a native `button` or `a`, Pagination reuses -that control instead of wrapping it. Native buttons use browser Enter/Space -activation; links with `href` retain link behavior. Other custom content keeps -its existing delegated interaction for compatibility. Custom components should -provide their own accessible control semantics; Pagination cannot assume that -they forward DOM props or safely wrap them in another interactive element. +When `itemRender` or an icon supplies an element, Pagination merges its control +semantics (role, labels, tab order, and activation) directly onto that element +instead of wrapping it. Native buttons and links with `href` keep their own +keyboard activation; other elements receive button semantics and Enter/Space +handling. Custom components must spread the DOM props they receive to remain +interactive and accessible; Pagination cannot assume they forward props or can +be wrapped in another interactive element. Custom styles that target the outer item's focus should also account for focus on its child, for example with `:focus-within` or a child `:focus-visible` rule. diff --git a/src/PaginationItem.tsx b/src/PaginationItem.tsx index c7560e1b..c291de5a 100644 --- a/src/PaginationItem.tsx +++ b/src/PaginationItem.tsx @@ -4,7 +4,10 @@ interface PaginationItemProps extends React.LiHTMLAttributes { disabled?: boolean; } -/** Keep the list item structural when itemRender supplies a native control. */ +/** + * Keep the list item structural and merge control semantics directly onto the + * element returned by `itemRender`, so custom controls own their DOM contract. + */ const PaginationItem: React.FC = ({ children, disabled, @@ -16,13 +19,11 @@ const PaginationItem: React.FC = ({ 'aria-disabled': ariaDisabled, ...restProps }) => { - const nativeControl = - React.isValidElement>(children) && - (children.type === 'button' || children.type === 'a'); + const controlDisabled = disabled || ariaDisabled === true; - if (!nativeControl) { - // Arbitrary components need not forward DOM props. Preserve their existing - // content and delegated interaction instead of wrapping interactive nodes. + // Only when a valid element is provided are control semantics merged into + // it; otherwise the list item itself keeps them as the fallback control. + if (!React.isValidElement>(children)) { return (
  • = ({ aria-current={ariaCurrent} aria-disabled={ariaDisabled} > - {React.isValidElement(children) && - typeof children.type !== 'string' && - ariaDisabled !== undefined - ? React.cloneElement( - children as React.ReactElement<{ disabled?: boolean }>, - { disabled }, - ) - : children} + {children}
    • ); } - const button = children.type === 'button'; - const link = !button && !!children.props.href; - const controlDisabled = disabled || (button && children.props.disabled); + const { props: controlProps } = children; + const control = React.cloneElement(children, { - 'aria-hidden': undefined, - 'aria-label': children.props['aria-label'] ?? ariaLabel, + role: controlProps.href === undefined ? 'button' : undefined, + 'aria-label': controlProps['aria-label'] ?? ariaLabel, 'aria-current': ariaCurrent, - 'aria-disabled': controlDisabled || ariaDisabled, - tabIndex: controlDisabled ? -1 : 0, - ...(button ? { type: 'button', disabled: controlDisabled } : {}), - ...(!button && !link - ? { - role: 'button', - onKeyDown: (event: React.KeyboardEvent) => { - children.props.onKeyDown?.(event); - if (!event.defaultPrevented) { - onKeyDown?.(event as React.KeyboardEvent); - } - }, - } - : {}), + 'aria-disabled': controlDisabled || undefined, + tabIndex: controlDisabled || tabIndex == null ? -1 : 0, onClick: (event: React.MouseEvent) => { if (controlDisabled) { event.preventDefault(); return; } - children.props.onClick?.(event); + controlProps.onClick?.(event); if (!event.isPropagationStopped()) { onClick?.(event as React.MouseEvent); } }, + onKeyDown: (event: React.KeyboardEvent) => { + controlProps.onKeyDown?.(event); + if (event.defaultPrevented) { + return; + } + // Native buttons and links already activate on Enter/Space through + // their own click synthesis; activating here as well would fire twice. + const { currentTarget } = event; + const nativeActivation = + currentTarget.tagName === 'BUTTON' || + (currentTarget.tagName === 'A' && currentTarget.hasAttribute('href')); + if (!nativeActivation) { + onKeyDown?.(event as React.KeyboardEvent); + } + }, }); - // Native buttons synthesize clicks for Enter/Space; handling keydown on the - // parent as well would activate the same action twice. - return ( -
  • - {control} -
    • - ); + return
  • {control}
    • ; }; export default PaginationItem; diff --git a/tests/__snapshots__/demo.test.tsx.snap b/tests/__snapshots__/demo.test.tsx.snap index 92f78786..9ddb8617 100644 --- a/tests/__snapshots__/demo.test.tsx.snap +++ b/tests/__snapshots__/demo.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +// Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Example align 1`] = `
    @@ -6,14 +6,13 @@ exports[`Example align 1`] = ` class="rc-pagination rc-pagination-start" >
  • -
    +
    Next
    • @@ -1735,13 +1718,12 @@ exports[`Example jumper 1`] = ` class="rc-pagination" >
  • ( + +); + +describe('itemRender interaction compatibility', () => { + it.each([ + [1, 'prev'], + [5, 'next'], + ] as const)('keeps native %s-page %s controls disabled', (current, type) => { + const { container } = render( + {}} + itemRender={buttonItemRender} + />, + ); + expect( + container.querySelector(`.rc-pagination-${type} button`), + ).toBeDisabled(); + }); + + it('blocks custom keyboard callbacks before handling a disabled DOM control', () => { + const onKeyDown = jest.fn(); + const onClick = jest.fn(); + const onChange = jest.fn(); + const { container } = render( + ( + + {page} + + )} + />, + ); + const control = container.querySelector('.rc-pagination-item-2 span'); + fireEvent.keyDown(control, { key: 'Enter' }); + fireEvent.click(control); + expect(onKeyDown).not.toHaveBeenCalled(); + expect(onClick).not.toHaveBeenCalled(); + expect(onChange).not.toHaveBeenCalled(); + }); + + it.each([ + ['prev', 49], + ['next', 51], + ['jump-prev', 45], + ['jump-next', 55], + ['item-51', 51], + ] as const)( + 'keeps mouse and keyboard activation for an opaque %s component', + async (type, page) => { + const Label = ({ value }: { value: number }) => {value}; + const onChange = jest.fn(); + const { container } = render( +
  • { + onClickCapture={(event) => { if (controlDisabled) { event.preventDefault(); - return; + event.stopPropagation(); } - onClick?.(event); }} - onKeyDown={(event) => { - if (controlDisabled) { - if (isEnterOrSpaceKey(event)) { - event.preventDefault(); - } - return; + onKeyDownCapture={(event) => { + if (controlDisabled && isEnterOrSpaceKey(event)) { + event.preventDefault(); + event.stopPropagation(); } + }} + onClick={onClick} + onKeyDown={(event) => { if ( + !controlDisabled && !event.defaultPrevented && !event.isPropagationStopped() && !hasNativeActivation(event.target) diff --git a/tests/__snapshots__/demo.test.tsx.snap b/tests/__snapshots__/demo.test.tsx.snap index db0e3da9..55dc2bd9 100644 --- a/tests/__snapshots__/demo.test.tsx.snap +++ b/tests/__snapshots__/demo.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Example align 1`] = `
    diff --git a/tests/__snapshots__/simple.test.tsx.snap b/tests/__snapshots__/simple.test.tsx.snap index 9bc60f00..5ec8e8e3 100644 --- a/tests/__snapshots__/simple.test.tsx.snap +++ b/tests/__snapshots__/simple.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`simple Pagination props: showQuickJumper should render normally quick-jumper 1`] = `
    { const prevButton = $('li.rc-pagination-prev'); expect(prevButton).toBeTruthy(); - fireEvent.click(prevButton.querySelector('button, a') || prevButton); - fireEvent.click(prevButton.querySelector('button, a') || prevButton); + fireEvent.click(prevButton); + fireEvent.click(prevButton); await activate(prevButton, '{Enter}'); await activate(prevButton, '{Enter}'); @@ -677,8 +677,8 @@ describe('keyboard support', () => { await activate(nextButton, '{Enter}'); await activate(nextButton, '{Enter}'); - fireEvent.click(nextButton.querySelector('button, a') || nextButton); - fireEvent.click(nextButton.querySelector('button, a') || nextButton); + fireEvent.click(nextButton); + fireEvent.click(nextButton); expect(onChange).toHaveBeenLastCalledWith(54, 10); }); @@ -707,9 +707,7 @@ describe('keyboard support', () => { expect(jumpPrevButton).toBeTruthy(); await activate(jumpPrevButton, '{Enter}'); - fireEvent.click( - jumpPrevButton.querySelector('button, a') || jumpPrevButton, - ); + fireEvent.click(jumpPrevButton); expect(onChange).toHaveBeenLastCalledWith(40, 10); }); @@ -728,9 +726,7 @@ describe('keyboard support', () => { const jumpNextButton = $('li.rc-pagination-jump-next'); expect(jumpNextButton).toBeTruthy(); - fireEvent.click( - jumpNextButton.querySelector('button, a') || jumpNextButton, - ); + fireEvent.click(jumpNextButton); await activate(jumpNextButton, '{Enter}'); expect(onChange).toHaveBeenLastCalledWith(60, 10); diff --git a/tests/itemRender-compat.test.tsx b/tests/itemRender-compat.test.tsx index 4e41ccb7..977ec50c 100644 --- a/tests/itemRender-compat.test.tsx +++ b/tests/itemRender-compat.test.tsx @@ -96,6 +96,89 @@ describe('itemRender interaction compatibility', () => { }, ); + it.each([ + ['fragment', 'global'], + ['fragment', 'prev'], + ['fragment', 'next'], + ['opaque', 'global'], + ['opaque', 'prev'], + ['opaque', 'next'], + ] as const)( + 'blocks child activation for a disabled %s renderer (%s)', + (kind, mode) => { + const onClick = jest.fn(); + const onKeyDown = jest.fn(); + const onChange = jest.fn(); + const Control = () => ( + + ); + const { container } = render( + + kind === 'fragment' ? ( + <> + + + ) : ( + + ) + } + />, + ); + const button = container.querySelector( + `.rc-pagination-${mode === 'global' ? 'item-2' : mode} button`, + ); + expect(fireEvent.click(button)).toBe(false); + expect(fireEvent.keyDown(button, { key: 'Enter' })).toBe(false); + expect(fireEvent.keyDown(button, { key: ' ' })).toBe(false); + expect(onClick).not.toHaveBeenCalled(); + expect(onKeyDown).not.toHaveBeenCalled(); + expect(onChange).not.toHaveBeenCalled(); + expect(fireEvent.keyDown(button, { key: 'Tab' })).toBe(true); + expect(onKeyDown).toHaveBeenCalledTimes(1); + }, + ); + + it.each([false, true])( + 'allows focus to leave a native button after disabling (shift=%s)', + async (shift) => { + const onChange = jest.fn(); + const renderPagination = (disabled: boolean) => ( +
    + + + +
    + ); + const { container, rerender } = render(renderPagination(false)); + const button = container.querySelector( + '.rc-pagination-item-2 button', + ); + button.focus(); + expect(button).toHaveFocus(); + rerender(renderPagination(true)); + expect(button).toBeDisabled(); + await userEvent.tab({ shift }); + expect( + container + .querySelector('.rc-pagination') + .contains(document.activeElement), + ).toBe(false); + expect(onChange).not.toHaveBeenCalled(); + }, + ); + it.each([ ['prev', 49], ['next', 51],