From 390b6b048de5df44d0a2a24ebd0f13c634f3caaf Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 22 May 2025 17:05:44 -0700 Subject: [PATCH 1/7] Update tree and listlayout to handle multi loaders --- .../@react-stately/layout/src/ListLayout.ts | 36 ++-- packages/react-aria-components/src/Tree.tsx | 59 +++++-- .../stories/Tree.stories.tsx | 167 ++++++++++++++++-- 3 files changed, 222 insertions(+), 40 deletions(-) diff --git a/packages/@react-stately/layout/src/ListLayout.ts b/packages/@react-stately/layout/src/ListLayout.ts index 7930e10a2e4..1b2b52fa685 100644 --- a/packages/@react-stately/layout/src/ListLayout.ts +++ b/packages/@react-stately/layout/src/ListLayout.ts @@ -253,9 +253,11 @@ export class ListLayout exte protected buildCollection(y = this.padding): LayoutNode[] { let collection = this.virtualizer!.collection; - let skipped = 0; + let collectionNodes = [...collection]; + let loaderNodes = collectionNodes.filter(node => node.type === 'loader'); let nodes: LayoutNode[] = []; - let isEmptyOrLoading = collection?.size === 0 || (collection.size === 1 && collection.getItem(collection.getFirstKey()!)!.type === 'loader'); + + let isEmptyOrLoading = collection?.size === 0 || !collectionNodes.some(item => item.type !== 'loader'); if (isEmptyOrLoading) { y = 0; } @@ -265,29 +267,33 @@ export class ListLayout exte // Skip rows before the valid rectangle unless they are already cached. if (node.type === 'item' && y + rowHeight < this.requestedRect.y && !this.isValid(node, y)) { y += rowHeight; - skipped++; continue; } let layoutNode = this.buildChild(node, this.padding, y, null); y = layoutNode.layoutInfo.rect.maxY + this.gap; nodes.push(layoutNode); - if (node.type === 'item' && y > this.requestedRect.maxY) { - let itemsAfterRect = collection.size - (nodes.length + skipped); - let lastNode = collection.getItem(collection.getLastKey()!); - if (lastNode?.type === 'loader') { - itemsAfterRect--; - } - - y += itemsAfterRect * rowHeight; + if (node.type === 'loader') { + let index = loaderNodes.indexOf(node); + loaderNodes.splice(index, 1); + } - // Always add the loader sentinel if present. This assumes the loader is the last option/row - // will need to refactor when handling multi section loading - if (lastNode?.type === 'loader' && nodes.at(-1)?.layoutInfo.type !== 'loader') { - let loader = this.buildChild(lastNode, this.padding, y, null); + // Build each loader that exists in the collection that is outside the visible rect so that they are persisted + // at the proper estimated location + if (y > this.requestedRect.maxY) { + let lastProcessedIndex = collectionNodes.indexOf(node); + for (let loaderNode of loaderNodes) { + let loaderNodeIndex = collectionNodes.indexOf(loaderNode); + // Subtract by an addition 1 since we've already added the current item's height to y + y += (loaderNodeIndex - lastProcessedIndex - 1) * rowHeight; + let loader = this.buildChild(loaderNode, this.padding, y, null); nodes.push(loader); y = loader.layoutInfo.rect.maxY; + lastProcessedIndex = loaderNodeIndex; } + + // Account for the rest of the items after the last loader spinner, subtract by 1 since we've processed the current node's height already + y += (collectionNodes.length - lastProcessedIndex - 1) * rowHeight; break; } } diff --git a/packages/react-aria-components/src/Tree.tsx b/packages/react-aria-components/src/Tree.tsx index f3f012ad81f..217a77aae66 100644 --- a/packages/react-aria-components/src/Tree.tsx +++ b/packages/react-aria-components/src/Tree.tsx @@ -20,7 +20,7 @@ import {DisabledBehavior, DragPreviewRenderer, Expandable, forwardRefType, Hover import {DragAndDropContext, DropIndicatorContext, useDndPersistedKeys, useRenderDropIndicator} from './DragAndDrop'; import {DragAndDropHooks} from './useDragAndDrop'; import {DraggableCollectionState, DroppableCollectionState, Collection as ICollection, Node, SelectionBehavior, TreeState, useTreeState} from 'react-stately'; -import {filterDOMProps, useObjectRef} from '@react-aria/utils'; +import {filterDOMProps, inertValue, LoadMoreSentinelProps, UNSTABLE_useLoadMoreSentinel, useObjectRef} from '@react-aria/utils'; import React, {createContext, ForwardedRef, forwardRef, JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; import {useControlledState} from '@react-stately/utils'; @@ -686,10 +686,31 @@ export interface UNSTABLE_TreeLoadingIndicatorRenderProps { level: number } -export interface TreeLoaderProps extends RenderProps, StyleRenderProps {} +export interface TreeLoaderProps extends Omit, RenderProps { + /** + * The load more spinner to render when loading additional items. + */ + children?: ReactNode | ((values: UNSTABLE_TreeLoadingIndicatorRenderProps & {defaultChildren: ReactNode | undefined}) => ReactNode), + /** + * Whether or not the loading spinner should be rendered or not. + */ + isLoading?: boolean +} export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', function TreeLoader(props: TreeLoaderProps, ref: ForwardedRef, item: Node) { let state = useContext(TreeStateContext)!; + let {isLoading, onLoadMore, scrollOffset, ...otherProps} = props; + let sentinelRef = useRef(null); + let memoedLoadMoreProps = useMemo(() => ({ + onLoadMore, + // TODO: this collection will update anytime a row is expanded/collapsed becaused the flattenedRows will change. + // This means onLoadMore will trigger but that might be ok cause the user should have logic to handle multiple loadMore calls + collection: state?.collection, + sentinelRef, + scrollOffset + }), [onLoadMore, scrollOffset, state?.collection]); + UNSTABLE_useLoadMoreSentinel(memoedLoadMoreProps, sentinelRef); + // This loader row is is non-interactable, but we want the same aria props calculated as a typical row // @ts-ignore let {rowProps} = useTreeItem({node: item}, state, ref); @@ -702,7 +723,7 @@ export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', funct }; let renderProps = useRenderProps({ - ...props, + ...otherProps, id: undefined, children: item.rendered, defaultClassName: 'react-aria-TreeLoader', @@ -713,16 +734,23 @@ export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', funct return ( <> -
-
- {renderProps.children} -
+ {/* Alway render the sentinel. For now onus is on the user for styling when using flex + gap (this would introduce a gap even though it doesn't take room) */} + {/* @ts-ignore - compatibility with React < 19 */} +
+
+ {isLoading && renderProps.children && ( +
+
+ {renderProps.children} +
+
+ )} ); }); @@ -775,9 +803,10 @@ function flattenTree(collection: TreeCollection, opts: TreeGridCollectionO keyMap.set(node.key, node as CollectionNode); } - if (node.level === 0 || (parentKey != null && expandedKeys.has(parentKey) && flattenedRows.find(row => row.key === parentKey))) { - // Grab the modified node from the key map so our flattened list and modified key map point to the same nodes - flattenedRows.push(keyMap.get(node.key) || node); + // Grab the modified node from the key map so our flattened list and modified key map point to the same nodes + let modifiedNode = keyMap.get(node.key) || node; + if (modifiedNode.level === 0 || (modifiedNode.parentKey != null && expandedKeys.has(modifiedNode.parentKey) && flattenedRows.find(row => row.key === modifiedNode.parentKey))) { + flattenedRows.push(modifiedNode); } } else if (node.type !== null) { keyMap.set(node.key, node as CollectionNode); diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 8c6e94b1c1a..b74c293e20f 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -14,10 +14,10 @@ import {action} from '@storybook/addon-actions'; import {Button, Checkbox, CheckboxProps, Collection, DroppableCollectionReorderEvent, isTextDropItem, Key, ListLayout, Menu, MenuTrigger, Popover, Text, Tree, TreeItem, TreeItemContent, TreeItemProps, TreeProps, useDragAndDrop, Virtualizer} from 'react-aria-components'; import {classNames} from '@react-spectrum/utils'; import {MyMenuItem} from './utils'; -import React, {ReactNode} from 'react'; +import React, {ReactNode, useCallback, useRef, useState} from 'react'; import styles from '../example/index.css'; import {UNSTABLE_TreeLoadingIndicator} from '../src/Tree'; -import {useTreeData} from '@react-stately/data'; +import {useListData, useTreeData} from '@react-stately/data'; export default { title: 'React Aria Components' @@ -206,9 +206,9 @@ let rows = [ ]} ]; -const MyTreeLoader = () => { +const MyTreeLoader = (props) => { return ( - + {({level}) => { let message = `Level ${level} loading spinner`; if (level === 1) { @@ -278,6 +278,8 @@ const DynamicTreeItem = (props: DynamicTreeItemProps) => { )} + {/* TODO: can make this have loaders by adding making the childItems=asyncList.items + another prop that is isLoading: asyncList.loadingState in rows above. + Will need to refactor the below to then render the loader after the collection */} {(item: any) => ( @@ -392,7 +394,7 @@ function LoadingStoryDepOnCollection(args) { getKey: item => item.id, getChildren: item => item.childItems }); - + return ( @@ -659,11 +661,11 @@ function SecondTree(args) { }); return ( - 'Drop items here'}> {(item: any) => ( @@ -702,3 +704,148 @@ export const TreeWithDragAndDrop = { ...TreeExampleDynamic.argTypes } }; + +let projects: {id: string, value: string}[] = []; +let projectsLevel3: {id: string, value: string}[] = []; +let documents: {id: string, value: string}[] = []; +for (let i = 0; i < 10; i++) { + projects.push({id: `projects-${i}`, value: `Projects-${i}`}); + projectsLevel3.push({id: `project-1-${i}`, value: `Projects-1-${i}`}); + documents.push({id: `document-${i}`, value: `Document-${i}`}); +} + +function MultiLoaderTreeStatic(args) { + let projectsData = useListData({ + initialItems: projects + }); + + let projects3Data = useListData({ + initialItems: projectsLevel3 + }); + + let documentsData = useListData({ + initialItems: documents + }); + + let [isRootLoading, setRootLoading] = useState(false); + let [isProjectsLoading, setProjectsLoading] = useState(false); + let [isProjectsLevel3Loading, setProjects3Loading] = useState(false); + let [isDocumentsLoading, setDocumentsLoading] = useState(false); + + let onRootLoadMore = useCallback(() => { + if (!isRootLoading) { + action('root loading')(); + setRootLoading(true); + setTimeout(() => { + setRootLoading(false); + }, args.delay); + } + }, [isRootLoading, args.delay]); + + let onProjectsLoadMore = useCallback(() => { + if (!isProjectsLoading) { + action('projects loading')(); + setProjectsLoading(true); + setTimeout(() => { + let dataToAppend: {id: string, value: string}[] = []; + let projectsLength = projectsData.items.length; + for (let i = 0; i < 5; i++) { + dataToAppend.push({id: `projects-${i + projectsLength}`, value: `Projects-${i + projectsLength}`}); + } + projectsData.append(...dataToAppend); + setProjectsLoading(false); + }, args.delay); + } + }, [isProjectsLoading, projectsData, args.delay]); + + let onProjectsLevel3LoadMore = useCallback(() => { + if (!isProjectsLevel3Loading) { + action('projects level 3 loading')(); + setProjects3Loading(true); + setTimeout(() => { + let dataToAppend: {id: string, value: string}[] = []; + let projects3Length = projects3Data.items.length; + for (let i = 0; i < 5; i++) { + dataToAppend.push({id: `project-1-${i + projects3Length}`, value: `Project-1-${i + projects3Length}`}); + } + projects3Data.append(...dataToAppend); + setProjects3Loading(false); + }, args.delay); + } + }, [isProjectsLevel3Loading, projects3Data, args.delay]); + + let onDocumentsLoadMore = useCallback(() => { + if (!isDocumentsLoading) { + action('documents loading')(); + setDocumentsLoading(true); + setTimeout(() => { + let dataToAppend: {id: string, value: string}[] = []; + let documentsLength = documentsData.items.length; + for (let i = 0; i < 5; i++) { + dataToAppend.push({id: `document-${i + documentsLength}`, value: `Document-${i + documentsLength}`}); + } + documentsData.append(...dataToAppend); + setDocumentsLoading(false); + }, args.delay); + } + }, [isDocumentsLoading, documentsData, args.delay]); + + return ( + + + Photos 1 + Photos 2 + + {/* NOTE: important to provide dependencies here, otherwise the nested level doesn't perform loading updates properly */} + + {(item: any) => { + return item.id !== 'projects-1' ? + ( + + {item.value} + + ) : ( + + + {(item: any) => ( + + {item.value} + + )} + + + + ); + } + } + + + + Photos 3 + Photos 4 + + + {(item: any) => ( + + {item.value} + + )} + + + + Photos 5 + Photos 6 + + + + ); +} + +export const VirtualizedTreeMultiLoader = { + render: MultiLoaderTreeStatic, + args: { + delay: 2000 + } +}; From 50c6f1428d8983c17c427f83a6b9ed3eaee2b846 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 23 May 2025 11:36:16 -0700 Subject: [PATCH 2/7] adapting other stories to new loader api and adding useAsync example story --- .../test/SearchAutocomplete.test.js | 4 +- .../stories/Tree.stories.tsx | 265 ++++++++++++++---- 2 files changed, 206 insertions(+), 63 deletions(-) diff --git a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js index db972fd07ce..f39231cd8ca 100644 --- a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js +++ b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js @@ -1844,7 +1844,7 @@ describe('SearchAutocomplete', function () { expect(() => within(tray).getByText('No results')).toThrow(); }); - it.skip('user can select options by pressing them', async function () { + it('user can select options by pressing them', async function () { let {getByRole, getByText, getByTestId} = renderSearchAutocomplete(); let button = getByRole('button'); @@ -1892,7 +1892,7 @@ describe('SearchAutocomplete', function () { expect(items[1]).toHaveAttribute('aria-selected', 'true'); }); - it.skip('user can select options by focusing them and hitting enter', async function () { + it('user can select options by focusing them and hitting enter', async function () { let {getByRole, getByText, getByTestId} = renderSearchAutocomplete(); let button = getByRole('button'); diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index b74c293e20f..a01e6694ef1 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -14,10 +14,10 @@ import {action} from '@storybook/addon-actions'; import {Button, Checkbox, CheckboxProps, Collection, DroppableCollectionReorderEvent, isTextDropItem, Key, ListLayout, Menu, MenuTrigger, Popover, Text, Tree, TreeItem, TreeItemContent, TreeItemProps, TreeProps, useDragAndDrop, Virtualizer} from 'react-aria-components'; import {classNames} from '@react-spectrum/utils'; import {MyMenuItem} from './utils'; -import React, {ReactNode, useCallback, useRef, useState} from 'react'; +import React, {ReactNode, useCallback, useState} from 'react'; import styles from '../example/index.css'; import {UNSTABLE_TreeLoadingIndicator} from '../src/Tree'; -import {useListData, useTreeData} from '@react-stately/data'; +import {useAsyncList, useListData, useTreeData} from '@react-stately/data'; export default { title: 'React Aria Components' @@ -207,9 +207,14 @@ let rows = [ ]; const MyTreeLoader = (props) => { + let {omitChildren} = props; return ( {({level}) => { + if (omitChildren) { + return; + } + let message = `Level ${level} loading spinner`; if (level === 1) { message = 'Load more spinner'; @@ -228,12 +233,15 @@ interface DynamicTreeItemProps extends TreeItemProps { children: ReactNode, childItems?: Iterable, isLoading?: boolean, + onLoadMore?: () => void, renderLoader?: (id: Key | undefined) => boolean, - supportsDragging?: boolean + supportsDragging?: boolean, + isLastInRoot?: boolean } const DynamicTreeItem = (props: DynamicTreeItemProps) => { let {childItems, renderLoader, supportsDragging} = props; + return ( <> { )} - {/* TODO: can make this have loaders by adding making the childItems=asyncList.items + another prop that is isLoading: asyncList.loadingState in rows above. - Will need to refactor the below to then render the loader after the collection */} {(item: any) => ( - - {item.value.name} + + {item.name ?? item.value.name} )} + {renderLoader?.(props.id) && } - {/* TODO this would need to check if the parent was loading and then the user would insert this tree loader after last row of that section. - theoretically this would look like (loadingKeys.includes(parentKey) && props.id === last key of parent) &&.... - both the parentKey of a given item as well as checking if the current tree item is the last item of said parent would need to be done by the user outside of this tree item? - */} - {props.isLoading && renderLoader?.(props.id) && } + {props.isLastInRoot && } ); }; @@ -372,7 +375,7 @@ const EmptyTreeStatic = (args: {isLoading: boolean}) => ( renderEmptyState={() => renderEmptyLoader({isLoading: args.isLoading})}> {(item: any) => ( - id === 'project-2C'} isLoading={args.isLoading} id={item.id} childItems={item.childItems} textValue={item.name}> + id === 'project-2'} isLoading={args.isLoading} id={item.id} childItems={item.childItems} textValue={item.name}> {item.name} )} @@ -399,12 +402,12 @@ function LoadingStoryDepOnCollection(args) { {(item) => ( - id === 'project-2C'} isLoading={args.isLoading} id={item.key} childItems={item.children ?? []} textValue={item.value.name}> + id === 'project-2'} isLoading={args.isLoading} id={item.key} childItems={item.children ?? []} textValue={item.value.name}> {item.value.name} )} - {args.isLoading && } + ); } @@ -432,7 +435,7 @@ function LoadingStoryDepOnTop(args: TreeProps & {isLoading: boolean}) { return ( {(item) => ( - (id === 'reports' || id === 'project-2C')} isLoading={args.isLoading} id={item.key} childItems={item.children ?? []} textValue={item.value.name}> + (id === 'root' || id === 'project-2')} isLoading={args.isLoading} id={item.key} childItems={item.children ?? []} textValue={item.value.name}> {item.value.name} )} @@ -514,6 +517,7 @@ const DynamicTreeItemWithButtonLoader = (props: DynamicTreeItemProps) => { )} + {renderLoader?.(props.id) && } ); }; @@ -713,8 +717,22 @@ for (let i = 0; i < 10; i++) { projectsLevel3.push({id: `project-1-${i}`, value: `Projects-1-${i}`}); documents.push({id: `document-${i}`, value: `Document-${i}`}); } +let root = [ + {id: 'photos-1', value: 'Photos 1'}, + {id: 'photos-2', value: 'Photos 2'}, + {id: 'projects', value: 'Projects'}, + {id: 'photos-3', value: 'Photos 3'}, + {id: 'photos-4', value: 'Photos 4'}, + {id: 'documents', value: 'Documents'}, + {id: 'photos-5', value: 'Photos 5'}, + {id: 'photos-6', value: 'Photos 6'} +]; + +function MultiLoaderTreeMockAsync(args) { + let rootData = useListData({ + initialItems: root + }); -function MultiLoaderTreeStatic(args) { let projectsData = useListData({ initialItems: projects }); @@ -737,10 +755,16 @@ function MultiLoaderTreeStatic(args) { action('root loading')(); setRootLoading(true); setTimeout(() => { + let dataToAppend: {id: string, value: string}[] = []; + let rootLength = rootData.items.length - 1; + for (let i = 0; i < 5; i++) { + dataToAppend.push({id: `photos-${i + rootLength}`, value: `Photos-${i + rootLength}`}); + } + rootData.append(...dataToAppend); setRootLoading(false); }, args.delay); } - }, [isRootLoading, args.delay]); + }, [isRootLoading, rootData, args.delay]); let onProjectsLoadMore = useCallback(() => { if (!isProjectsLoading) { @@ -795,57 +819,176 @@ function MultiLoaderTreeStatic(args) { - Photos 1 - Photos 2 - - {/* NOTE: important to provide dependencies here, otherwise the nested level doesn't perform loading updates properly */} - - {(item: any) => { - return item.id !== 'projects-1' ? - ( - - {item.value} - - ) : ( - - - {(item: any) => ( - - {item.value} - - )} - - - - ); + + {(item: any) => { + if (item.id === 'projects') { + return ( + + {/* NOTE: important to provide dependencies here, otherwise the nested level doesn't perform loading updates properly */} + + {(item: any) => { + return item.id !== 'projects-1' ? + ( + + {item.value} + + ) : ( + + + {(item: any) => ( + + {item.value} + + )} + + + + ); + } + } + + + + ); + } else if (item.id === 'documents') { + return ( + + + {(item: any) => ( + + {item.value} + + )} + + + + ); + } else { + return ( + {item.value} + ); } - } - - - - Photos 3 - Photos 4 - - - {(item: any) => ( - - {item.value} - - )} - - - - Photos 5 - Photos 6 + }} + + + + + ); +} + +export const VirtualizedTreeMultiLoaderMockAsync = { + render: MultiLoaderTreeMockAsync, + args: { + delay: 2000 + } +}; + +interface Character { + name: string, + height: number, + mass: number, + birth_year: number +} + +function MultiLoaderTreeUseAsyncList(args) { + let root = [ + {id: 'photos-1', name: 'Photos 1'}, + {id: 'photos-2', name: 'Photos 2'}, + {id: 'photos-3', name: 'Photos 3'}, + {id: 'photos-4', name: 'Photos 4'}, + {id: 'starwars', name: 'Star Wars'}, + {id: 'photos-5', name: 'Photos 5'}, + {id: 'photos-6', name: 'Photos 6'} + ]; + + let rootData = useListData({ + initialItems: root + }); + + let starWarsList = useAsyncList({ + async load({signal, cursor, filterText}) { + if (cursor) { + cursor = cursor.replace(/^http:\/\//i, 'https://'); + } + + action('starwars loading')(); + await new Promise(resolve => setTimeout(resolve, args.delay)); + let res = await fetch(cursor || `https://swapi.py4e.com/api/people/?search=${filterText}`, {signal}); + let json = await res.json(); + + return { + items: json.results, + cursor: json.next + }; + } + }); + + let [isRootLoading, setRootLoading] = useState(false); + let onRootLoadMore = useCallback(() => { + if (!isRootLoading) { + action('root loading')(); + setRootLoading(true); + setTimeout(() => { + let dataToAppend: {id: string, name: string}[] = []; + let rootLength = rootData.items.length; + for (let i = 0; i < 5; i++) { + dataToAppend.push({id: `photos-${i + rootLength}`, name: `Photos-${i + rootLength}`}); + } + rootData.append(...dataToAppend); + setRootLoading(false); + }, args.delay); + } + }, [isRootLoading, rootData, args.delay]); + + return ( + + + + {(item: any) => ( + id === 'starwars'} + isLoading={item.id === 'starwars' ? starWarsList.isLoading : undefined} + onLoadMore={item.id === 'starwars' ? starWarsList.loadMore : undefined} + id={item.id} + childItems={item.id === 'starwars' ? starWarsList.items : []} + textValue={item.name}> + {item.name} + + )} + ); } -export const VirtualizedTreeMultiLoader = { - render: MultiLoaderTreeStatic, +export const VirtualizedTreeMultiLoaderUseAsyncList = { + render: MultiLoaderTreeUseAsyncList, args: { delay: 2000 } }; + +// TODO: A fully dynamic render case like below seems to have problems with dupe keys for some reason +// Either way it feels more ergonomic to use Collection and place your loading sentinel after it anyways +{/* +{(item) => { + return ( + id === 'starwars'} + isLoading={item.id === 'starwars' ? starWarsList.isLoading : isRootLoading} + onLoadMore={item.id === 'starwars' ? starWarsList.loadMore : onRootLoadMore} + id={item.id} + childItems={item.id === 'starwars' ? starWarsList.items : []} + textValue={item.name}> + {item.name} + + ); +}} */} From f99e4bdd9418bd26ef17d08433cffe8115885a3c Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 23 May 2025 15:09:32 -0700 Subject: [PATCH 3/7] add tests --- packages/react-aria-components/src/Tree.tsx | 8 +- packages/react-aria-components/src/index.ts | 2 +- .../stories/Tree.stories.tsx | 9 +- .../react-aria-components/test/Tree.test.tsx | 413 +++++++++++++++++- 4 files changed, 415 insertions(+), 17 deletions(-) diff --git a/packages/react-aria-components/src/Tree.tsx b/packages/react-aria-components/src/Tree.tsx index 217a77aae66..e2582718467 100644 --- a/packages/react-aria-components/src/Tree.tsx +++ b/packages/react-aria-components/src/Tree.tsx @@ -678,7 +678,7 @@ export const TreeItem = /*#__PURE__*/ createBranchComponent('item', , RenderProps { +export interface TreeLoadingSentinelProps extends Omit, RenderProps { /** * The load more spinner to render when loading additional items. */ - children?: ReactNode | ((values: UNSTABLE_TreeLoadingIndicatorRenderProps & {defaultChildren: ReactNode | undefined}) => ReactNode), + children?: ReactNode | ((values: UNSTABLE_TreeLoadingSentinelRenderProps & {defaultChildren: ReactNode | undefined}) => ReactNode), /** * Whether or not the loading spinner should be rendered or not. */ isLoading?: boolean } -export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', function TreeLoader(props: TreeLoaderProps, ref: ForwardedRef, item: Node) { +export const UNSTABLE_TreeLoadingSentinel = createLeafComponent('loader', function TreeLoadingSentinel(props: TreeLoadingSentinelProps, ref: ForwardedRef, item: Node) { let state = useContext(TreeStateContext)!; let {isLoading, onLoadMore, scrollOffset, ...otherProps} = props; let sentinelRef = useRef(null); diff --git a/packages/react-aria-components/src/index.ts b/packages/react-aria-components/src/index.ts index faa0787494d..472dd7a52fd 100644 --- a/packages/react-aria-components/src/index.ts +++ b/packages/react-aria-components/src/index.ts @@ -75,7 +75,7 @@ export {ToggleButton, ToggleButtonContext} from './ToggleButton'; export {ToggleButtonGroup, ToggleButtonGroupContext, ToggleGroupStateContext} from './ToggleButtonGroup'; export {Toolbar, ToolbarContext} from './Toolbar'; export {TooltipTrigger, Tooltip, TooltipTriggerStateContext, TooltipContext} from './Tooltip'; -export {UNSTABLE_TreeLoadingIndicator, Tree, TreeItem, TreeContext, TreeItemContent, TreeStateContext} from './Tree'; +export {UNSTABLE_TreeLoadingSentinel, Tree, TreeItem, TreeContext, TreeItemContent, TreeStateContext} from './Tree'; export {useDragAndDrop} from './useDragAndDrop'; export {DropIndicator, DropIndicatorContext, DragAndDropContext} from './DragAndDrop'; export {Virtualizer} from './Virtualizer'; diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index a01e6694ef1..60a7dff601a 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -16,7 +16,7 @@ import {classNames} from '@react-spectrum/utils'; import {MyMenuItem} from './utils'; import React, {ReactNode, useCallback, useState} from 'react'; import styles from '../example/index.css'; -import {UNSTABLE_TreeLoadingIndicator} from '../src/Tree'; +import {UNSTABLE_TreeLoadingSentinel} from '../src/Tree'; import {useAsyncList, useListData, useTreeData} from '@react-stately/data'; export default { @@ -209,7 +209,7 @@ let rows = [ const MyTreeLoader = (props) => { let {omitChildren} = props; return ( - + {({level}) => { if (omitChildren) { return; @@ -225,7 +225,7 @@ const MyTreeLoader = (props) => { ); }} - + ); }; @@ -819,7 +819,8 @@ function MultiLoaderTreeMockAsync(args) { - + {/* TODO: wonder if there is something we can do to ensure that these depenedcies are provided, need to dig to make sure if there is an alternative */} + {(item: any) => { if (item.id === 'projects') { return ( diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index 6640907afd1..df288db37a2 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -10,12 +10,13 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, mockClickDefault, pointerMap, render, within} from '@react-spectrum/test-utils-internal'; +import {act, fireEvent, mockClickDefault, pointerMap, render, setupIntersectionObserverMock, within} from '@react-spectrum/test-utils-internal'; import {AriaTreeTests} from './AriaTree.test-util'; -import {Button, Checkbox, Collection, ListLayout, Text, Tree, TreeItem, TreeItemContent, Virtualizer} from '../'; +import {Button, Checkbox, Collection, ListLayout, Text, Tree, TreeItem, TreeItemContent, UNSTABLE_TreeLoadingSentinel, Virtualizer} from '../'; import {composeStories} from '@storybook/react'; import React from 'react'; import * as stories from '../stories/Tree.stories'; +import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; let { @@ -136,6 +137,7 @@ let DynamicTree = ({treeProps = {}, rowProps = {}}) => ( describe('Tree', () => { let user; + let testUtilUser = new User(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); @@ -1199,15 +1201,410 @@ describe('Tree', () => { }); }); + describe('loading sentinels', () => { + let LoadingSentinelTree = (props) => { + let {isLoading, onLoadMore, ...treeProps} = props; + + return ( + + Photos + + + + Projects-1A + + + Loading... + + + + Projects-2 + + + Projects-3 + + + + Loading... + + + ); + }; + + let onLoadMore = jest.fn(); + let observe = jest.fn(); + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should render the loading elements when loading', async () => { + let tree = render(); + + let treeTester = testUtilUser.createTester('Tree', {root: tree.getByRole('treegrid')}); + let rows = treeTester.rows; + expect(rows).toHaveLength(3); + let loaderRow = rows[2]; + expect(loaderRow).toHaveTextContent('Loading...'); + + let sentinel = tree.getByTestId('loadMoreSentinel'); + expect(sentinel.closest('[inert]')).toBeTruthy; + + // Should render the second sentinel if the row is expanded + tree.rerender(); + rows = treeTester.rows; + expect(rows).toHaveLength(8); + let newLoaderRow = rows[4]; + expect(newLoaderRow).toHaveTextContent('Loading...'); + + loaderRow = rows[7]; + expect(loaderRow).toHaveTextContent('Loading...'); + let sentinels = tree.queryAllByTestId('loadMoreSentinel'); + expect(sentinels).toHaveLength(2); + }); + + it('should render the sentinel but not the loading indicator when not loading', async () => { + let tree = render(); + + let treeTester = testUtilUser.createTester('Tree', {root: tree.getByRole('treegrid')}); + let rows = treeTester.rows; + expect(rows).toHaveLength(2); + expect(tree.queryByText('Loading...')).toBeFalsy(); + expect(tree.getByTestId('loadMoreSentinel')).toBeInTheDocument(); + }); + + it('should only fire loadMore when intersection is detected regardless of loading state', async () => { + let observer = setupIntersectionObserverMock({ + observe + }); + + let tree = render(); + let sentinel = tree.getByTestId('loadMoreSentinel'); + expect(observe).toHaveBeenLastCalledWith(sentinel); + expect(onLoadMore).toHaveBeenCalledTimes(0); + + act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);}); + expect(onLoadMore).toHaveBeenCalledTimes(1); + observe.mockClear(); + + tree.rerender(); + expect(observe).toHaveBeenLastCalledWith(sentinel); + expect(onLoadMore).toHaveBeenCalledTimes(1); + + act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);}); + expect(onLoadMore).toHaveBeenCalledTimes(2); + }); + + describe('virtualized', () => { + let projects: {id: string, value: string}[] = []; + let projectsLevel3: {id: string, value: string}[] = []; + let documents: {id: string, value: string}[] = []; + for (let i = 0; i < 10; i++) { + projects.push({id: `projects-${i}`, value: `Projects-${i}`}); + projectsLevel3.push({id: `project-1-${i}`, value: `Projects-1-${i}`}); + documents.push({id: `document-${i}`, value: `Document-${i}`}); + } + let root = [ + {id: 'photos-1', value: 'Photos 1'}, + {id: 'photos-2', value: 'Photos 2'}, + {id: 'projects', value: 'Projects'}, + {id: 'photos-3', value: 'Photos 3'}, + {id: 'photos-4', value: 'Photos 4'}, + {id: 'documents', value: 'Documents'}, + {id: 'photos-5', value: 'Photos 5'}, + {id: 'photos-6', value: 'Photos 6'} + ]; + let clientWidth, clientHeight; + + beforeAll(() => { + clientWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100); + clientHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100); + }); + + afterAll(function () { + clientWidth.mockReset(); + clientHeight.mockReset(); + }); + + let VirtualizedLoadingSentinelTree = (props) => { + let { + rootData = root, + rootIsLoading, + projectsData = projects, + projectsIsLoading, + projects3Data = projectsLevel3, + projects3IsLoading, + documentData = documents, + documentsIsLoading, + ...treeProps + } = props; + return ( + + + + {(item: any) => { + if (item.id === 'projects') { + return ( + + + {(item: any) => { + return item.id !== 'projects-1' ? + ( + + {item.value} + + ) : ( + + + {(item: any) => ( + + {item.value} + + )} + + + Loading... + + + ); + } + } + + + Loading... + + + ); + } else if (item.id === 'documents') { + return ( + + + {(item: any) => ( + + {item.value} + + )} + + + Loading... + + + ); + } else { + return ( + {item.value} + ); + } + }} + + + Loading... + + + + ); + }; + + it('should always render the sentinel even when virtualized', async () => { + let tree = render( + + ); + let treeTester = testUtilUser.createTester('Tree', {root: tree.getByRole('treegrid')}); + let rows = treeTester.rows; + expect(rows).toHaveLength(8); + let rootLoaderRow = rows[7]; + expect(rootLoaderRow).toHaveTextContent('Loading...'); + expect(rootLoaderRow).toHaveAttribute('aria-posinset', '9'); + expect(rootLoaderRow).toHaveAttribute('aria-setsize', '9'); + let rootLoaderParentStyles = rootLoaderRow.parentElement!.style; + // 8 items * 25px + expect(rootLoaderParentStyles.top).toBe('200px'); + expect(rootLoaderParentStyles.height).toBe('30px'); + let sentinel = tree.getByTestId('loadMoreSentinel'); + expect(sentinel.closest('[inert]')).toBeTruthy; + let sentinels = tree.queryAllByTestId('loadMoreSentinel'); + expect(sentinels).toHaveLength(1); + + // Expand projects, adding 10 rows to the tree + tree.rerender( + + ); + + rows = treeTester.rows; + expect(rows).toHaveLength(9); + rootLoaderRow = rows[8]; + expect(rootLoaderRow).toHaveAttribute('aria-posinset', '9'); + expect(rootLoaderRow).toHaveAttribute('aria-setsize', '9'); + rootLoaderParentStyles = rootLoaderRow.parentElement!.style; + // 18 items * 25px + intermediate loader * 30px + expect(rootLoaderParentStyles.top).toBe('480px'); + expect(rootLoaderParentStyles.height).toBe('30px'); + let projectsLoader = rows[7]; + expect(projectsLoader).toHaveAttribute('aria-posinset', '11'); + expect(projectsLoader).toHaveAttribute('aria-setsize', '11'); + let projectsLoaderParentStyles = projectsLoader.parentElement!.style; + // 13 items * 25px + expect(projectsLoaderParentStyles.top).toBe('325px'); + expect(projectsLoaderParentStyles.height).toBe('30px'); + sentinels = tree.queryAllByTestId('loadMoreSentinel'); + expect(sentinels).toHaveLength(2); + for (let sentinel of sentinels) { + expect(sentinel.closest('[inert]')).toBeTruthy; + } + + // Expand projects-1, adding 10 rows to the tree + tree.rerender( + + ); + + rows = treeTester.rows; + expect(rows).toHaveLength(10); + rootLoaderRow = rows[9]; + expect(rootLoaderRow).toHaveAttribute('aria-posinset', '9'); + expect(rootLoaderRow).toHaveAttribute('aria-setsize', '9'); + rootLoaderParentStyles = rootLoaderRow.parentElement!.style; + // 28 items * 25px + 2 intermediate loaders * 30px + expect(rootLoaderParentStyles.top).toBe('760px'); + expect(rootLoaderParentStyles.height).toBe('30px'); + // Project loader is still the 2nd to last item that is preserved since the projects-1 is a child folder + projectsLoader = rows[8]; + expect(projectsLoader).toHaveAttribute('aria-posinset', '11'); + expect(projectsLoader).toHaveAttribute('aria-setsize', '11'); + projectsLoaderParentStyles = projectsLoader.parentElement!.style; + // 23 items * 25px + 1 intermediate loaders * 30px + expect(projectsLoaderParentStyles.top).toBe('605px'); + expect(projectsLoaderParentStyles.height).toBe('30px'); + // Project-1 loader is 3rd to last item that is preserved since it is in the child folder of projects + let projects1Loader = rows[7]; + expect(projects1Loader).toHaveAttribute('aria-posinset', '11'); + expect(projects1Loader).toHaveAttribute('aria-setsize', '11'); + let projectsLoader1ParentStyles = projects1Loader.parentElement!.style; + // 15 items * 25px aka photos-1 -> 2 + projects + projects-0 -> 1 + 10 items in the folder + expect(projectsLoader1ParentStyles.top).toBe('375px'); + expect(projectsLoader1ParentStyles.height).toBe('30px'); + sentinels = tree.queryAllByTestId('loadMoreSentinel'); + expect(sentinels).toHaveLength(3); + for (let sentinel of sentinels) { + expect(sentinel.closest('[inert]')).toBeTruthy; + } + + // Expand projects-1, adding 10 rows to the tree + tree.rerender( + + ); + + rows = treeTester.rows; + expect(rows).toHaveLength(11); + rootLoaderRow = rows[10]; + expect(rootLoaderRow).toHaveAttribute('aria-posinset', '9'); + expect(rootLoaderRow).toHaveAttribute('aria-setsize', '9'); + rootLoaderParentStyles = rootLoaderRow.parentElement!.style; + // 38 items * 25px + 3 intermediate loaders * 30px + expect(rootLoaderParentStyles.top).toBe('1040px'); + expect(rootLoaderParentStyles.height).toBe('30px'); + // Project loader is now the 3nd to last item since document's loader is after it + projectsLoader = rows[8]; + expect(projectsLoader).toHaveAttribute('aria-posinset', '11'); + expect(projectsLoader).toHaveAttribute('aria-setsize', '11'); + projectsLoaderParentStyles = projectsLoader.parentElement!.style; + // 23 items * 25px + 1 intermediate loaders * 30px + expect(projectsLoaderParentStyles.top).toBe('605px'); + expect(projectsLoaderParentStyles.height).toBe('30px'); + // Project-1 loader is 4th to last item + projects1Loader = rows[7]; + expect(projects1Loader).toHaveAttribute('aria-posinset', '11'); + expect(projects1Loader).toHaveAttribute('aria-setsize', '11'); + projectsLoader1ParentStyles = projects1Loader.parentElement!.style; + // 15 items * 25px aka photos-1 -> 2 + projects + projects-0 -> 1 + 10 items in the folder + expect(projectsLoader1ParentStyles.top).toBe('375px'); + expect(projectsLoader1ParentStyles.height).toBe('30px'); + // Document loader is 2nd to last item + let documentLoader = rows[9]; + expect(documentLoader).toHaveAttribute('aria-posinset', '11'); + expect(documentLoader).toHaveAttribute('aria-setsize', '11'); + let documentLoader1ParentStyles = documentLoader.parentElement!.style; + // 36 items * 25px + 2 intermediate loaders * 30px + expect(documentLoader1ParentStyles.top).toBe('960px'); + expect(documentLoader1ParentStyles.height).toBe('30px'); + + sentinels = tree.queryAllByTestId('loadMoreSentinel'); + expect(sentinels).toHaveLength(4); + for (let sentinel of sentinels) { + expect(sentinel.closest('[inert]')).toBeTruthy; + } + }); + + it('should not reserve room for the loader if isLoading is false', async () => { + let tree = render( + + ); + + let treeTester = testUtilUser.createTester('Tree', {root: tree.getByRole('treegrid')}); + let rows = treeTester.rows; + expect(rows).toHaveLength(9); + let rootLoaderRow = rows[8]; + let rootLoaderParentStyles = rootLoaderRow.parentElement!.style; + // 38 items * 25px + 1 intermediate loaders * 30px + expect(rootLoaderParentStyles.top).toBe('980px'); + expect(rootLoaderParentStyles.height).toBe('30px'); + + // Sentinels that aren't in a loading state don't have a "row" rendered but still have a virtualizer node + let sentinels = tree.queryAllByTestId('loadMoreSentinel'); + expect(sentinels).toHaveLength(4); + let projectsLoader = sentinels[1].closest('[inert]')!; + let projectsLoaderParentStyles = projectsLoader.parentElement!.style; + // 23 items * 25px + expect(projectsLoaderParentStyles.top).toBe('575px'); + expect(projectsLoaderParentStyles.height).toBe('0px'); + + let projects1Loader = sentinels[0].closest('[inert]')!; + let projectsLoader1ParentStyles = projects1Loader.parentElement!.style; + // 15 items * 25px aka photos-1 -> 2 + projects + projects-0 -> 1 + 10 items in the folder + expect(projectsLoader1ParentStyles.top).toBe('375px'); + expect(projectsLoader1ParentStyles.height).toBe('0px'); + + let documentLoader = rows[7]; + let documentLoader1ParentStyles = documentLoader.parentElement!.style; + // 36 items * 25px + expect(documentLoader1ParentStyles.top).toBe('900px'); + expect(documentLoader1ParentStyles.height).toBe('30px'); + }); + }); + }); + describe('shouldSelectOnPressUp', () => { it('should select an item on pressing down when shouldSelectOnPressUp is not provided', async () => { let onSelectionChange = jest.fn(); let {getAllByRole} = render(); let items = getAllByRole('row'); - await user.pointer({target: items[0], keys: '[MouseLeft>]'}); + await user.pointer({target: items[0], keys: '[MouseLeft>]'}); expect(onSelectionChange).toBeCalledTimes(1); - + await user.pointer({target: items[0], keys: '[/MouseLeft]'}); expect(onSelectionChange).toBeCalledTimes(1); }); @@ -1217,9 +1614,9 @@ describe('Tree', () => { let {getAllByRole} = render(); let items = getAllByRole('row'); - await user.pointer({target: items[0], keys: '[MouseLeft>]'}); + await user.pointer({target: items[0], keys: '[MouseLeft>]'}); expect(onSelectionChange).toBeCalledTimes(1); - + await user.pointer({target: items[0], keys: '[/MouseLeft]'}); expect(onSelectionChange).toBeCalledTimes(1); }); @@ -1229,9 +1626,9 @@ describe('Tree', () => { let {getAllByRole} = render(); let items = getAllByRole('row'); - await user.pointer({target: items[0], keys: '[MouseLeft>]'}); + await user.pointer({target: items[0], keys: '[MouseLeft>]'}); expect(onSelectionChange).toBeCalledTimes(0); - + await user.pointer({target: items[0], keys: '[/MouseLeft]'}); expect(onSelectionChange).toBeCalledTimes(1); }); From 76bc6ddb2328701d8eed3ade09d16c6f310cc5c4 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 23 May 2025 15:15:05 -0700 Subject: [PATCH 4/7] fix story for correctness, should only need to provide a dependecy at the top most collection --- packages/react-aria-components/stories/Tree.stories.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 60a7dff601a..92cc0a92a70 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -820,13 +820,14 @@ function MultiLoaderTreeMockAsync(args) { aria-label="multi loader tree" className={styles.tree}> {/* TODO: wonder if there is something we can do to ensure that these depenedcies are provided, need to dig to make sure if there is an alternative */} + {/* NOTE: important to provide dependencies here, otherwise the nested level doesn't perform loading updates properly */} {(item: any) => { if (item.id === 'projects') { return ( - {/* NOTE: important to provide dependencies here, otherwise the nested level doesn't perform loading updates properly */} - + + {(item: any) => { return item.id !== 'projects-1' ? ( From 1b4b7b54dbe6c7ebf81f50035d088af189550afb Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 27 May 2025 15:27:04 -0700 Subject: [PATCH 5/7] restoring focus back to the tree if the user was focused on the loader --- .../@react-stately/tree/src/useTreeState.ts | 9 ++++++-- .../react-aria-components/test/Tree.test.tsx | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/@react-stately/tree/src/useTreeState.ts b/packages/@react-stately/tree/src/useTreeState.ts index c454a13a9fe..93f3f450be2 100644 --- a/packages/@react-stately/tree/src/useTreeState.ts +++ b/packages/@react-stately/tree/src/useTreeState.ts @@ -65,8 +65,13 @@ export function useTreeState(props: TreeProps): TreeState { - if (selectionState.focusedKey != null && !tree.getItem(selectionState.focusedKey)) { - selectionState.setFocusedKey(null); + if (selectionState.focusedKey != null) { + let focusedItem = tree.getItem(selectionState.focusedKey); + // TODO: do we want to have the same logic as useListState/useGridState where it tries to find the nearest row? + // We could possibly special case this loader case and have it try to find the item just before it/the parent + if (!focusedItem || focusedItem.type === 'loader' && !focusedItem.props.isLoading) { + selectionState.setFocusedKey(null); + } } // eslint-disable-next-line react-hooks/exhaustive-deps }, [tree, selectionState.focusedKey]); diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index 6710b0cb075..0748f57e787 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -1592,6 +1592,27 @@ describe('Tree', () => { expect(documentLoader1ParentStyles.top).toBe('900px'); expect(documentLoader1ParentStyles.height).toBe('30px'); }); + + it('should restore focus to the tree if the loader is keyboard focused when loading finishes', async () => { + let tree = render( + + ); + let treeTester = testUtilUser.createTester('Tree', {root: tree.getByRole('treegrid')}); + let rows = treeTester.rows; + expect(rows).toHaveLength(8); + let rootLoaderRow = rows[7]; + expect(rootLoaderRow).toHaveTextContent('Loading...'); + + await user.tab(); + await user.keyboard('{End}'); + expect(document.activeElement).toBe(rootLoaderRow); + + tree.rerender( + + ); + + expect(document.activeElement).toBe(treeTester.tree); + }); }); }); From 5dbf2048d0613f47da9d48cb1f28a27f8fb79925 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 2 Jun 2025 15:13:53 -0700 Subject: [PATCH 6/7] fixing estimated loader position if sections exist taken from https://github.com/adobe/react-spectrum/pull/8326 --- packages/@react-stately/layout/src/ListLayout.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/@react-stately/layout/src/ListLayout.ts b/packages/@react-stately/layout/src/ListLayout.ts index 1b2b52fa685..bdff9b8e9ca 100644 --- a/packages/@react-stately/layout/src/ListLayout.ts +++ b/packages/@react-stately/layout/src/ListLayout.ts @@ -262,7 +262,7 @@ export class ListLayout exte y = 0; } - for (let node of collection) { + for (let node of collectionNodes) { let rowHeight = (this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT) + this.gap; // Skip rows before the valid rectangle unless they are already cached. if (node.type === 'item' && y + rowHeight < this.requestedRect.y && !this.isValid(node, y)) { @@ -279,12 +279,13 @@ export class ListLayout exte } // Build each loader that exists in the collection that is outside the visible rect so that they are persisted - // at the proper estimated location - if (y > this.requestedRect.maxY) { + // at the proper estimated location. If the node.type is "section" then we don't do this shortcut since we have to + // build the sections to see how tall they are. + if ((node.type === 'item' || node.type === 'loader') && y > this.requestedRect.maxY) { let lastProcessedIndex = collectionNodes.indexOf(node); for (let loaderNode of loaderNodes) { let loaderNodeIndex = collectionNodes.indexOf(loaderNode); - // Subtract by an addition 1 since we've already added the current item's height to y + // Subtract by an additional 1 since we've already added the current item's height to y y += (loaderNodeIndex - lastProcessedIndex - 1) * rowHeight; let loader = this.buildChild(loaderNode, this.padding, y, null); nodes.push(loader); From ddffc91204999fd12b63adaac790b18904db028a Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 3 Jun 2025 17:38:20 -0700 Subject: [PATCH 7/7] skip test for now --- packages/react-aria-components/test/Tree.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index 0386d72233b..0309f912c07 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -1594,7 +1594,8 @@ describe('Tree', () => { expect(documentLoader1ParentStyles.height).toBe('30px'); }); - it('should restore focus to the tree if the loader is keyboard focused when loading finishes', async () => { + // TODO: bring this back when we enable keyboard focus on tree loaders again + it.skip('should restore focus to the tree if the loader is keyboard focused when loading finishes', async () => { let tree = render( );