refactor: InputNumber semantic refactor #726
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 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. Walkthrough将 InputNumber 组件从内部语义上下文架构重构为公开的 forwardRef API；将样式类名由 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户代码
participant InputNumber as InputNumber (forwardRef)
participant Ref as InputNumberRef
participant DOM as DOM 元素
User->>InputNumber: <InputNumber ref={ref} .../>
InputNumber->>DOM: 渲染 root, input, prefix, suffix, actions(action-up, action-down)
User->>Ref: ref.current.focus(options)
Ref->>DOM: 调用 focus -> 聚焦 input
User->>Ref: ref.current.nativeElement
Ref->>User: 返回 HTMLElement
Estimated code review effort🎯 3 (中等复杂度) | ⏱️ ~25 分钟 需要额外关注的文件/区域：
Possibly related PRs
Suggested reviewers
诗歌
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 significantly refactors 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
docs/demo/prefix-suffix.tsx
Outdated
| const [value, setValue] = React.useState<string | number>(100); | ||
|
|
||
| const onChange = (val: number) => { | ||
| console.warn('onChange:', val, typeof val); |
There was a problem hiding this comment.
在示例代码中使用 console.warn 可能会让使用者误以为这里存在需要注意的问题。如果只是为了演示 onChange 事件，建议移除此行，或者使用 console.log。为了保持示例代码的简洁，最好是移除掉。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/InputNumber.tsx (2)
78-80:addonBefore/After目前被静默忽略，会导致破坏性回归重构移除了
BaseInput包裹后，addonBefore/addonAfter虽然还在类型上暴露，但在新的渲染树里已经没有任何地方去渲染它们，用户传值会被悄悄吃掉。建议要么恢复对应 DOM（保持向后兼容），要么删掉这两个 props 并在文档里明确这是一次 breaking change，否则就是实打实的回归。
687-761: 删除整块遗留的旧实现注释旧实现已经完全被新的 forwardRef 版本替换，继续保留这百行注释只会干扰后续维护。建议在合并前直接删掉整块遗留代码。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/__snapshots__/baseInput.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
assets/index.less(9 hunks)docs/demo/prefix-suffix.tsx(1 hunks)docs/example.md(1 hunks)package.json(1 hunks)src/InputNumber.tsx(2 hunks)src/SemanticContext.ts(0 hunks)src/StepHandler.tsx(3 hunks)tests/click.test.tsx(8 hunks)tests/decimal.test.tsx(3 hunks)tests/formatter.test.tsx(2 hunks)tests/github.test.tsx(3 hunks)tests/input.test.tsx(1 hunks)tests/longPress.test.tsx(3 hunks)tests/props.test.tsx(9 hunks)tests/semantic.test.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/SemanticContext.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/formatter.test.tsxtests/props.test.tsxtests/click.test.tsxtests/github.test.tsxtests/longPress.test.tsxtests/input.test.tsxtests/semantic.test.tsxtests/decimal.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/formatter.test.tsxtests/props.test.tsxtests/click.test.tsxtests/input.test.tsxdocs/demo/prefix-suffix.tsxtests/semantic.test.tsxsrc/InputNumber.tsxtests/decimal.test.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:
tests/click.test.tsxdocs/demo/prefix-suffix.tsxtests/semantic.test.tsxsrc/InputNumber.tsx
🧬 Code graph analysis (1)
src/InputNumber.tsx (4)
docs/demo/focus.tsx (1)
inputRef(6-28)src/hooks/useCursor.ts (1)
useCursor(7-76)src/utils/numberUtil.ts (1)
getDecupleSteps(3-10)src/StepHandler.tsx (1)
StepHandler(26-110)
🪛 GitHub Actions: ✅ test
src/InputNumber.tsx
[error] 19-19: Cannot find module '@rc-component/util/lib/DOM/focus' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (18)
docs/example.md (1)
12-14: LGTM!新增的 prefix-suffix 示例文档结构正确，格式与其他示例保持一致。
assets/index.less (4)
26-44: LGTM!类名从
&-handler重构为&-action是合理的语义化改进，更准确地描述了这些元素的用途。内部元素-inner的重命名也保持一致。
79-106: LGTM!
&-actions容器和&-action-up/down按钮的类名更新与整体重构保持一致。使用content: '+'和content: '-'在:after伪元素中渲染操作符号是一个简洁的实现。
137-150: LGTM!Spinner 模式下的 action 选择器更新正确，保持了与新命名约定的一致性。
4-5: 布局变更验证已完成，无需修改经过完整搜索验证：
- assets/index.less 第 4-5 行的变更已确认
- 全库搜索未发现任何
inline-block依赖或样式覆盖flex-wrap: nowrap仅在该文件出现 2 次（主组件和内部 spinner 子选择器）- 现有测试（10+ 测试文件）通过 CSS 类选择器访问元素，不依赖特定 display 属性值
- 无外部组件对旧 display 属性有依赖
该变更不会破坏现有样式，可安全合并。
tests/input.test.tsx (1)
237-237: LGTM!测试选择器从
.rc-input-number-affix-wrapper更新为.rc-input-number，正确反映了移除 rc-input 包裹器后的新 DOM 结构。这与 PR 的目标（去除多余的 DOM 结构）一致。tests/formatter.test.tsx (2)
18-22: LGTM!测试选择器从
.rc-input-number-handler-up/down更新为.rc-input-number-action-up/down，与样式表中的类名重构保持一致。测试逻辑和断言保持不变。
97-97: LGTM!选择器更新正确。
tests/github.test.tsx (3)
71-71: LGTM!Issue 222 测试用例中的选择器更新正确，从
.rc-input-number-handler-up改为.rc-input-number-action-up。
124-124: LGTM!长按测试用例中的选择器更新正确。
522-522: LGTM!全局修改测试用例中的选择器更新正确。
tests/longPress.test.tsx (3)
19-19: LGTM!长按向上按钮测试中的选择器更新正确。
29-29: LGTM!长按向下按钮测试中的选择器更新正确。
40-60: LGTM!Safari 事件顺序模拟测试中的所有选择器更新正确，从
.rc-input-number-handler-up改为.rc-input-number-action-up。测试逻辑和时序保持不变。tests/decimal.test.tsx (3)
18-21: LGTM!整数步长递增/递减小数 InputNumber 测试中的选择器更新正确，从
.rc-input-number-handler-up/down改为.rc-input-number-action-up/down。
47-47: LGTM!小数值和步长测试中的选择器更新正确。
63-63: LGTM!空值小步长测试中的选择器更新正确。
package.json (1)
50-50: 依赖版本升级已验证通过，无破坏性变更验证结果确认：
- 版本有效性：@rc-component/util v1.4.0 已在 npm 上发布，版本号正确
- 新增功能已集成：src/InputNumber.tsx 第 19 行已正确导入并使用新增的
triggerFocus和InputFocusOptionsfocus 工具函数- 向后兼容性：现有 API（KeyCode、raf、warning、useLayoutUpdateEffect、proxyObject、useEvent 等）在所有使用处保持可用，无不兼容问题
- 测试覆盖：焦点功能已有专项测试（tests/focus.test.tsx）覆盖
- 破坏性变更：未发现公开的破坏性变更文档
升级安全可行。
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/InputNumber.tsx (1)
678-682: 修复导出组件的 ref 类型定义第 680 行的 ref 类型应为
InputNumberRef而非React.Ref<HTMLInputElement>。当前的类型断言与第 111 行的React.forwardRef<InputNumberRef, InputNumberProps>不一致，导致消费者无法访问focus(options?)、blur()和nativeElement等自定义方法。需要将类型从
React.Ref<HTMLInputElement>更新为InputNumberRef。
🧹 Nitpick comments (1)
src/InputNumber.tsx (1)
292-293: 移除 useMemo 依赖数组中多余的 precision
getDecimalIfValidate函数不使用precision参数，但依赖数组中包含了它。这会导致precision变化时不必要的重新计算。应用此 diff：
- const maxDecimal = React.useMemo(() => getDecimalIfValidate(max), [max, precision]); - const minDecimal = React.useMemo(() => getDecimalIfValidate(min), [min, precision]); + const maxDecimal = React.useMemo(() => getDecimalIfValidate(max), [max]); + const minDecimal = React.useMemo(() => getDecimalIfValidate(min), [min]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/InputNumber.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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:
src/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
📚 Learning: 2024-09-29T07:54:50.892Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: tests/validator.test.tsx:34-83
Timestamp: 2024-09-29T07:54:50.892Z
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:
src/InputNumber.tsx
🧬 Code graph analysis (1)
src/InputNumber.tsx (4)
docs/demo/focus.tsx (1)
inputRef(6-28)src/hooks/useCursor.ts (1)
useCursor(7-76)src/utils/numberUtil.ts (1)
getDecupleSteps(3-10)src/StepHandler.tsx (1)
StepHandler(26-110)
🔇 Additional comments (2)
src/InputNumber.tsx (2)
111-171: Ref API 设计合理forwardRef 和 useImperativeHandle 的实现正确，正确暴露了
focus、blur和nativeElement接口。通过 proxyObject 代理 inputRef 并扩展自定义方法的方式很优雅。
617-677: 渲染结构重构合理新的渲染结构成功移除了 rc-input 包裹层，简化了 DOM 结构。关键改进：
- 使用 root div 直接包裹所有元素
- prefix/suffix 的条件渲染使用
!== undefined检查，避免 falsy 值被意外跳过- 'spinner' 和 'input' 两种模式的控件布局清晰
- 所有语义化 className 和 style 正确应用
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #726 +/- ##
==========================================
- Coverage 96.01% 95.23% -0.78%
==========================================
Files 7 6 -1
Lines 301 294 -7
Branches 81 82 +1
==========================================
- Hits 289 280 -9
- Misses 12 14 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
去除 rc-input 包裹从而移除多余的 dom 结构
Summary by CodeRabbit
新特性
重构
测试
杂项