fix logic to determine hasChildNodes

Author: snowystinger

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

@@ -257,7 +257,18 @@ export class ElementNode<T> extends BaseNode<T> {
     node.parentKey = this.parentNode instanceof ElementNode ? this.parentNode.node.key : null;
     node.prevKey = this.previousSibling?.node.key ?? null;
     node.nextKey = this.nextSibling?.node.key ?? null;
-    node.hasChildNodes = !!this.firstChild;
+
+    // Check if this node has any child nodes, but specifically any that are items.
+    let child = this.firstChild;
+    let hasChildNodes = false;
+    while (!hasChildNodes && child && child?.nextSibling) {
+      child = child.nextSibling;
+      if (child.node.type === 'item') {
+        hasChildNodes = true;
+      }
+    }
+
+    node.hasChildNodes = hasChildNodes;
     node.firstChildKey = this.firstChild?.node.key ?? null;
     node.lastChildKey = this.lastChild?.node.key ?? null;
   }

packages/@react-aria/gridlist/src/useGridListItem.ts

@@ -28,7 +28,9 @@ export interface AriaGridListItemOptions {
   /** Whether the list row is contained in a virtual scroller. */
   isVirtualized?: boolean,
   /** Whether selection should occur on press up instead of press down. */
-  shouldSelectOnPressUp?: boolean
+  shouldSelectOnPressUp?: boolean,
+  /** Whether this item has children, even if not loaded yet. */
+  hasChildItems?: boolean
 }
 
 export interface GridListItemAria extends SelectableItemStates {
@@ -86,13 +88,9 @@ export function useGridListItem<T>(props: AriaGridListItemOptions, state: ListSt
   };
 
   let treeGridRowProps: HTMLAttributes<HTMLElement> = {};
-  let hasChildRows;
+  let hasChildRows = props.hasChildItems || node.hasChildNodes;
   let hasLink = state.selectionManager.isLink(node.key);
   if (node != null && 'expandedKeys' in state) {
-    // TODO: ideally node.hasChildNodes would be a way to tell if a row has child nodes, but the row's contents make it so that value is always
-    // true...
-    let children = state.collection.getChildren?.(node.key);
-    hasChildRows = [...(children ?? [])].length > 1;
     if (onAction == null && !hasLink && state.selectionManager.selectionMode === 'none' && hasChildRows) {
       onAction = () => state.toggleKey(node.key);
     }

packages/@react-spectrum/s2/src/TreeView.tsx

@@ -294,24 +294,19 @@ const treeRowFocusIndicator = raw(`
   }`
 );
 
-let TreeItemContext = createContext<{hasChildItems?: boolean}>({});
-
 export const TreeViewItem = <T extends object>(props: TreeViewItemProps<T>) => {
   let {
     href
   } = props;
   let {isDetached, isEmphasized} = useContext(InternalTreeContext);
 
   return (
-    // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful
-    <TreeItemContext.Provider value={{hasChildItems: !!props.childItems}}>
-      <UNSTABLE_TreeItem
-        {...props}
-        className={(renderProps) => treeRow({
-          ...renderProps,
-          isLink: !!href, isEmphasized
-        }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} />
-    </TreeItemContext.Provider>
+    <UNSTABLE_TreeItem
+      {...props}
+      className={(renderProps) => treeRow({
+        ...renderProps,
+        isLink: !!href, isEmphasized
+      }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} />
   );
 };
 
@@ -320,7 +315,6 @@ export const TreeItemContent = (props: Omit<TreeItemContentProps, 'children'> &
     children
   } = props;
   let {isDetached, isEmphasized} = useContext(InternalTreeContext);
-  let {hasChildItems} = useContext(TreeItemContext);
 
   return (
     <UNSTABLE_TreeItemContent>
@@ -348,7 +342,7 @@ export const TreeItemContent = (props: Omit<TreeItemContentProps, 'children'> &
                 width: '[calc(calc(var(--tree-item-level, 0) - 1) * var(--indent))]'
               })} />
             {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */}
-            {(hasChildRows || hasChildItems) && <ExpandableRowChevron isDisabled={isDisabled} isExpanded={isExpanded} />}
+            {hasChildRows && <ExpandableRowChevron isDisabled={isDisabled} isExpanded={isExpanded} />}
             <Provider
               values={[
                 [TextContext, {styles: treeContent}],
@@ -357,8 +351,7 @@ export const TreeItemContent = (props: Omit<TreeItemContentProps, 'children'> &
                   styles: style({size: fontRelative(20), flexShrink: 0})
                 }],
                 [ActionButtonGroupContext, {styles: treeActions}],
-                [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}],
-                [TreeItemContext, {}]
+                [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}]
               ]}>
               {children}
             </Provider>

