feat: improve focus behavior #681
Conversation
Walkthrough此拉取请求引入了对 Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)src/InputNumber.tsx (2)
建议移除冗余实现 由于 React.useImperativeHandle(ref, () =>
proxyObject(inputFocusRef.current, {
- focus,
nativeElement: holderRef.current.nativeElement || inputNumberDomRef.current,
}),
);
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@afc163 @zombieJ @li-jia-nan 三位大佬有空看看这个pr |
|
对于focus的单测，我是照搬了rc-input，但是都挂了，大佬帮忙看一下是哪里出了问题 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
docs/demo/focus.tsx (2)
6-8: 建议为组件添加显式名称为了提高调试体验和组件堆栈跟踪的可读性，建议给组件一个明确的名称，而不是使用匿名函数。
-export default () => { +const FocusDemo = () => { const inputRef = React.useRef<InputNumberRef>(null); return ( // ... ); }; + +export default FocusDemo;
11-11: 建议增强无障碍性支持虽然已经添加了 aria-label，但建议添加更多的无障碍性支持：
- 为输入框添加描述性标签
- 考虑添加 aria-describedby 属性
-<InputNumber aria-label="focus example" value={10} style={{ width: 100 }} ref={inputRef} /> +<InputNumber + aria-label="focus example" + aria-describedby="focus-help-text" + value={10} + style={{ width: 100 }} + ref={inputRef} +/> +<div id="focus-help-text" className="sr-only"> + 这是一个数字输入框，可以通过下方按钮控制焦点位置 +</div>tests/focus.test.tsx (1)
12-29: 建议添加更详细的类型注解测试设置的整体结构很好，但可以通过添加更明确的类型定义来提高代码的可维护性。
建议如下修改：
- let focus: ReturnType<typeof jest.fn>; - let setSelectionRange: ReturnType<typeof jest.fn>; + let focus: jest.Mock<void, [options?: FocusOptions]>; + let setSelectionRange: jest.Mock<void, [start: number, end: number]>;docs/api.md (2)
192-204: 建议完善代码示例的健壮性和文档性代码示例展示了基本用法，但建议做如下改进：
- 添加 null 检查以提高代码健壮性
- 补充 focus 方法的可选参数说明
建议按如下方式优化代码示例：
const inputRef = useRef<InputNumberRef>(null); useEffect(() => { - inputRef.current.focus(); // the input will get focus - inputRef.current.blur(); // the input will lose focus - console.log(inputRef.current.input); // The origin input element + if (inputRef.current) { + // 支持设置选区范围，例如：{ cursor: 'start' | 'end' | 'all' } + inputRef.current.focus({ cursor: 'end' }); + inputRef.current.blur(); + console.log(inputRef.current.input); + } }, []);
206-210: 建议丰富 API 表格的描述信息当前的属性描述较为简单，建议补充更多使用细节：
| Property | Type | Description | | -------- | --------------------------------------- | --------------------------------- | -| focus | `(options?: InputFocusOptions) => void` | The input get focus when called | -| blur | `() => void` | The input loses focus when called | -| input | `HTMLInputElement \| null` | The origin input element | +| focus | `(options?: InputFocusOptions) => void` | 使输入框获得焦点。options 可选参数支持设置光标位置和选区范围 | +| blur | `() => void` | 使输入框失去焦点 | +| input | `HTMLInputElement \| null` | 原生输入框 DOM 元素，可用于直接操作输入框 |tests/input.test.tsx (1)
Line range hint
236-256: 测试结构合理，建议补充边缘场景当前测试结构清晰，分别验证了基础场景和带有后缀的场景。不过建议考虑添加以下场景的测试：
- 禁用状态下的引用行为
- 只读状态下的引用行为
- 组件销毁时的引用清理
需要我帮您生成这些额外测试用例的代码吗？
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
docs/api.md(1 hunks)docs/demo/focus.tsx(1 hunks)docs/example.md(1 hunks)src/InputNumber.tsx(2 hunks)tests/focus.test.tsx(1 hunks)tests/input.test.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/example.md
🔇 Additional comments (5)
docs/demo/focus.tsx (1)
1-5: 导入声明看起来很完整！
导入语句包含了所有必要的依赖项，并且正确地导入了类型定义。
tests/focus.test.tsx (1)
1-10: 代码结构清晰，复用性好！
辅助函数 getInputRef 很好地封装了组件初始化逻辑，便于测试用例重用。导入的依赖项选择恰当，符合测试需求。
docs/api.md (1)
189-191: 文档结构清晰，位置恰当！
新增的 inputRef 章节放置在主要属性表格之后，结构合理。
tests/input.test.tsx (1)
227-235: 新增的输入引用测试用例完善了组件的测试覆盖率
测试用例验证了 InputNumberRef 的两个关键属性：
input属性正确指向输入元素nativeElement属性正确指向根元素
这与 PR 的目标（使 rc-input-number 的 ref 行为与 rc-input 保持一致）完全吻合。
src/InputNumber.tsx (1)
27-35: 接口定义完善且类型安全！
新增的接口方法与 rc-input 保持一致，包含了完整的类型定义，这样的改动有助于提供一致的用户体验。
| <div style={{ marginTop: 10 }}> | ||
| <button type="button" onClick={() => inputRef.current?.focus({ cursor: 'start' })}> | ||
| focus at start | ||
| </button> | ||
| <button type="button" onClick={() => inputRef.current?.focus({ cursor: 'end' })}> | ||
| focus at end | ||
| </button> | ||
| <button type="button" onClick={() => inputRef.current?.focus({ cursor: 'all' })}> | ||
| focus to select all | ||
| </button> | ||
| <button type="button" onClick={() => inputRef.current?.focus({ preventScroll: true })}> | ||
| focus prevent scroll | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议改进按钮组的样式和结构
当前按钮组的布局比较简单，建议使用更规范的样式和结构：
- 使用语义化的按钮组结构
- 添加适当的间距
- 提供更好的视觉反馈
-<div style={{ marginTop: 10 }}>
+<div
+ style={{
+ marginTop: 10,
+ display: 'flex',
+ gap: 8,
+ flexWrap: 'wrap'
+ }}
+ role="group"
+ aria-label="焦点控制按钮组"
+>
<button
type="button"
+ style={{
+ padding: '4px 8px',
+ borderRadius: 4,
+ border: '1px solid #d9d9d9'
+ }}
onClick={() => inputRef.current?.focus({ cursor: 'start' })}
>
focus at start
</button>
// 对其他按钮应用相同的样式...
</div>Committable suggestion was skipped due to low confidence.
| it('start', () => { | ||
| const input = getInputRef(); | ||
| input.current?.focus({ cursor: 'start' }); | ||
|
|
||
| expect(focus).toHaveBeenCalled(); | ||
| expect(setSelectionRange).toHaveBeenCalledWith(expect.anything(), 0, 0); | ||
| }); | ||
|
|
||
| it('end', () => { | ||
| const input = getInputRef(); | ||
| input.current?.focus({ cursor: 'end' }); | ||
|
|
||
| expect(focus).toHaveBeenCalled(); | ||
| expect(setSelectionRange).toHaveBeenCalledWith(expect.anything(), 5, 5); | ||
| }); | ||
|
|
||
| it('all', () => { | ||
| const input = getInputRef(); | ||
| input.current?.focus({ cursor: 'all' }); | ||
|
|
||
| expect(focus).toHaveBeenCalled(); | ||
| expect(setSelectionRange).toHaveBeenCalledWith(expect.anything(), 0, 5); | ||
| }); | ||
|
|
||
| it('disabled should reset focus', () => { | ||
| const { container, rerender } = render(<InputNumber prefixCls="rc-input-number" />); | ||
| const input = container.querySelector('input')!; | ||
|
|
||
| fireEvent.focus(input); | ||
| expect(container.querySelector('.rc-input-number-focused')).toBeTruthy(); | ||
|
|
||
| rerender(<InputNumber prefixCls="rc-input-number" disabled />); | ||
| fireEvent.blur(input); | ||
|
|
||
| expect(container.querySelector('.rc-input-number-focused')).toBeFalsy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议补充更多测试用例
现有的测试用例覆盖了基本的焦点行为，但还可以增加以下测试场景：
blur()方法的测试select()方法的测试- 边界情况的处理（如空值时的光标位置）
建议添加如下测试用例：
it('blur', () => {
const input = getInputRef();
input.current?.focus();
input.current?.blur();
expect(blur).toHaveBeenCalled();
});
it('select', () => {
const input = getInputRef();
input.current?.select();
expect(focus).toHaveBeenCalled();
expect(setSelectionRange).toHaveBeenCalledWith(expect.anything(), 0, 5);
});
本地跑不通，但是ci竟然跑通了🤔这河里吗😶 |
|
肯定是有一些patch依赖不一致，删除lock和node_modules重新安装就一样了 |
原来如此，get✅ |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
docs/api.md (2)
192-203: 代码示例需要改进建议对代码示例进行以下改进：
- 添加必要的导入声明
- 增加错误处理
- 补充更详细的注释
建议按如下方式修改代码示例：
```tsx | pure -import InputNumber, { InputNumberRef } from 'rc-input-number'; +import { useEffect, useRef } from 'react'; +import InputNumber, { InputNumberRef } from 'rc-input-number'; const inputRef = useRef<InputNumberRef>(null); useEffect(() => { - inputRef.current.focus(); // the input will get focus - inputRef.current.blur(); // the input will lose focus + // 安全地调用 focus/blur 方法 + if (inputRef.current) { + // 使输入框获得焦点 + inputRef.current.focus(); + + // 使输入框失去焦点 + inputRef.current.blur(); + } }, []); -// .... + <InputNumber ref={inputRef} />;
205-208: API 表格需要补充更多信息当前的 API 表格缺少一些重要信息：
- focus 方法的 options 参数说明
- 返回值类型信息
- 更详细的使用说明
建议扩展表格内容如下：
| Property | Type | Description | | -------- | --------------------------------------- | --------------------------------- | -| focus | `(options?: InputFocusOptions) => void` | The input get focus when called | -| blur | `() => void` | The input loses focus when called | +| focus | `(options?: InputFocusOptions) => void` | 使输入框获得焦点。options 参数可选，类型与 DOM 的 focus() 方法参数一致，可用于控制焦点行为（如 preventScroll） | +| blur | `() => void` | 使输入框失去焦点。返回 void |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
docs/api.md(1 hunks)src/InputNumber.tsx(2 hunks)
🔇 Additional comments (1)
src/InputNumber.tsx (1)
27-28: 接口定义符合 rc-input 规范
InputNumberRef 接口新增的 focus 和 blur 方法与 rc-input 保持一致，这有助于统一组件行为。
src/InputNumber.tsx
Outdated
| focus, | ||
| blur: () => { | ||
| inputFocusRef.current?.blur(); | ||
| }, | ||
| setSelectionRange: ( | ||
| start: number, | ||
| end: number, | ||
| direction?: 'forward' | 'backward' | 'none', | ||
| ) => { | ||
| inputFocusRef.current?.setSelectionRange(start, end, direction); | ||
| }, | ||
| select: () => { | ||
| inputFocusRef.current?.select(); | ||
| }, | ||
| input: inputFocusRef.current, |
There was a problem hiding this comment.
建议优化 focus 和 blur 方法的空值检查
根据之前的讨论，建议仅为 focus 和 blur 方法添加空值检查，以提高代码的健壮性。其他方法可以让用户通过 nativeElement 自行处理。
建议按如下方式修改：
- focus,
+ focus: (option?: InputFocusOptions) => {
+ if (!inputFocusRef.current) return;
+ triggerFocus(inputFocusRef.current, option);
+ },
blur: () => {
+ if (!inputFocusRef.current) return;
inputFocusRef.current?.blur();
},
setSelectionRange: (
start: number,
end: number,
direction?: 'forward' | 'backward' | 'none',
) => {
inputFocusRef.current?.setSelectionRange(start, end, direction);
},
select: () => {
inputFocusRef.current?.select();
},Committable suggestion was skipped due to low confidence.
src/InputNumber.tsx
Outdated
| blur: () => { | ||
| inputFocusRef.current?.blur(); | ||
| }, | ||
| setSelectionRange: ( |
src/InputNumber.tsx
Outdated
| @@ -660,6 +662,21 @@ const InputNumber = React.forwardRef<InputNumberRef, InputNumberProps>((props, r | |||
|
|
|||
| React.useImperativeHandle(ref, () => | |||
| proxyObject(inputFocusRef.current, { | |||
| focus, | |||
| blur: () => { | |||
There was a problem hiding this comment.
proxyObject 本来就支持 blur 的，这边不需要额外处理哈~
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #681 +/- ##
==========================================
+ Coverage 95.28% 95.36% +0.07%
==========================================
Files 6 7 +1
Lines 297 302 +5
Branches 78 78
==========================================
+ Hits 283 288 +5
Misses 14 14 ☔ View full report in Codecov by Sentry. |
rc-input-number的ref属性与rc-input对齐，支持了：
另外：
Summary by CodeRabbit
新功能
InputNumber组件的inputRef功能说明，包含focus和blur方法。InputNumber组件进行程序化聚焦控制。测试
InputNumber组件聚焦和选择范围的单元测试。