fix: prevent selecting disabled items with keyboard navigation#326
Conversation
现状总结键盘导航逻辑改进，通过过滤禁用选项（保留当前选中项）来构建有效选项列表，基于该列表计算导航索引，并在状态更新前验证下一个选项与当前值不同。 更改
估算代码审查工作量🎯 3 (中等复杂) | ⏱️ ~20 分钟 可能相关的 PR
建议审查者
诗歌
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the keyboard navigation logic in the Segmented component to ensure that disabled options are skipped during interaction. It filters the available options to exclude disabled items before calculating the next index and adds a unit test to verify this behavior. A review comment identified a potential edge case where an invalid initial value could result in a -1 index, leading to incorrect navigation calculations, and suggested a fix to handle this scenario.
| const currentIndex = validOptions.findIndex( | ||
| (option) => option.value === rawValue, | ||
| ); | ||
|
|
||
| const total = segmentedOptions.length; | ||
| const total = validOptions.length; | ||
| const nextIndex = (currentIndex + offset + total) % total; | ||
| const nextOption = validOptions[nextIndex]; |
There was a problem hiding this comment.
当 rawValue 不在 validOptions 中时（例如初始值不在选项列表中），currentIndex 会被设为 -1。在这种情况下，如果 offset 为 -1（按下左/上方向键），nextIndex 的计算结果 (currentIndex + offset + total) % total 会指向错误的索引（例如 total 为 3 时，结果为 1 而不是 2）。建议对 currentIndex === -1 的情况进行特殊处理，以确保在初始值无效时也能正确导航到第一个或最后一个有效选项。
const currentIndex = validOptions.findIndex(
(option) => option.value === rawValue,
);
const total = validOptions.length;
const nextIndex =
currentIndex === -1
? (offset > 0 ? 0 : total - 1)
: (currentIndex + offset + total) % total;
const nextOption = validOptions[nextIndex];
There was a problem hiding this comment.
这里看起来和之前的逻辑是一致的；应该无需处理吧；
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.tsx`:
- Around line 245-255: The navigation logic uses validOptions.findIndex to
compute currentIndex which can be -1 when the controlled value (rawValue) is no
longer in options; to avoid accidentally changing value with arrow keys, add a
short-circuit that returns early when currentIndex === -1 before computing
nextIndex/nextOption. Update the block around
validOptions.findIndex/currentIndex (and before nextIndex, nextOption,
setRawValue, onChange) to check currentIndex === -1 and exit without calling
setRawValue or onChange.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f30b3f17-191a-40f3-bffb-ab0783cad515
📒 Files selected for processing (2)
src/index.tsxtests/index.test.tsx
| const currentIndex = validOptions.findIndex( | ||
| (option) => option.value === rawValue, | ||
| ); | ||
|
|
||
| const total = segmentedOptions.length; | ||
| const total = validOptions.length; | ||
| const nextIndex = (currentIndex + offset + total) % total; | ||
| const nextOption = validOptions[nextIndex]; | ||
|
|
||
| const nextOption = segmentedOptions[nextIndex]; | ||
| if (nextOption) { | ||
| if (nextOption && nextOption.value !== rawValue) { | ||
| setRawValue(nextOption.value); | ||
| onChange?.(nextOption.value); |
There was a problem hiding this comment.
在当前值已失效时先短路，避免方向键偷偷改值。
当 value 是受控的且 rawValue 已不在 options 里时，Line 245 的 findIndex 会返回 -1。这样 Line 250 的取模会把 ArrowRight/ArrowDown 导到第一个有效项、ArrowLeft/ArrowUp 导到最后一个有效项，并触发一次非预期的 onChange，这和上面“值不存在时不自动切换”的约束相冲突。这里建议在 currentIndex === -1 时直接返回。
💡 建议修复
const currentIndex = validOptions.findIndex(
(option) => option.value === rawValue,
);
+ if (currentIndex === -1) {
+ return;
+ }
const total = validOptions.length;
const nextIndex = (currentIndex + offset + total) % total;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.tsx` around lines 245 - 255, The navigation logic uses
validOptions.findIndex to compute currentIndex which can be -1 when the
controlled value (rawValue) is no longer in options; to avoid accidentally
changing value with arrow keys, add a short-circuit that returns early when
currentIndex === -1 before computing nextIndex/nextOption. Update the block
around validOptions.findIndex/currentIndex (and before nextIndex, nextOption,
setRawValue, onChange) to check currentIndex === -1 and exit without calling
setRawValue or onChange.
There was a problem hiding this comment.
同上，可以check一下这个边界是否需要处理；或者单独pr处理？
There was a problem hiding this comment.
@QDyanbing，经过分析，我建议直接在本 PR 中处理这个边界情况，原因如下：
分析
validOptions 的过滤逻辑是 option.value === rawValue || !option.disabled，因此在正常情况下（rawValue 存在于 options 中），当前值一定会出现在 validOptions 里，currentIndex 不会是 -1。
但在受控模式下，如果上层传入的 value 不在 options 列表中，validOptions 里就找不到当前值，findIndex 返回 -1。此时：
| offset | 计算结果 | 效果 |
|---|---|---|
+1（ArrowRight） |
(-1 + 1 + N) % N = 0 |
跳到第一项 |
-1（ArrowLeft） |
(-1 - 1 + N) % N = N-2 |
跳到倒数第二项 |
由于 nextOption.value !== rawValue 成立，onChange 会被意外触发——这与代码中已有的注释"不应自动切换不存在于 options 中的值"直接冲突。
建议：在本 PR 中一并修复
修复量极小（3 行），且改动就在本次修改的同一函数内，建议直接加入：
const currentIndex = validOptions.findIndex(
(option) => option.value === rawValue,
);
+ if (currentIndex === -1) {
+ return;
+ }
const total = validOptions.length;
const nextIndex = (currentIndex + offset + total) % total;这样既保持本 PR 的改动集中，又能一次性封住"受控值不在 options 中时方向键仍触发 onChange"的漏洞，无需单独开 PR。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #326 +/- ##
=======================================
Coverage 99.39% 99.40%
=======================================
Files 2 2
Lines 165 167 +2
Branches 41 40 -1
=======================================
+ Hits 164 166 +2
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
背景
修复 ant-design/ant-design#57567 提到的问题：当
Segmented通过键盘方向键切换选项时，不应选中 disabled item。变更说明
关联缺陷
fix ant-design/ant-design#57567
Summary by CodeRabbit
Bug Fixes