fix: Click should focus input #727
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 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. 演练该变更扩展了 InputNumber 组件的公开 API，显式暴露并转发更多输入与鼠标事件属性（包括 onMouse* 系列、value、defaultValue、onInput、onChange、prefix、suffix、disabled、readOnly 等），并添加了内部鼠标按下处理以在点击根容器或前缀/后缀时聚焦输入，同时新增相应测试用例覆盖前缀点击聚焦行为。 变更
序列图sequenceDiagram
participant User
participant Prefix as 前缀/子元素
participant Root as InputNumber 根容器
participant Input as 底层 <input>
User->>Prefix: 鼠标按下 (mouseDown)
Prefix->>Root: 触发根容器 onMouseDown
Note over Root: onInternalMouseDown 处理\n(调用 focus 并转发事件)
Root->>Input: 调用 focus()
Root->>User: 调用用户的 onMouseDown 回调（若提供）
Note over Input: 输入获得焦点
预估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟
可能相关的 PR
诗
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 @zombieJ, 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 addresses an issue where clicking on non-input areas of the 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #727 +/- ##
==========================================
+ Coverage 95.23% 95.30% +0.06%
==========================================
Files 6 6
Lines 294 298 +4
Branches 81 83 +2
==========================================
+ Hits 280 284 +4
Misses 14 14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 fix to focus the input element when other parts of the InputNumber component, such as a prefix, are clicked. This is achieved by adding an onMouseDown handler to the component's wrapper element. The changes also include updating the component's props interface to correctly handle mouse events on the wrapper and adding a test case to verify the new behavior. My review includes one suggestion to prevent a potential runtime error by adding a null check.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/input.test.tsx (1)
241-249: 前缀点击聚焦用例合理，可考虑适当扩展覆盖面这个用例准确验证了修复目标：点击
prefix时，input会获得焦点，断言也很直接清晰，和组件当前行为匹配。可以作为后续优化考虑再补一两个类似用例，例如点击
suffix、整个.rc-input-number容器或步进按钮时同样会聚焦输入框，以便完整覆盖 “点击 wrapper 内任意区域应聚焦 input” 的约定。src/InputNumber.tsx (2)
55-83: onMouse 事件从 input 属性切到 div 属性，类型与行为都有所改变，请确认是否是预期 API*这里通过：
- 在
InputNumberProps中从InputHTMLAttributes<HTMLInputElement>里Omit掉所有 mouse 事件键（Line 55-70），- 再用
Pick<HTMLAttributes<HTMLDivElement>, ...>引入同名 mouse 事件键（Line 71-80），- 并在渲染时把这些 handler 直接挂到最外层
<div>上（Line 669-675），实际上把
onMouseDown/onClick/...等 props 的事件目标从 input 改成了 wrapper div：
- TypeScript 维度：这些回调现在推断为
React.MouseEvent<HTMLDivElement>，而不是之前的React.MouseEvent<HTMLInputElement>。如果用户在回调签名中显式写了event: React.MouseEvent<HTMLInputElement>，会出现类型不兼容或需要改动。- 运行时维度：点击 input 时事件依然会冒泡到 wrapper，因此回调仍会触发，但
event.currentTarget现在是 div 而不是 input；依赖event.currentTarget去读取value或做类型断言为HTMLInputElement的代码会被破坏。另一方面，点击 prefix/suffix 时，同一个回调现在也会触发，这属于行为增强。从“修复点击 prefix 聚焦 input”的 PR 目标来看，这个改动稍微超出了一点范围，而且可能带来微妙的破坏性变更。建议：
- 明确确认：是否希望今后
onMouseDown/onClick/...的语义就是「以 wrapper 为 currentTarget」？如果是，建议在 CHANGELOG 或文档里标明这个行为变更；- 如果希望兼顾原有 TS 类型/语义，可以考虑自定义一组更宽泛的 handler 类型（例如
React.MouseEventHandler<HTMLInputElement | HTMLDivElement>），或区分 input 与 wrapper 的事件 props（例如新增onWrapperMouseDown），从而减少对现有用户代码的冲击。Also applies to: 170-178, 669-675
597-605: onInternalMouseDown 聚焦逻辑工作正常，但可以更好地与 disabled/用户回调协作当前实现为：
- 在 wrapper 的
onMouseDown中调用onInternalMouseDown；- 若
event.target !== inputRef.current则调用inputRef.current.focus()，然后再调用用户传入的onMouseDown（Line 597-604）；...restProps只下发到<input>（Line 708），因此 input 本身不再直接接收onMouseDown等鼠标事件 props。行为上可以满足「点击 prefix（或其它子节点）时聚焦 input」的需求，但有两点可以考虑优化：
disabled 时是否应跳过聚焦
目前即便disabled为 true，点击 prefix 仍会尝试focus()。浏览器一般不会真正聚焦到 disabled input，但从语义上看可以在onInternalMouseDown里先判断disabled，避免多余操作，也更符合“完全不可交互”的直觉。用户无法通过 preventDefault 阻止聚焦
现在是先聚焦再调用onMouseDown?.(event)。如果有需求让外部通过event.preventDefault()来阻止自动聚焦，可以改为：
- 先调用用户的
onMouseDown；- 若
!event.defaultPrevented && event.target !== inputRef.current，再去focus()。这两点都不影响当前修复的正确性，只是让行为对外更可控、与常见交互约定更一致，可视情况选择是否调整。
Also applies to: 708-708
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/InputNumber.tsx(5 hunks)tests/input.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-08T21:56:37.546Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: tests/validator.test.tsx:34-83
Timestamp: 2024-10-08T21:56:37.546Z
Learning: In `tests/validator.test.tsx`, the test cases focus on the `validator` function. When the validator condition is not met, the input value should not change. The `formatter` is tested separately in another file.
Applied to files:
tests/input.test.tsx
📚 Learning: 2024-09-29T06:18:11.993Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: src/InputNumber.tsx:173-174
Timestamp: 2024-09-29T06:18:11.993Z
Learning: In `src/InputNumber.tsx`, within the `InternalInputNumber` component, the state variables `prevValueRef` and `inputValueRef` are typed as `string | number` to maintain consistency with existing code.
Applied to files:
tests/input.test.tsxsrc/InputNumber.tsx
📚 Learning: 2024-10-08T21:56:37.546Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: src/InputNumber.tsx:393-395
Timestamp: 2024-10-08T21:56:37.546Z
Learning: The `InputNumber` component does not use error states or messages; it is designed to prevent the value from updating if validation fails without displaying additional feedback to the user.
Applied to files:
src/InputNumber.tsx
Summary by CodeRabbit
发布说明
新功能
测试