-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
fix: getInputElement为textarea时,上下键用默认行为 #1072
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改涉及 Changes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
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 (
|
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
🧹 Outside diff range and nitpick comments (1)
src/Selector/index.tsx (1)
Line range hint
142-164
: 逻辑更新正确处理了文本区域的箭头键行为这些更改很好地解决了问题 #50935 中描述的行为。现在,当输入元素是文本区域且下拉列表关闭时,箭头键可以正常用于文本导航。这提高了组件的可用性和一致性。
建议优化:考虑将
isTextAreaElement
的检查移到函数开头,这样可以避免在每次键盘事件中重复进行这个检查。您可以考虑如下重构:
const onInternalInputKeyDown: React.KeyboardEventHandler< HTMLInputElement | HTMLTextAreaElement > = (event) => { const { which } = event; + const isTextAreaElement = inputRef.current instanceof HTMLTextAreaElement; - // Compatible with multiple lines in TextArea - const isTextAreaElement = inputRef.current instanceof HTMLTextAreaElement; if (!isTextAreaElement && open && (which === KeyCode.UP || which === KeyCode.DOWN)) { event.preventDefault(); } // ... (rest of the function) // Move within the text box if ( isTextAreaElement && !open && ~[KeyCode.UP, KeyCode.DOWN, KeyCode.LEFT, KeyCode.RIGHT].indexOf(which) ) { return; } // ... (rest of the function) };这个小改动可以稍微提高性能,特别是在频繁触发键盘事件的情况下。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/Selector/index.tsx (2 hunks)
- tests/Select.test.tsx (1 hunks)
🔇 Additional comments (3)
src/Selector/index.tsx (2)
137-139
: 类型定义更新有助于支持多行输入这个更改扩展了
onInternalInputKeyDown
事件处理器的类型定义,使其同时支持HTMLInputElement
和HTMLTextAreaElement
。这是一个很好的改进,因为它允许组件在处理单行输入和多行文本区域时使用相同的事件处理逻辑。
Line range hint
137-164
: 总体评价:改进了文本区域的处理,提高了组件的可用性这些更改很好地解决了 AutoComplete 组件中使用文本区域时的箭头键导航问题。主要改进包括:
- 扩展了事件处理器的类型定义,支持both
HTMLInputElement
和HTMLTextAreaElement
。- 为文本区域输入添加了特殊的逻辑处理,允许在下拉列表关闭时使用箭头键进行文本导航。
这些修改提高了组件的灵活性和用户体验,特别是在处理多行输入时。代码质量良好,符合现有的编码风格。
建议:考虑添加单元测试来验证这些新的行为,确保在未来的更改中不会意外破坏这些功能。
tests/Select.test.tsx (1)
978-1050
: 代码看起来不错,但有一些改进的空间。这个测试用例很好地覆盖了在Select组件中使用textarea的关键行为。然而,我有以下几点建议来增强测试的清晰度和完整性:
考虑添加对模拟函数(如onKeyDown、onChange等)被调用的明确断言。这将确保事件处理器按预期工作。
为某些操作添加注释,解释它们的目的。例如,mousedown事件的作用可能不是立即明显的。
使用更具描述性的变量名。例如,
start
和end
可以改为cursorStart
和cursorEnd
。考虑将测试拆分为多个较小的测试用例,每个用例专注于特定的行为(如换行、光标移动等)。这将提高测试的可读性和可维护性。
添加更多的断言来验证中间状态,而不仅仅是最终状态。这将有助于在失败时更容易定位问题。
总的来说,这是一个很好的测试用例,只需要一些小的调整就能变得更加强大和清晰。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1072 +/- ##
=======================================
Coverage 98.22% 98.23%
=======================================
Files 39 39
Lines 1467 1473 +6
Branches 411 444 +33
=======================================
+ Hits 1441 1447 +6
Misses 26 26 ☔ View full report in Codecov by Sentry. |
用例要补一下。 |
if (which === KeyCode.UP || which === KeyCode.DOWN) { | ||
// Compatible with multiple lines in TextArea | ||
const isTextAreaElement = inputRef.current instanceof HTMLTextAreaElement; | ||
if (!isTextAreaElement && open && (which === KeyCode.UP || which === KeyCode.DOWN)) { |
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.
open 且显示 not found 的时候,也可以走默认行为。
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.
测试 open 且显示 not found 的时候会走默认行为
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.
只有textarea时,才需要处理上下的默认行为,这的判断只要是textarea就不会阻止默认行为。
如果是 (单行&open¬ found),按上下只能到行首和行尾,需要处理这种吗。
使用多行文本,listbox未展示时,上下键用默认行为
fix ant-design/ant-design#50935
Summary by CodeRabbit
新功能
测试