Skip to content

fix: option position announcement for virtual mode #1226

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

Open
Pareder wants to merge 1 commit into
base: master
Choose a base branch
from
+195 −35
Open

fix: option position announcement for virtual mode #1226

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
66 changes: 61 additions & 5 deletions src/OptionList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,12 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r

const getLabel = (item: Record<string, any>) => item.label;

// Skip group headers to match native <select> announcements with <optgroup>
const optionCount = memoFlattenOptions.filter((option) => !option.group).length;

const getOptionPosition = (index: number) =>
memoFlattenOptions.slice(0, index + 1).filter((option) => !option.group).length;
Comment on lines +278 to +282

@gemini-code-assist gemini-code-assist Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Calculating optionCount and getOptionPosition on every render performs array filtering and slicing operations ($O(N)$) over the entire memoFlattenOptions list. Since getOptionPosition is called up to three times per render (for activeIndex - 1, activeIndex, and activeIndex + 1), and renders occur on every keyboard navigation step (as activeIndex changes), this can cause noticeable lag/performance degradation in large virtualized lists.

Since memoFlattenOptions is already memoized, we can precompute the option count and positions map in a single $O(N)$ pass using React.useMemo. This reduces the lookup in getOptionPosition to $O(1)$ and avoids redundant array allocations and filtering on every render.

  // Skip group headers to match native <select> announcements with <optgroup>
  const [optionCount, optionPositions] = React.useMemo(() => {
    let count = 0;
    const positions = new Array(memoFlattenOptions.length);
    for (let i = 0; i < memoFlattenOptions.length; i += 1) {
      if (!memoFlattenOptions[i].group) {
        count += 1;
      }
      positions[i] = count;
    }
    return [count, positions];
  }, [memoFlattenOptions]);

  const getOptionPosition = (index: number) => optionPositions[index] || 0;


