fix: listbox collection simplification

Author: tenphi

src/components/fields/FilterListBox/FilterListBox.tsx

@@ -12,7 +12,7 @@ import React, {
   useState,
 } from 'react';
 import { useFilter, useKeyboard } from 'react-aria';
-import { Section as BaseSection, Item } from 'react-stately';
+import { Section as BaseSection, Item, useListState } from 'react-stately';
 
 import { LoadingIcon } from '../../../icons';
 import { useProviderProps } from '../../../provider';
@@ -301,27 +301,23 @@ export const FilterListBox = forwardRef(function FilterListBox<
     }
   }
 
+  // Create a local collection for efficient key lookups and existence checks
+  const localCollectionState = useListState({
+    children: children as any,
+    items: items as any,
+    selectionMode: 'none' as any,
+  });
+
   // Collect original option keys to avoid duplicating them as custom values.
   const originalKeys = useMemo(() => {
     const keys = new Set<string>();
-
-    const collectKeys = (nodes: ReactNode): void => {
-      React.Children.forEach(nodes, (child: any) => {
-        if (!child || typeof child !== 'object') return;
-
-        if (child.type === Item) {
-          if (child.key != null) keys.add(String(child.key));
-        }
-
-        if (child.props?.children) {
-          collectKeys(child.props.children);
-        }
-      });
-    };
-
-    if (children) collectKeys(children);
+    for (const item of localCollectionState.collection) {
+      if (item.type === 'item') {
+        keys.add(String(item.key));
+      }
+    }
     return keys;
-  }, [children]);
+  }, [localCollectionState.collection]);
 
   // State to keep track of custom (user-entered) items that were selected.
   const [customKeys, setCustomKeys] = useState<Set<string>>(new Set());
