Add option to specify the input type of QuickJumper #664
Conversation
|
@ansabamane is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 概览将快速跳转输入框的类型从文本改为数字，并相应更新测试用例以验证新的输入行为和值处理逻辑。 变更
代码审查工作量🎯 2 (简单) | ⏱️ ~10 分钟
诗
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ansabamane, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new configuration option for the QuickJumper component, allowing developers to explicitly define the input type as either 'text' or 'number'. This change aims to improve the user experience by preventing non-numeric character entry in the page navigation input field, thereby reducing potential confusion and input errors. The update includes modifications to type definitions, component logic to handle the new prop, and corresponding documentation examples. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by allowing the input type of the QuickJumper to be specified, enhancing user experience by preventing non-numeric inputs when desired. The changes are well-structured, including updates to interfaces, component props, and the addition of relevant examples. However, I've identified a couple of areas for improvement in src/Pagination.tsx. Firstly, the new inputType is not applied to the simple mode pager, leading to incomplete functionality. Secondly, there's an opportunity to enhance type safety by avoiding the use of as any. My specific comments below provide details and suggestions for these points.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Pagination.tsx (1)
394-404: 简单模式中的输入类型不一致。简单模式（simple mode）中的输入框硬编码为
type="text"（第 395 行），而普通模式通过 Options 组件支持配置inputType。这种不一致可能会让用户感到困惑，因为他们无法在简单模式下使用inputType: 'number'配置。建议在简单模式中也支持
inputType配置：{isReadOnly ? ( internalInputVal ) : ( <input - type="text" + type={inputType || 'text'} aria-label={locale.jump_to} value={internalInputVal} disabled={disabled} onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} onChange={handleKeyUp} onBlur={handleBlur} size={3} /> )}这样可以确保两种模式下的行为一致。
🧹 Nitpick comments (1)
src/Options.tsx (1)
163-163: 考虑 HTML input type="number" 的用户体验影响。需要注意 HTML
<input type="number">的一些特性：
- 允许输入 'e', '+', '-', '.' 等字符（科学计数法）
- 在某些浏览器中会显示上下箭头控件
- 输入非数字字符时，
e.target.value可能返回空字符串您可能需要考虑以下替代方案以获得更好的用户体验：
- 使用
type="text"配合inputMode="numeric"和pattern="[0-9]*"来在移动设备上显示数字键盘，同时避免 type="number" 的副作用- 或者在文档中说明 type="number" 的行为特性
是否需要我生成一个替代实现方案的示例？
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/examples/jumper.tsx(1 hunks)docs/examples/jumperWithGoButton.tsx(2 hunks)src/Options.tsx(3 hunks)src/Pagination.tsx(2 hunks)src/interface.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
docs/examples/jumper.tsx (1)
docs/examples/utils/commonUtil.tsx (1)
PaginationWithSizeChanger(33-45)
🔇 Additional comments (8)
docs/examples/jumper.tsx (1)
40-50: 示例代码正确展示了新功能。这个新示例有效地演示了
inputType: 'number'的用法，与文件中其他示例的结构保持一致。src/Options.tsx (2)
30-30: 类型定义正确。新增的
inputType属性类型定义合理，支持 'text' 和 'number' 两种值。
48-48: 默认值保持向后兼容。将
inputType默认设置为 'text' 确保了向后兼容性，这是一个明智的选择。src/interface.ts (2)
23-26: 类型定义清晰准确。新增的
QuickJumperOptions类型定义结构良好，清楚地表达了快速跳转功能的配置选项，包括可选的goButton和inputType字段。
49-49: 改进了类型安全性。将
showQuickJumper的类型从boolean | object更新为boolean | QuickJumperOptions提供了更好的类型安全性和 IDE 自动补全支持，这是一个很好的改进。docs/examples/jumperWithGoButton.tsx (2)
25-27: 格式化改进了可读性。将
showQuickJumper对象展开为多行格式提高了代码的可读性，特别是当添加更多属性时。
41-47: 示例有效展示了组合用法。这个新示例很好地展示了
goButton和inputType: 'number'的组合使用，为用户提供了实用的参考。src/Pagination.tsx (1)
600-600: 正确传递了 inputType 属性。
inputType属性被正确地传递给 Options 组件，实现了普通模式下的功能。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/jumper.test.tsx (3)
276-293: 增强测试以验证 input 属性和完整的导航流程。当前测试仅检查输入值是否被接受，但未验证：
- input 元素是否实际具有
type="number"属性- 输入有效数字后的页面导航行为（例如按 Enter 键或点击 go 按钮）
建议增强测试：
it('should accept only number', () => { const onChange = jest.fn(); const { container } = render( <Pagination onChange={onChange} defaultCurrent={1} total={95} pageSize={10} showQuickJumper={{ inputType: 'number' }} />, ); const quickJumper = container.querySelector( '.rc-pagination-options-quick-jumper', ); const input = quickJumper.querySelector('input'); + expect(input).toHaveAttribute('type', 'number'); fireEvent.change(input, { target: { value: '42' } }); expect(input.value).toBe('42'); + fireEvent.keyUp(input, { key: 'Enter', keyCode: 13, which: 13 }); + expect(onChange).toHaveBeenCalledWith(5, 10); });注意：预期页码为 5，因为 total=95, pageSize=10，第 42 页超出范围（最多 10 页），应该跳转到有效页码。请根据实际实现调整预期值。
295-312: 验证 type 属性以确保测试的是组件行为而非浏览器行为。当前测试依赖浏览器对
type="number"的原生验证行为（浏览器会自动拒绝非数字输入）。为确保测试验证的是组件正确设置了 input 类型，应显式检查 type 属性。应用此差异以增强测试：
it('should not accept non-number', () => { const onChange = jest.fn(); const { container } = render( <Pagination onChange={onChange} defaultCurrent={1} total={95} pageSize={10} showQuickJumper={{ inputType: 'number' }} />, ); const quickJumper = container.querySelector( '.rc-pagination-options-quick-jumper', ); const input = quickJumper.querySelector('input'); + expect(input).toHaveAttribute('type', 'number'); fireEvent.change(input, { target: { value: 'abc' } }); expect(input.value).toBe(''); });
275-313: 考虑添加额外的测试用例以提高覆盖率。当前测试套件验证了
inputType: 'number'的基本行为。为了更全面的测试覆盖，可以考虑添加：
- 默认行为测试（不指定
inputType时应默认为 'text'）- 显式
inputType: 'text'的测试- 边缘情况（如小数、负数等）
示例测试：
it('should default to text input type', () => { const { container } = render( <Pagination defaultCurrent={1} total={95} showQuickJumper /> ); const input = container.querySelector('.rc-pagination-options-quick-jumper input'); expect(input).toHaveAttribute('type', 'text'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/__snapshots__/demo.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
docs/examples/jumper.tsx(1 hunks)tests/jumper.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/examples/jumper.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #664 +/- ##
=======================================
Coverage 99.03% 99.03%
=======================================
Files 3 3
Lines 310 310
Branches 139 139
=======================================
Hits 307 307
Misses 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Options.tsx (1)
161-161: 实现与 PR 描述不一致，但考虑使用inputMode作为更优方案根据 PR 描述，原本计划添加一个可配置的选项来指定输入类型。然而，当前实现直接硬编码为
type="number"。虽然从过往评论来看，维护者和贡献者已就此方案达成一致，但 PR 描述需要更新以反映实际实现。关于
type="number"的一些注意事项：
- 允许输入
e、E（科学计数法）、+、-、.（小数点），这些对于页码输入并无意义- 在桌面端会显示上下调节按钮（spinners），可能影响用户体验
- 在移动端会显示数字键盘（这是优点）
建议的替代方案：
考虑使用
inputMode="numeric"配合type="text"和pattern属性，这样可以：
- 在移动端显示数字键盘
- 避免桌面端的调节按钮
- 通过
pattern严格限制只能输入纯数字- 不允许
e、E、+、-、.等字符- type="number" + type="text" + inputMode="numeric" + pattern="[0-9]*"这种方式在保持移动端数字键盘优势的同时，提供了更精确的输入控制。不过鉴于当前方案已获维护者认可，此建议可作为未来优化考虑。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/__snapshots__/demo.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/options.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/simple.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/Options.tsx(1 hunks)tests/jumper.test.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/jumper.test.tsx
2 participants
What problem does this PR solve?
Related to ant-design/ant-design#55637
Currently, the QuickJumper page input fields accept any characters (letters, symbols, etc.), which can cause confusion when users accidentally type non-numeric characters.
What is changed and how it works?
Summary by CodeRabbit
发行说明