packages/@react-spectrum/tree/src/TreeView.tsx

@@ -17,7 +17,7 @@ import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium';
 import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium';
 import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared';
 import {isAndroid} from '@react-aria/utils';
-import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react';
+import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useRef} from 'react';
 import {SlotProvider, useDOMRef, useStyleProps} from '@react-spectrum/utils';
 import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'};
 import {useButton} from '@react-aria/button';
@@ -40,8 +40,6 @@ export interface SpectrumTreeViewProps<T> extends Omit<AriaTreeGridListProps<T>,
 export interface SpectrumTreeViewItemProps<T extends object = object> extends Omit<TreeItemProps, 'className' | 'style' | 'value' | 'onHoverStart' | 'onHoverEnd' | 'onHoverChange'> {
   /** Rendered contents of the tree item or child items. */
   children: ReactNode,
-  /** Whether this item has children, even if not loaded yet. */
-  hasChildItems?: boolean,
   /** A list of child tree item objects used when dynamically rendering the tree item children. */
   childItems?: Iterable<T>
 }
@@ -219,23 +217,18 @@ const treeRowOutline = style({
   }
 });
 
-let TreeItemContext = createContext<{hasChildItems?: boolean}>({});
-
 export const TreeViewItem = <T extends object>(props: SpectrumTreeViewItemProps<T>) => {
   let {
     href
   } = props;
 
   return (
-    // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful
-    <TreeItemContext.Provider value={{hasChildItems: !!props.childItems || props.hasChildItems}}>
-      <UNSTABLE_TreeItem
-        {...props}
-        className={renderProps => treeRow({
-          ...renderProps,
-          isLink: !!href
-        })} />
-    </TreeItemContext.Provider>
+    <UNSTABLE_TreeItem
+      {...props}
+      className={renderProps => treeRow({
+        ...renderProps,
+        isLink: !!href
+      })} />
   );
 };
 
