diff --git a/packages/react-aria/src/listbox/useOption.ts b/packages/react-aria/src/listbox/useOption.ts index dbab6edf899..0a89c1ef2c8 100644 --- a/packages/react-aria/src/listbox/useOption.ts +++ b/packages/react-aria/src/listbox/useOption.ts @@ -16,6 +16,7 @@ import {DOMAttributes, FocusableElement, Key, RefObject} from '@react-types/shar import {filterDOMProps} from '../utils/filterDOMProps'; import {getItemCount} from 'react-stately/private/collections/getItemCount'; import {getItemId, listData} from './utils'; +import {getPosInSet} from '../utils/posinset'; import {isFocusVisible} from '../interactions/useFocusVisible'; import {ListState} from 'react-stately/useListState'; import {mergeProps} from '../utils/mergeProps'; @@ -128,8 +129,8 @@ export function useOption( let item = state.collection.getItem(key); if (isVirtualized) { - let index = Number(item?.index); - optionProps['aria-posinset'] = Number.isNaN(index) ? undefined : index + 1; + let posInSet = getPosInSet(state.collection, key, item); + optionProps['aria-posinset'] = Number.isNaN(posInSet) ? undefined : posInSet; optionProps['aria-setsize'] = getItemCount(state.collection); } diff --git a/packages/react-aria/src/menu/useMenuItem.ts b/packages/react-aria/src/menu/useMenuItem.ts index d38d3958e5a..7f301ced23d 100644 --- a/packages/react-aria/src/menu/useMenuItem.ts +++ b/packages/react-aria/src/menu/useMenuItem.ts @@ -25,6 +25,7 @@ import { import {filterDOMProps} from '../utils/filterDOMProps'; import {getEventTarget} from '../utils/shadowdom/DOMFunctions'; import {getItemCount} from 'react-stately/private/collections/getItemCount'; +import {getPosInSet} from '../utils/posinset'; import {handleLinkClick, useLinkProps, useRouter} from '../utils/openLink'; import {isFocusVisible, setInteractionModality} from '../interactions/useFocusVisible'; import {menuData} from './utils'; @@ -225,8 +226,8 @@ export function useMenuItem( } if (isVirtualized) { - let index = Number(item?.index); - ariaProps['aria-posinset'] = Number.isNaN(index) ? undefined : index + 1; + let posInSet = getPosInSet(state.collection, key, item); + ariaProps['aria-posinset'] = Number.isNaN(posInSet) ? undefined : posInSet; ariaProps['aria-setsize'] = getItemCount(state.collection); } diff --git a/packages/react-aria/src/utils/posinset.ts b/packages/react-aria/src/utils/posinset.ts new file mode 100644 index 00000000000..507f0e96c2e --- /dev/null +++ b/packages/react-aria/src/utils/posinset.ts @@ -0,0 +1,54 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {Collection, Key, Node} from '@react-types/shared'; +import {getChildNodes} from 'react-stately/private/collections/getChildNodes'; + +/** + * Computes the 1-based position of an item within the whole set of items, ignoring + * section headers. `item.index` restarts at 0 within each section, but `aria-setsize` + * is the global item count (see getItemCount), so the position must be global too - + * otherwise aria-posinset/aria-setsize are inconsistent for sectioned collections. + * Items that are not inside a section keep their existing O(1) index-based position. + */ +export function getPosInSet( + collection: Collection>, + key: Key, + item: Node | null +): number { + if (item == null) { + return NaN; + } + + // Not inside a section: the flat index is already correct. + if (item.parentKey == null) { + return Number(item.index) + 1; + } + + // Inside a section: add the item count of every preceding section, then the + // item's position within its own section. + let position = Number(item.index) + 1; + for (let node of collection) { + if (node.key === item.parentKey) { + break; + } + if (node.type === 'section') { + for (let child of getChildNodes(node, collection)) { + if (child.type === 'item') { + position++; + } + } + } + } + + return position; +} diff --git a/packages/react-aria/test/menu/useMenu.test.tsx b/packages/react-aria/test/menu/useMenu.test.tsx index 9b19bbec8dc..f1abdc5300f 100644 --- a/packages/react-aria/test/menu/useMenu.test.tsx +++ b/packages/react-aria/test/menu/useMenu.test.tsx @@ -13,6 +13,8 @@ import {AriaMenuProps, useMenu} from '../../src/menu/useMenu'; import {Item} from 'react-stately/Item'; +import {Section} from 'react-stately/Section'; +import {getChildNodes} from 'react-stately/private/collections/getChildNodes'; import {Key} from '@react-types/shared'; import {pointerMap, render} from '@react-spectrum/test-utils-internal'; import React from 'react'; @@ -81,6 +83,33 @@ function VirtualizedMenu(props: AriaMenuProps) { ); } +function VirtualizedMenuWithSections(props: AriaMenuProps) { + let state = useTreeState(props); + let ref = React.useRef(null); + let {menuProps} = useMenu(props, state, ref); + + // Flatten sections into their items so useMenuItem is invoked for each item, + // mirroring how the real listbox iterates a sectioned collection. + let nodes: {key: Key; rendered: React.ReactNode; index?: number}[] = []; + for (let node of state.collection) { + if (node.type === 'section') { + for (let child of getChildNodes(node, state.collection)) { + nodes.push(child); + } + } else if (node.type === 'item') { + nodes.push(node); + } + } + + return ( +
    + {nodes.map(item => ( + + ))} +
+ ); +} + describe('useMenuTrigger', function () { let user; beforeAll(() => { @@ -137,4 +166,29 @@ describe('useMenuItem with isVirtualized', function () { expect(items[1]).toHaveAttribute('aria-setsize', '3'); expect(items[2]).toHaveAttribute('aria-setsize', '3'); }); + + it('sets global aria-posinset across sections', () => { + let {getAllByRole} = render( + +
+ One + Two +
+
+ Three + Four +
+
+ ); + + // aria-posinset should be global (1..4) and match aria-setsize, not restart + // per section (which would report 1..2 for both groups). + let items = getAllByRole('menuitem'); + expect(items[0]).toHaveAttribute('aria-posinset', '1'); + expect(items[1]).toHaveAttribute('aria-posinset', '2'); + expect(items[2]).toHaveAttribute('aria-posinset', '3'); + expect(items[3]).toHaveAttribute('aria-posinset', '4'); + expect(items[0]).toHaveAttribute('aria-setsize', '4'); + expect(items[3]).toHaveAttribute('aria-setsize', '4'); + }); });