fix rowindex calculation when filtering async s2 combobox

the document didnt seem to be both updating its _minInvalidChildIndex nor updating its child indicies properly when the combobox list was filtered async (aka the collection got new items added and removed via insertBefore and/or removeChild. Additionally, we didnt see to ever call updateChildIndices on the Document other than the first time the collection loaded

Author: LFDanLu

packages/@react-aria/collections/src/Document.ts

@@ -103,7 +103,7 @@ export class BaseNode<T> {
   }
 
   private invalidateChildIndices(child: ElementNode<T>): void {
-    if (this._minInvalidChildIndex == null || child.index < this._minInvalidChildIndex.index) {
+    if (this._minInvalidChildIndex == null || !this._minInvalidChildIndex.isConnected || child.index < this._minInvalidChildIndex.index) {
       this._minInvalidChildIndex = child;
     }
   }
@@ -154,7 +154,11 @@ export class BaseNode<T> {
 
     newNode.nextSibling = referenceNode;
     newNode.previousSibling = referenceNode.previousSibling;
-    newNode.index = referenceNode.index;
+    // Ensure that the newNode's index is less than that of the reference node so that
+    // invalidateChildIndices will properly use the newNode as the _minInvalidChildIndex, thus making sure
+    // we will properly update the indexes of all sibiling nodes after the newNode. The value here doesn't matter
+    // since updateChildIndices should calculate the proper indexes.
+    newNode.index = referenceNode.index - 1;
     if (this.firstChild === referenceNode) {
       this.firstChild = newNode;
     } else if (referenceNode.previousSibling) {
@@ -469,6 +473,12 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
     }
 
     // Next, update dirty collection nodes.
+    // TODO: when updateCollection is called here, shouldn't we be calling this.updateChildIndicies as well? Theoretically it should only update
+    // nodes from _minInvalidChildIndex onwards so the increase in dirtyNodes should be minimal.
+    // Is element.updateNode supposed to handle that (it currently assumes the index stored on the node is correct already).
+    // At the moment, without this call to updateChildIndicies, filtering an async combobox doesn't actually update the index values of the
+    // updated collection...
+    this.updateChildIndices();
     for (let element of this.dirtyNodes) {
       if (element instanceof ElementNode) {
         if (element.isConnected && !element.isHidden) {

packages/@react-aria/interactions/src/usePress.ts

@@ -372,7 +372,7 @@ export function usePress(props: PressHookProps): PressResult {
           if (isDisabled) {
             e.preventDefault();
           }
-          
+
           // If triggered from a screen reader or by using element.click(),
           // trigger as if it were a keyboard click.
           if (!state.ignoreEmulatedMouseEvents && !state.isPressed && (state.pointerType === 'virtual' || isVirtualClick(e.nativeEvent))) {
@@ -820,7 +820,7 @@ export function usePress(props: PressHookProps): PressResult {
       // Only apply touch-action if not already set by another CSS rule.
       let style = getOwnerWindow(element).getComputedStyle(element);
       if (style.touchAction === 'auto') {
-        // touchAction: 'manipulation' is supposed to be equivalent, but in 
+        // touchAction: 'manipulation' is supposed to be equivalent, but in
         // Safari it causes onPointerCancel not to fire on scroll.
         // https://bugs.webkit.org/show_bug.cgi?id=240917
         (element as HTMLElement).style.touchAction = 'pan-x pan-y pinch-zoom';

packages/@react-spectrum/s2/stories/Picker.stories.tsx

@@ -261,6 +261,7 @@ export const AsyncPickerStory  = {
   render: AsyncPicker,
   args: {
     ...Example.args,
+    label: 'Star Wars Character',
     delay: 50
   },
   name: 'Async loading picker',

packages/@react-spectrum/s2/test/Combobox.test.tsx

@@ -21,6 +21,19 @@ import userEvent from '@testing-library/user-event';
 describe('Combobox', () => {
   let user;
   let testUtilUser = new User();
+  function DynamicCombobox(props) {
+    let {items, loadingState, onLoadMore, ...otherProps} = props;
+    return (
+      <ComboBox
+        {...otherProps}
+        label="Test combobox"
+        items={items}
+        loadingState={loadingState}
+        onLoadMore={onLoadMore}>
+        {(item: any) => <ComboBoxItem id={item.name} textValue={item.name}>{item.name}</ComboBoxItem>}
+      </ComboBox>
+    );
+  }
 
   beforeAll(function () {
     jest.useFakeTimers();
@@ -101,7 +114,7 @@ describe('Combobox', () => {
     expect(onLoadMore).toHaveBeenCalledTimes(2);
   });
 
-  it('should omit the laoder from the count of items', async () => {
+  it('should omit the loader from the count of items', async () => {
     jest.spyOn(navigator, 'platform', 'get').mockImplementation(() => 'MacIntel');
     let tree = render(
       <ComboBox label="test" loadingState="loadingMore">
@@ -122,4 +135,31 @@ describe('Combobox', () => {
     await user.keyboard('C');
     expect(announce).toHaveBeenLastCalledWith('2 options available.');
   });
+
+  it('should properly calculate the expected row index values even when the content changes', async () => {
+    let items = [{name: 'Chocolate'}, {name: 'Mint'}, {name: 'Chocolate Chip'}];
+    let tree = render(<DynamicCombobox items={items} />);
+
+    let comboboxTester = testUtilUser.createTester('ComboBox', {root: tree.container, interactionType: 'mouse'});
+    await comboboxTester.open();
+    let options = comboboxTester.options();
+    for (let [index, option] of options.entries()) {
+      expect(option).toHaveAttribute('aria-posinset', `${index + 1}`);
+    }
+
+    tree.rerender(<DynamicCombobox items={items} loadingState="filtering" />);
+    options = comboboxTester.options();
+    for (let [index, option] of options.entries()) {
+      expect(option).toHaveAttribute('aria-posinset', `${index + 1}`);
+    }
+
+    // A bit contrived, but essentially testing a combinaiton of insertions/deletions along side some of the old entries remaining
+    let newItems = [{name: 'Chocolate Mint'}, {name: 'Chocolate'}, {name: 'Chocolate Chip'}, {name: 'Chocolate Chip Cookie Dough'}]
+    tree.rerender(<DynamicCombobox items={newItems} loadingState="idle" />);
+
+    options = comboboxTester.options();
+    for (let [index, option] of options.entries()) {
+      expect(option).toHaveAttribute('aria-posinset', `${index + 1}`);
+    }
+  });
 });

packages/@react-spectrum/s2/test/Picker.test.tsx

@@ -0,0 +1,120 @@
+/*
+ * Copyright 2025 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 {act, render, setupIntersectionObserverMock} from '@react-spectrum/test-utils-internal';
+import {Picker, PickerItem} from '../src';
+import React from 'react';
+import {User} from '@react-aria/test-utils';
+
+describe('Picker', () => {
+  let testUtilUser = new User();
+  function DynamicPicker(props) {
+    let {items, isLoading, onLoadMore, ...otherProps} = props;
+    return (
+      <Picker
+        {...otherProps}
+        label="Test picker"
+        items={items}
+        isLoading={isLoading}
+         onLoadMore={onLoadMore}>
+        {(item: any) => <PickerItem id={item.name} textValue={item.name}>{item.name}</PickerItem>}
+      </Picker>
+    );
+  }
+
+  beforeAll(function () {
+    jest.useFakeTimers();
+    jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100);
+    jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100);
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+    act(() => jest.runAllTimers());
+  });
+
+  afterAll(function () {
+    jest.restoreAllMocks();
+  });
+
+  it.skip('should only call loadMore whenever intersection is detected', async () => {
+    let onLoadMore = jest.fn();
+    let observe = jest.fn();
+    let observer = setupIntersectionObserverMock({
+      observe
+    });
+
+    let tree = render(
+      <Picker label="test" isLoading onLoadMore={onLoadMore}>
+        <PickerItem>Chocolate</PickerItem>
+        <PickerItem>Mint</PickerItem>
+        <PickerItem>Strawberry</PickerItem>
+        <PickerItem>Vanilla</PickerItem>
+        <PickerItem>Chocolate Chip Cookie Dough</PickerItem>
+      </Picker>
+    );
+
+    let selectTester = testUtilUser.createTester('Select', {root: tree.container});
+    expect(selectTester.listbox).toBeFalsy();
+    selectTester.setInteractionType('mouse');
+    await selectTester.open();
+
+    expect(onLoadMore).toHaveBeenCalledTimes(0);
+    let sentinel = tree.getByTestId('loadMoreSentinel');
+    expect(observe).toHaveBeenLastCalledWith(sentinel);
+
+    act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
+    act(() => {jest.runAllTimers();});
+
+    tree.rerender(
+      <Picker label="test" onLoadMore={onLoadMore}>
+        <PickerItem>Chocolate</PickerItem>
+        <PickerItem>Mint</PickerItem>
+        <PickerItem>Strawberry</PickerItem>
+        <PickerItem>Vanilla</PickerItem>
+        <PickerItem>Chocolate Chip Cookie Dough</PickerItem>
+      </Picker>
+    );
+
+    act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
+    act(() => {jest.runAllTimers();});
+    // Note that if this was using useAsyncList, we'd be shielded from extranous onLoadMore calls but
+    // we want to leave that to user discretion
+    expect(onLoadMore).toHaveBeenCalledTimes(2);
+  });
+
+  it.skip('should properly calculate the expected row index values', async () => {
+    let items = [{name: 'Chocolate'}, {name: 'Mint'}, {name: 'Chocolate Chip'}];
+    let tree = render(<DynamicPicker items={items} />);
+
+    let selectTester = testUtilUser.createTester('Select', {root: tree.container});
+    await selectTester.open();
+    let options = selectTester.options();
+    for (let [index, option] of options.entries()) {
+      expect(option).toHaveAttribute('aria-posinset', `${index + 1}`);
+    }
+
+    tree.rerender(<DynamicPicker items={items} isLoading />);
+    options = selectTester.options();
+    for (let [index, option] of options.entries()) {
+      expect(option).toHaveAttribute('aria-posinset', `${index + 1}`);
+    }
+
+    let newItems = [...items, {name: 'Chocolate Mint'}, {name: 'Chocolate Chip Cookie Dough'}]
+    tree.rerender(<DynamicPicker items={newItems} />);
+
+    options = selectTester.options();
+    for (let [index, option] of options.entries()) {
+      expect(option).toHaveAttribute('aria-posinset', `${index + 1}`);
+    }
+  });
+});

packages/react-aria-components/test/ListBox.test.js

@@ -1365,6 +1365,7 @@ describe('ListBox', () => {
     let onLoadMore = jest.fn();
     let observe = jest.fn();
     afterEach(() => {
+      jest.runAllTimers();
       jest.clearAllMocks();
     });
 
@@ -1444,16 +1445,16 @@ describe('ListBox', () => {
         clientHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100);
       });
 
-      beforeEach(() => {
+      afterEach(() => {
         act(() => {jest.runAllTimers();});
       });
 
-      afterAll(function () {
+      afterAll(() => {
         clientWidth.mockReset();
         clientHeight.mockReset();
       });
 
-      let VirtualizedAsyncListbox = (props) => {
+      function VirtualizedAsyncListbox(props) {
         let {items, isLoading, onLoadMore, ...listBoxProps} = props;
         return (
           <Virtualizer
@@ -1499,6 +1500,8 @@ describe('ListBox', () => {
       });
 
       // TODO: for some reason this tree renders empty if ran with the above test...
+      // Even if the above test doesn't do anything within it, the below tree won't render with content until the above test
+      // is fully commented out (aka even the it(...))
       // It thinks that the contextSize is 0 and never updates
       it.skip('should not reserve room for the loader if isLoading is false', () => {
         let tree = render(<VirtualizedAsyncListbox items={items} />);