@@ -521,39 +517,19 @@ export const FilterListBox = forwardRef(function FilterListBox<
     if (!term) return childrenToProcess;
 
     // Helper to determine if the term is already present (exact match on rendered textValue or the key).
-    const doesTermExist = (nodes: ReactNode): boolean => {
-      let exists = false;
-
-      const checkNodes = (childNodes: ReactNode): void => {
-        React.Children.forEach(childNodes, (child: any) => {
-          if (!child || typeof child !== 'object') return;
-
-          // Check items directly
-          if (child.type === Item) {
-            const childText =
-              child.props.textValue ||
-              (typeof child.props.children === 'string'
-                ? child.props.children
-                : '') ||
-              String(child.props.children ?? '');
-
-            if (term === childText || String(child.key) === term) {
-              exists = true;
-            }
-          }
-
-          // Recurse into sections or other wrappers
-          if (child.props?.children) {
-            checkNodes(child.props.children);
+    const doesTermExist = (term: string): boolean => {
+      for (const item of localCollectionState.collection) {
+        if (item.type === 'item') {
+          const textValue = item.textValue || String(item.rendered || '');
+          if (term === textValue || String(item.key) === term) {
+            return true;
           }
-        });
-      };
-
-      checkNodes(nodes);
-      return exists;
+        }
+      }
+      return false;
     };
 
-    if (doesTermExist(mergedChildren)) {
+    if (doesTermExist(term)) {
       return childrenToProcess;
     }
 

src/components/fields/FilterPicker/FilterPicker.test.tsx

@@ -27,6 +27,36 @@ describe('<FilterPicker />', () => {
     </FilterPicker.Section>,
   ];
 
+  // Data arrays for testing sorting functionality (which requires items prop)
+  const basicItemsData = [
+    { key: 'apple', label: 'Apple' },
+    { key: 'banana', label: 'Banana' },
+    { key: 'cherry', label: 'Cherry' },
+    { key: 'date', label: 'Date' },
+    { key: 'elderberry', label: 'Elderberry' },
+  ];
+
+  const sectionsItemsData = [
+    {
+      key: 'fruits',
+      label: 'Fruits',
+      children: [
+        { key: 'apple', label: 'Apple' },
+        { key: 'banana', label: 'Banana' },
+        { key: 'cherry', label: 'Cherry' },
+      ],
+    },
+    {
+      key: 'vegetables',
+      label: 'Vegetables',
+      children: [
+        { key: 'carrot', label: 'Carrot' },
+        { key: 'broccoli', label: 'Broccoli' },
+        { key: 'spinach', label: 'Spinach' },
+      ],
+    },
+  ];
+
   describe('Basic functionality', () => {
     it('should render trigger button with placeholder', () => {
       const { getByRole } = renderWithRoot(
@@ -180,11 +210,15 @@ describe('<FilterPicker />', () => {
     it('should sort selected items to top when popover reopens in multiple mode', async () => {
       const { getByRole, getByText } = renderWithRoot(
         <FilterPicker
+          sortSelectedToTop
           label="Select fruits"
           placeholder="Choose fruits..."
           selectionMode="multiple"
+          items={basicItemsData}
         >
-          {basicItems}
+          {(item) => (
+            <FilterPicker.Item key={item.key}>{item.label}</FilterPicker.Item>
+          )}
         </FilterPicker>,
       );
 
@@ -225,11 +259,21 @@ describe('<FilterPicker />', () => {
     it('should sort selected items to top within their sections', async () => {
       const { getByRole, getByText } = renderWithRoot(
         <FilterPicker
+          sortSelectedToTop
           label="Select items"
           placeholder="Choose items..."
           selectionMode="multiple"
+          items={sectionsItemsData}
         >
-          {sectionsItems}
+          {(section) => (
+            <FilterPicker.Section key={section.key} title={section.label}>
+              {section.children.map((item) => (
+                <FilterPicker.Item key={item.key}>
+                  {item.label}
+                </FilterPicker.Item>
+              ))}
+            </FilterPicker.Section>
+          )}
         </FilterPicker>,
       );
 
@@ -297,11 +341,15 @@ describe('<FilterPicker />', () => {
     it('should maintain sorting when items are deselected and popover reopens', async () => {
       const { getByRole, getByText } = renderWithRoot(
         <FilterPicker
+          sortSelectedToTop
           label="Select fruits"
           placeholder="Choose fruits..."
           selectionMode="multiple"
+          items={basicItemsData}
         >
-          {basicItems}
+          {(item) => (
+            <FilterPicker.Item key={item.key}>{item.label}</FilterPicker.Item>
+          )}
         </FilterPicker>,
       );
 
@@ -371,11 +419,15 @@ describe('<FilterPicker />', () => {
     it('should not reorder items when selecting additional items after reopening popover', async () => {
       const { getByRole, getByText } = renderWithRoot(
         <FilterPicker
+          sortSelectedToTop
           label="Select fruits"
           placeholder="Choose fruits..."
           selectionMode="multiple"
+          items={basicItemsData}
         >
-          {basicItems}
+          {(item) => (
+            <FilterPicker.Item key={item.key}>{item.label}</FilterPicker.Item>
+          )}
         </FilterPicker>,
       );
 
@@ -428,11 +480,15 @@ describe('<FilterPicker />', () => {
     it('should work correctly in single selection mode', async () => {
       const { getByRole, getByText } = renderWithRoot(
         <FilterPicker
+          sortSelectedToTop
           label="Select fruit"
           placeholder="Choose a fruit..."
           selectionMode="single"
+          items={basicItemsData}
         >
-          {basicItems}
+          {(item) => (
+            <FilterPicker.Item key={item.key}>{item.label}</FilterPicker.Item>
+          )}
         </FilterPicker>,
       );
 
@@ -848,7 +904,7 @@ describe('<FilterPicker />', () => {
         await userEvent.click(trigger);
       });
 
-      // Session 2: Reopen and verify both are visible and selected items are sorted to top
+      // Session 2: Reopen and verify both custom and regular values are visible
       await act(async () => {
         await userEvent.click(trigger);
       });