function getItemAriaProps(item: FlattenOptionData<BaseOptionType>, index: number) {
const { group } = item;

Expand All @@ -291,12 +297,13 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r
}
const itemData = item.data || {};
const { value, disabled } = itemData;
const { group } = item;
const attrs = pickAttrs(itemData, true);
const mergedLabel = getLabel(item);
return item ? (
<div
aria-label={typeof mergedLabel === 'string' && !group ? mergedLabel : null}
aria-label={typeof mergedLabel === 'string' ? mergedLabel : null}
aria-setsize={optionCount}
aria-posinset={getOptionPosition(index)}
Comment on lines +304 to +306

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

建议将数字标签也映射到 aria-label，避免可读名称丢失。

当前仅在 label 为字符串时设置 aria-label。当标签是数字时（合法场景），隐藏无障碍节点会丢失该标签语义，读屏更可能退化为读 value。建议复用现有 isTitleType 逻辑统一转字符串。

💡 建议修改 
-        aria-label={typeof mergedLabel === 'string' ? mergedLabel : null}
+        aria-label={isTitleType(mergedLabel) ? String(mergedLabel) : null}
...
-          aria-label={typeof groupLabel === 'string' ? groupLabel : null}
+          aria-label={isTitleType(groupLabel) ? String(groupLabel) : null}

Also applies to: 361-362

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/OptionList.tsx` around lines 304 - 306, The aria-label currently only
sets when mergedLabel is a string, which drops semantic labels for numeric
titles; update the condition to reuse the existing isTitleType check and coerce
the title to string (e.g., aria-label={isTitleType(mergedLabel) ?
String(mergedLabel) : null}) so numeric labels are exposed to assistive tech;
apply the same change to the other occurrence that sets aria-label (the second
block using mergedLabel/getOptionPosition).

{...attrs}
key={index}
{...getItemAriaProps(item, index)}
Expand All @@ -308,6 +315,57 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r
) : null;
};

const getGroupItem = (index: number) => {
for (let i = index; i >= 0; i -= 1) {
const current = memoFlattenOptions[i];
if (current?.group) {
return current;
}
}
return null;
};

// Nest options inside `role="group"` wrappers
const renderHiddenItems = () => {
const segments: {
group: FlattenOptionData<BaseOptionType> | null;
indexes: number[];
}[] = [];

[activeIndex - 1, activeIndex, activeIndex + 1].forEach((index) => {
const item = memoFlattenOptions[index];
if (!item || item.group) {
return;
}

const groupItem = item.groupOption ? getGroupItem(index) : null;
const lastSegment = segments[segments.length - 1];

if (lastSegment && lastSegment.group === groupItem) {
lastSegment.indexes.push(index);
} else {
segments.push({ group: groupItem, indexes: [index] });
}
});

return segments.map(({ group, indexes }) => {
if (!group) {
return indexes.map(renderItem);
}

const groupLabel = getLabel(group);
return (
<div
key={group.key}
role="group"
aria-label={typeof groupLabel === 'string' ? groupLabel : null}
>
{indexes.map(renderItem)}
</div>
);
});
};

const a11yProps = {
role: 'listbox',
id: `${id}_list`,
Expand All @@ -317,9 +375,7 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r
<>
{virtual && (
<div {...a11yProps} style={{ height: 0, width: 0, overflow: 'hidden' }}>
{renderItem(activeIndex - 1)}
{renderItem(activeIndex)}
{renderItem(activeIndex + 1)}
{renderHiddenItems()}
</div>
)}
<List<FlattenOptionData<BaseOptionType>>
Expand Down
99 changes: 99 additions & 0 deletions tests/Accessibility.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,105 @@ describe('Select.Accessibility', () => {
});
});

it('should have correct aria-posinset and aria-setsize in virtual mode', () => {
const { container } = render(
<Select
id="virtual-select"
open
options={[{ value: '1' }, { value: '2' }, { value: '3' }, { value: '4' }, { value: '5' }]}
/>,
);

// The hidden accessibility container is the listbox itself in virtual mode
const getHiddenOptions = () =>
Array.from(document.querySelectorAll('#virtual-select_list div[role="option"]'));

// Active index is 0, so the hidden container renders options 0 and 1
let hiddenOptions = getHiddenOptions();
expect(hiddenOptions.map((option) => option.getAttribute('aria-posinset'))).toEqual([
'1',
'2',
]);
hiddenOptions.forEach((option) => {
expect(option).toHaveAttribute('aria-setsize', '5');
});

// Move active option to the middle of the list
keyDown(container.querySelector('input')!, KeyCode.DOWN);
keyDown(container.querySelector('input')!, KeyCode.DOWN);

// Active index is 2, so the hidden container renders options 1, 2 and 3
hiddenOptions = getHiddenOptions();
expect(hiddenOptions.map((option) => option.getAttribute('aria-posinset'))).toEqual([
'2',
'3',
'4',
]);
hiddenOptions.forEach((option) => {
expect(option).toHaveAttribute('aria-setsize', '5');
});
});

it('aria-posinset and aria-setsize should skip group headers like native optgroup', () => {
const { container } = render(
<Select
id="virtual-select"
open
options={[
{
label: 'First group',
options: [{ value: '1' }, { value: '2' }, { value: '3' }],
},
{
label: 'Second group',
options: [{ value: '4' }, { value: '5' }, { value: '6' }],
},
]}
/>,
);

const hiddenContainer = document.querySelector('#virtual-select_list');
const getHiddenOptions = () =>
Array.from(hiddenContainer.querySelectorAll('div[role="option"]'));
const getGroupWrappers = () =>
Array.from(hiddenContainer.querySelectorAll('div[role="group"]'));

// Active option is the first real option (flatten index 1), so the
// hidden container renders the first two options inside their group
let groupWrappers = getGroupWrappers();
expect(groupWrappers).toHaveLength(1);
expect(groupWrappers[0]).toHaveAttribute('aria-label', 'First group');

let hiddenOptions = getHiddenOptions();
expect(hiddenOptions.map((option) => option.getAttribute('aria-posinset'))).toEqual([
'1',
'2',
]);
hiddenOptions.forEach((option) => {
expect(option).toHaveAttribute('aria-setsize', '6');
expect(option.parentElement).toBe(groupWrappers[0]);
});

// Move into the second group: positions keep counting across groups
keyDown(container.querySelector('input')!, KeyCode.DOWN);
keyDown(container.querySelector('input')!, KeyCode.DOWN);
keyDown(container.querySelector('input')!, KeyCode.DOWN);

groupWrappers = getGroupWrappers();
expect(groupWrappers).toHaveLength(1);
expect(groupWrappers[0]).toHaveAttribute('aria-label', 'Second group');

hiddenOptions = getHiddenOptions();
expect(hiddenOptions.map((option) => option.getAttribute('aria-posinset'))).toEqual([
'4',
'5',
]);
hiddenOptions.forEach((option) => {
expect(option).toHaveAttribute('aria-setsize', '6');
expect(option.parentElement).toBe(groupWrappers[0]);
});
});

it('should have correct aria and role attributes in virtual false', () => {
render(
<Select
Expand Down
26 changes: 11 additions & 15 deletions tests/__snapshots__/OptionList.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,19 @@ exports[`OptionList renders correctly virtual 1`] = `
style="height: 0px; width: 0px; overflow: hidden;"
>
<div
aria-selected="false"
id="undefined_list_0"
role="presentation"
/>
<div
aria-label="value-1"
aria-selected="true"
id="undefined_list_1"
role="option"
role="group"
>
1
<div
aria-label="value-1"
aria-posinset="1"
aria-selected="true"
aria-setsize="2"
id="undefined_list_1"
role="option"
>
1
</div>
</div>
<div
aria-selected="false"
id="undefined_list_2"
role="presentation"
/>
</div>
<div
class="rc-virtual-list"
Expand Down
12 changes: 12 additions & 0 deletions tests/__snapshots__/Select.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@ exports[`Select.Basic does not filter when filterOption value is false 1`] = `
>
<div
aria-label="1"
aria-posinset="1"
aria-selected="false"
aria-setsize="2"
id="test-id_list_0"
role="option"
>
1
</div>
<div
aria-label="2"
aria-posinset="2"
aria-selected="false"
aria-setsize="2"
id="test-id_list_1"
role="option"
>
Expand Down Expand Up @@ -409,15 +413,19 @@ exports[`Select.Basic should contain falsy children 1`] = `
>
<div
aria-label="1"
aria-posinset="1"
aria-selected="true"
aria-setsize="2"
id="test-id_list_0"
role="option"
>
1
</div>
<div
aria-label="2"
aria-posinset="2"
aria-selected="false"
aria-setsize="2"
id="test-id_list_1"
role="option"
>
Expand Down Expand Up @@ -508,15 +516,19 @@ exports[`Select.Basic should render custom dropdown correctly 1`] = `
>
<div
aria-label="1"
aria-posinset="1"
aria-selected="false"
aria-setsize="2"
id="test-id_list_0"
role="option"
>
1
</div>
<div
aria-label="2"
aria-posinset="2"
aria-selected="false"
aria-setsize="2"
id="test-id_list_1"
role="option"
>
Expand Down
27 changes: 12 additions & 15 deletions tests/__snapshots__/Tags.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,20 @@ exports[`Select.Tags OptGroup renders correctly 1`] = `
style="height: 0px; width: 0px; overflow: hidden;"
>
<div
aria-selected="false"
id="test-id_list_0"
role="presentation"
/>
<div
aria-label="Jack"
aria-selected="true"
id="test-id_list_1"
role="option"
aria-label="Manager"
role="group"
>
jack
<div
aria-label="Jack"
aria-posinset="1"
aria-selected="true"
aria-setsize="3"
id="test-id_list_1"
role="option"
>
jack
</div>
</div>
<div
aria-selected="false"
id="test-id_list_2"
role="presentation"
/>
</div>
<div
class="rc-virtual-list"
Expand Down
Loading