Skip to content

fix: prevent selecting disabled items with keyboard navigation #326

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.

Sign up for GitHub

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

Merged
zombieJ merged 1 commit into from
Apr 8, 2026
Merged

fix: prevent selecting disabled items with keyboard navigation #326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,20 @@ const Segmented = React.forwardRef<HTMLDivElement, SegmentedProps>(

// ======================= Keyboard ========================
const onOffset = (offset: number) => {
const currentIndex = segmentedOptions.findIndex(
// keep the current option and not disabled options
const validOptions = segmentedOptions.filter(
(option) => option.value === rawValue || !option.disabled,
);

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];
Comment on lines +245 to +251
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

当 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];

Copy link
Copy Markdown
Contributor Author

@QDyanbing QDyanbing Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里看起来和之前的逻辑是一致的；应该无需处理吧；


const nextOption = segmentedOptions[nextIndex];
if (nextOption) {
if (nextOption && nextOption.value !== rawValue) {
setRawValue(nextOption.value);
onChange?.(nextOption.value);
Comment on lines +245 to 255
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

在当前值已失效时先短路，避免方向键偷偷改值。

当 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.

Copy link
Copy Markdown
Contributor Author

@QDyanbing QDyanbing Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上，可以check一下这个边界是否需要处理；或者单独pr处理？

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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。

}
Expand Down
27 changes: 27 additions & 0 deletions tests/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,33 @@ describe('Segmented keyboard navigation', () => {
expect(onChange).toHaveBeenCalledWith('iOS');
});

it('should not select a disabled option when it is the only next item', async () => {
const user = userEvent.setup();
const onChange = jest.fn();
const { container } = render(
<Segmented
options={[
{ label: 'Enabled', value: 'enabled' },
{ label: 'Disabled', value: 'disabled', disabled: true },
]}
defaultValue="enabled"
onChange={onChange}
/>,
);

await user.tab();
await user.tab();
await user.keyboard('{ArrowRight}');

const inputs = container.querySelectorAll<HTMLInputElement>(
'.rc-segmented-item-input',
);

expect(onChange).not.toHaveBeenCalled();
expect(inputs[0].checked).toBe(true);
expect(inputs[1].checked).toBe(false);
});

it('should not have focus style when clicking', async () => {
const user = userEvent.setup();
const { container } = render(
Expand Down
Loading