-
-
Notifications
You must be signed in to change notification settings - Fork 465
fix: allow keyboard navigation on selected options when maxCount is reached #1088
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
Walkthrough此次更改在 Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/OptionList.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
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 ↗︎
|
@yoyo837 @li-jia-nan @afc163 @zombieJ 几位大佬有空看看,小改动 |
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)
tests/OptionList.test.tsx (1)
421-461
: 键盘导航测试逻辑完整,建议补充边缘场景!当前测试用例已经很好地覆盖了主要功能:
- 验证了向下键导航到已选选项
- 验证了在到达末尾时的环绕行为
- 验证了向上键导航功能
- 验证了在达到最大选择数量时无法激活未选中选项
建议考虑添加以下测试场景:
- 当没有选中任何选项时的键盘导航行为
- 当只有一个选中选项时的键盘导航行为
- 在选项之间快速连续按键的情况
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/OptionList.tsx
(2 hunks)tests/OptionList.test.tsx
(1 hunks)
🔇 Additional comments (3)
src/OptionList.tsx (2)
88-97
: 实现良好,建议批准!
新增的 isSelected
函数实现清晰,并且通过 useCallback
进行了性能优化。依赖项列表完整,包含了 mode、rawValues 的值和大小变化。
108-108
: 键盘导航逻辑修改正确,建议验证边界情况
修改后的条件 isSelected(data.value) || !overMaxCount
很好地实现了PR的目标,允许在达到最大选择数时仍可以通过键盘导航已选中的选项。
建议验证以下场景:
- 达到 maxCount 时的键盘导航
- 未选中选项的跳过逻辑
- 循环导航功能
✅ Verification successful
Let me gather more specific information about the test cases.
Let me check the actual test implementation to verify the test coverage.
键盘导航功能已得到充分验证,无需额外测试
测试用例已经完整覆盖了关键场景:
- 已验证达到 maxCount (2) 时的键盘导航功能
- 测试包含了预选状态('1'和'2'被选中)
- 验证了键盘导航事件的正确触发
代码实现和测试用例都表明该修改是安全的。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索相关测试用例,确保边界情况得到覆盖
# 查找与键盘导航相关的测试
rg -l "keyDown|maxCount.*keyboard|navigate.*selected" --type ts
# 检查测试文件中的相关测试用例
ast-grep --pattern 'test($_, function() {
$$$
maxCount
$$$
keyDown
$$$
})'
Length of output: 384
Script:
#!/bin/bash
# 检查 OptionList 相关的键盘导航测试用例
rg "keyDown.*maxCount|maxCount.*keyDown" -B 2 -A 10 tests/OptionList.test.tsx
# 检查键盘导航测试文件中的相关用例
rg "describe.*keyboard|test.*keyboard" -B 1 -A 5 tests/shared/keyDownTest.tsx
# 查找与 maxCount 相关的测试设置
rg "maxCount.*=|maxCount:" -B 2 -A 2 tests/OptionList.test.tsx
Length of output: 663
Script:
#!/bin/bash
# 获取完整的测试用例实现
rg -A 30 "should allow keyboard navigation on selected options when reach maxCount" tests/OptionList.test.tsx
# 检查其他相关的键盘导航测试
rg "it\(.*keyboard.*maxCount" -A 20 tests/
Length of output: 2132
tests/OptionList.test.tsx (1)
397-419
: 测试用例结构清晰,设置合理!
测试用例的设置包含了以下优点:
- 测试目标明确,注释详细说明了三个验证点
- 正确配置了多选模式和最大选择数量限制
- 预选值的初始化符合测试场景要求
} | ||
return rawValues.has(value); | ||
}, | ||
[mode, [...rawValues].toString(), rawValues.size], |
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.
rawValues.size 放 ...rawValues].toString()
前面,性能好点。
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.
大佬你好快的合(
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1088 +/- ##
=======================================
Coverage 98.24% 98.24%
=======================================
Files 39 39
Lines 1483 1483
Branches 441 447 +6
=======================================
Hits 1457 1457
Misses 26 26 ☔ View full report in Codecov by Sentry. |
🐛 问题描述
rc-select 在达到 maxCount 时存在交互不一致的问题:
🔨 修复方案
统一了鼠标与键盘的交互行为:
🧪 测试覆盖
添加了专门的测试用例验证:
📝 备注
这个修复提升了组件的可访问性,使键盘操作的用户获得与鼠标操作相同的体验。
Summary by CodeRabbit
新功能
测试