@@ -244,7 +237,6 @@ export const TreeItemContent = (props: Omit<TreeItemContentProps, 'children'> &
   let {
     children
   } = props;
-  let {hasChildItems} = useContext(TreeItemContext);
 
   return (
     <UNSTABLE_TreeItemContent>
@@ -260,7 +252,7 @@ export const TreeItemContent = (props: Omit<TreeItemContentProps, 'children'> &
             )}
           <div style={{gridArea: 'level-padding', marginInlineEnd: `calc(${level - 1} * var(--spectrum-global-dimension-size-200))`}} />
           {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */}
-          {(hasChildRows || hasChildItems) && <ExpandableRowChevron isDisabled={isDisabled} isExpanded={isExpanded} />}
+          {hasChildRows && <ExpandableRowChevron isDisabled={isDisabled} isExpanded={isExpanded} />}
           <SlotProvider
             slots={{
               text: {UNSAFE_className: treeContent({isDisabled})},
@@ -278,9 +270,7 @@ export const TreeItemContent = (props: Omit<TreeItemContentProps, 'children'> &
               },
               actionMenu: {UNSAFE_className: treeActionMenu(), UNSAFE_style: {marginInlineEnd: '.5rem'}, isQuiet: true}
             }}>
-            <TreeItemContext.Provider value={{}}>
-              {children}
-            </TreeItemContext.Provider>
+            {children}
           </SlotProvider>
           <div className={treeRowOutline({isFocusVisible, isSelected})} />
         </div>

packages/@react-spectrum/tree/test/TreeView.test.tsx

@@ -480,7 +480,7 @@ describe('Tree', () => {
     expect(rows[0]).toHaveAttribute('aria-label', 'Test');
     // Until the row gets children, don't mark it with the aria/data attributes.
     expect(rows[0]).not.toHaveAttribute('aria-expanded');
-    expect(rows[0]).not.toHaveAttribute('data-has-child-rows');
+    expect(rows[0]).toHaveAttribute('data-has-child-rows');
     expect(chevron).toBeTruthy();
   });
 

packages/@react-stately/grid/src/useGridState.ts

@@ -97,9 +97,10 @@ export function useGridState<T extends object, C extends GridCollection<T>>(prop
         }
       }
       if (newRow) {
-        const childNodes = newRow.hasChildNodes ? [...getChildNodes(newRow, collection)] : [];
+        let hasChildNodes = newRow.firstChildKey === undefined ? newRow.hasChildNodes : newRow.firstChildKey != null;
+        const childNodes = hasChildNodes ? [...getChildNodes(newRow, collection)] : [];
         const keyToFocus =
-          newRow.hasChildNodes &&
+          hasChildNodes &&
           parentNode !== node &&
           node &&
           node.index < childNodes.length ?

packages/@react-stately/selection/src/SelectionManager.ts

@@ -398,7 +398,7 @@ export class SelectionManager implements MultipleSelectionManager {
           }
 
           // Add child keys. If cell selection is allowed, then include item children too.
-          if (item?.hasChildNodes && (this.allowsCellSelection || item.type !== 'item')) {
+          if (item && (item?.firstChildKey === undefined ? item?.hasChildNodes : item?.firstChildKey != null) && (this.allowsCellSelection || item.type !== 'item')) {
             addKeys(getFirstItem(getChildNodes(item, this.collection))?.key ?? null);
           }
         }

packages/@react-types/shared/src/collections.d.ts

@@ -215,6 +215,10 @@ export interface Node<T> {
   prevKey?: Key | null,
   /** The key of the node after this node. */
   nextKey?: Key | null,
+  /** The key of the first child node. */
+  firstChildKey?: Key | null,
+  /** The key of the last child node. */
+  lastChildKey?: Key | null,
   /** Additional properties specific to a particular node type. */
   props?: any,
   /** @private */

packages/react-aria-components/src/Table.tsx

@@ -58,7 +58,7 @@ class TableCollection<T> extends BaseCollection<T> implements ITableCollection<T
       switch (node.type) {
         case 'column':
           columnKeyMap.set(node.key, node);
-          if (!node.hasChildNodes) {
+          if (node.firstChildKey == null) {
             node.index = this.columns.length;
             this.columns.push(node);
 

packages/react-aria-components/src/Tree.tsx

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {AriaTreeGridListProps, useTreeGridList, useTreeGridListItem} from '@react-aria/tree';
+import {AriaTreeGridListItemOptions, AriaTreeGridListProps, useTreeGridList, useTreeGridListItem} from '@react-aria/tree';
 import {ButtonContext} from './Button';
 import {CheckboxContext} from './RSPContexts';
 import {Collection, CollectionBuilder, CollectionNode, createBranchComponent, createLeafComponent, useCachedChildren} from '@react-aria/collections';
@@ -302,7 +302,7 @@ export const UNSTABLE_TreeItemContent = /*#__PURE__*/ createLeafComponent('conte
 
 export const TreeItemContentContext = createContext<TreeItemContentRenderProps | null>(null);
 
-export interface TreeItemProps<T = object> extends StyleRenderProps<TreeItemRenderProps>, LinkDOMProps, HoverEvents {
+export interface TreeItemProps<T = object> extends StyleRenderProps<TreeItemRenderProps>, LinkDOMProps, HoverEvents, Pick<AriaTreeGridListItemOptions, 'hasChildItems'> {
   /** The unique id of the tree row. */
   id?: Key,
   /** The object value that this tree item represents. When using dynamic collections, this is set automatically. */
@@ -325,7 +325,7 @@ export const UNSTABLE_TreeItem = /*#__PURE__*/ createBranchComponent('item', <T
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   let {rowProps, gridCellProps, expandButtonProps, descriptionProps, ...states} = useTreeGridListItem({node: item}, state, ref);
   let isExpanded = rowProps['aria-expanded'] === true;
-  let hasChildRows = [...state.collection.getChildren!(item.key)]?.length > 1;
+  let hasChildRows = props.hasChildItems || item.hasChildNodes;
   let level = rowProps['aria-level'] || 1;
 
   let {hoverProps, isHovered} = useHover({

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

@@ -668,10 +668,8 @@ describe('Table', () => {
 
     let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')});
     await user.tab();
-    fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'});
-    fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'});
-    fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
-    fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
+    await user.keyboard('{ArrowDown}');
+    await user.keyboard('{ArrowRight}');
 
     let gridRows = tableTester.rows;
     expect(gridRows).toHaveLength(4);