-
-
Notifications
You must be signed in to change notification settings - Fork 483
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
aa2d435
0580206
3cbfc05
2c814d0
e06bc3c
2fbf34d
53adb6b
24ca449
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -257,6 +257,12 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r | |
| }, | ||
| })); | ||
|
|
||
| // Skip group headers to match native <select> announcements with <optgroup> | ||
| const optionPositions = React.useMemo(() => { | ||
| let count = 0; | ||
| return memoFlattenOptions.map((item) => (item.group ? count : (count += 1))); | ||
| }, [memoFlattenOptions]); | ||
|
|
||
| // ========================== Render ========================== | ||
| if (memoFlattenOptions.length === 0) { | ||
| return ( | ||
|
|
@@ -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 ? ( | ||
| return ( | ||
| <div | ||
| aria-label={typeof mergedLabel === 'string' && !group ? mergedLabel : null} | ||
| aria-label={isTitleType(mergedLabel) ? String(mergedLabel) : null} | ||
| aria-setsize={optionPositions[optionPositions.length - 1] ?? 0} | ||
| aria-posinset={optionPositions[index]} | ||
| {...attrs} | ||
| key={index} | ||
| {...getItemAriaProps(item, index)} | ||
|
|
@@ -305,7 +312,60 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r | |
| > | ||
| {value} | ||
| </div> | ||
| ) : null; | ||
| ); | ||
| }; | ||
|
|
||
| const getGroupItem = (index: number) => { | ||
| for (let i = index; i >= 0; i -= 1) { | ||
| const current = memoFlattenOptions[i]; | ||
| if (current?.group) { | ||
| return current; | ||
| } | ||
| } | ||
| // Unreachable: a grouped option always has a preceding group header | ||
| /* istanbul ignore next */ | ||
| 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={isTitleType(groupLabel) ? String(groupLabel) : null} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we preserve an accessible name when the group label is a React node? Option/group labels may be non-string React nodes; in that case this new |
||
| > | ||
| {indexes.map(renderItem)} | ||
| </div> | ||
| ); | ||
| }); | ||
| }; | ||
|
|
||
| const a11yProps = { | ||
|
|
@@ -317,9 +377,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>> | ||
|
|
||
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.
Could we precompute the owning group for each flattened item together with
optionPositions?getGroupItemwalks backward from the active index for each of up to three hidden items, so keyboard navigation near the end of a large single group can become O(N) per render and partially undermine virtualization. A memoized parallel array/map would make these lookups O(1).