-
Notifications
You must be signed in to change notification settings - Fork 192
feat: triggerFocus support #709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough在 Changes
代码审查工作量估计🎯 2 (简单) | ⏱️ ~10 分钟
诗句
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 @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! 此拉取请求引入了一个新的 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
这个 PR 将 triggerFocus 函数移入 rc-util 中,以便在 rc-input, rc-textarea, rc-input-number 中共享,从而实现解耦,这是一个很好的重构。代码实现基本正确,并且包含了相应的单元测试。我只提一个关于代码可读性和健壮性的小建议。
| switch (cursor) { | ||
| case 'start': | ||
| element.setSelectionRange(0, 0); | ||
| break; | ||
|
|
||
| case 'end': | ||
| element.setSelectionRange(len, len); | ||
| break; | ||
|
|
||
| default: | ||
| element.setSelectionRange(0, len); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switch 语句使用 default 来处理 cursor 为 'all' 的情况。虽然功能上是正确的,但这不够明确。如果未来 cursor 的类型增加了新的选项,它们会意外地执行 default 的逻辑(即全选文本),这可能不是期望的行为。建议明确地处理 'all' 情况,让代码更具可读性和健壮性。
switch (cursor) {
case 'start':
element.setSelectionRange(0, 0);
break;
case 'end':
element.setSelectionRange(len, len);
break;
case 'all':
element.setSelectionRange(0, len);
break;
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #709 +/- ##
==========================================
+ Coverage 90.28% 90.47% +0.19%
==========================================
Files 37 37
Lines 885 903 +18
Branches 301 311 +10
==========================================
+ Hits 799 817 +18
Misses 84 84
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Dom/focus.ts (1)
115-115: 建议解构 option 参数以避免传递额外属性。当前实现将整个
option对象(包含自定义的cursor属性)传递给element.focus()。虽然浏览器会忽略额外的属性,但更规范的做法是仅传递标准的FocusOptions属性。应用此 diff 来改进实现:
- element.focus(option); + const { cursor, ...focusOptions } = option || {}; + element.focus(focusOptions); // Selection content - const { cursor } = option || {}; if (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Dom/focus.ts(1 hunks)tests/focus.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/focus.test.ts (1)
src/Dom/focus.ts (1)
triggerFocus(109-139)
⏰ 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: test / react component workflow
🔇 Additional comments (2)
src/Dom/focus.ts (1)
101-103: 接口设计合理!
InputFocusOptions接口扩展了标准的FocusOptions,添加了cursor选项,设计清晰且类型安全。tests/focus.test.ts (1)
35-58: 测试覆盖全面!新增的测试用例全面验证了
triggerFocus函数的三种光标位置选项('start'、'end'、'all'),并正确验证了focus和setSelectionRange的调用。测试实现遵循最佳实践,包括正确的 spy 清理。
将函数从 rc-input 挪过来,rc-input-number 本身与 rc-input 解耦
Summary by CodeRabbit
新功能
测试