-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Tree multiple level loading support #8299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
LFDanLu
wants to merge
7
commits into
main
Choose a base branch
from
multi_loader_support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
390b6b0
Update tree and listlayout to handle multi loaders
LFDanLu 50c6f14
adapting other stories to new loader api and adding useAsync example …
LFDanLu f99e4bd
add tests
LFDanLu 76bc6dd
fix story for correctness, should only need to provide a dependecy at…
LFDanLu 7123f7e
Merge branch 'main' of github.com:adobe/react-spectrum into multi_loa…
LFDanLu 1b4b7b5
restoring focus back to the tree if the user was focused on the loader
LFDanLu 5dbf204
fixing estimated loader position if sections exist
LFDanLu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
||
|
@@ -699,18 +699,39 @@ export const TreeItem = /*#__PURE__*/ createBranchComponent('item', <T extends o | |
); | ||
}); | ||
|
||
export interface UNSTABLE_TreeLoadingIndicatorRenderProps extends Pick<TreeItemRenderProps, 'isFocused' | 'isFocusVisible'> { | ||
export interface UNSTABLE_TreeLoadingSentinelRenderProps extends Pick<TreeItemRenderProps, 'isFocused' | 'isFocusVisible'> { | ||
/** | ||
* What level the tree item has within the tree. | ||
* @selector [data-level] | ||
*/ | ||
level: number | ||
} | ||
|
||
export interface TreeLoaderProps extends RenderProps<UNSTABLE_TreeLoadingIndicatorRenderProps>, StyleRenderProps<UNSTABLE_TreeLoadingIndicatorRenderProps> {} | ||
export interface TreeLoadingSentinelProps extends Omit<LoadMoreSentinelProps, 'collection'>, RenderProps<UNSTABLE_TreeLoadingSentinelRenderProps> { | ||
/** | ||
* The load more spinner to render when loading additional items. | ||
*/ | ||
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<T extends object>(props: TreeLoaderProps, ref: ForwardedRef<HTMLDivElement>, item: Node<T>) { | ||
export const UNSTABLE_TreeLoadingSentinel = createLeafComponent('loader', function TreeLoadingSentinel<T extends object>(props: TreeLoadingSentinelProps, ref: ForwardedRef<HTMLDivElement>, item: Node<T>) { | ||
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, | ||
Comment on lines
+725
to
+729
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. think this should be ok, but open to discuss |
||
sentinelRef, | ||
scrollOffset | ||
}), [onLoadMore, scrollOffset, state?.collection]); | ||
UNSTABLE_useLoadMoreSentinel(memoedLoadMoreProps, sentinelRef); | ||
|
||
ref = useObjectRef<HTMLDivElement>(ref); | ||
let {rowProps, gridCellProps, ...states} = useTreeItem({node: item}, state, ref); | ||
let level = rowProps['aria-level'] || 1; | ||
|
@@ -726,7 +747,7 @@ export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', funct | |
let {isFocusVisible, focusProps} = useFocusRing(); | ||
|
||
let renderProps = useRenderProps({ | ||
...props, | ||
...otherProps, | ||
id: undefined, | ||
children: item.rendered, | ||
defaultClassName: 'react-aria-TreeLoader', | ||
|
@@ -739,19 +760,26 @@ export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', funct | |
|
||
return ( | ||
<> | ||
<div | ||
ref={ref} | ||
{...mergeProps(filterDOMProps(props as any), ariaProps, focusProps)} | ||
{...renderProps} | ||
data-key={rowProps['data-key']} | ||
data-collection={rowProps['data-collection']} | ||
data-focused={states.isFocused || undefined} | ||
data-focus-visible={isFocusVisible || undefined} | ||
data-level={level}> | ||
<div {...gridCellProps}> | ||
{renderProps.children} | ||
</div> | ||
{/* 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 */} | ||
<div style={{position: 'relative', width: 0, height: 0}} inert={inertValue(true)} > | ||
<div data-testid="loadMoreSentinel" ref={sentinelRef} style={{position: 'absolute', height: 1, width: 1}} /> | ||
</div> | ||
{isLoading && renderProps.children && ( | ||
<div | ||
ref={ref} | ||
{...mergeProps(filterDOMProps(props as any), ariaProps, focusProps)} | ||
{...renderProps} | ||
data-key={rowProps['data-key']} | ||
data-collection={rowProps['data-collection']} | ||
data-focused={states.isFocused || undefined} | ||
data-focus-visible={isFocusVisible || undefined} | ||
data-level={level}> | ||
<div {...gridCellProps}> | ||
{renderProps.children} | ||
</div> | ||
</div> | ||
)} | ||
</> | ||
); | ||
}); | ||
|
@@ -804,9 +832,10 @@ function flattenTree<T>(collection: TreeCollection<T>, opts: TreeGridCollectionO | |
keyMap.set(node.key, node as CollectionNode<T>); | ||
} | ||
|
||
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<T>); | ||
|
@@ -844,7 +873,7 @@ function TreeDropIndicatorWrapper(props: DropIndicatorProps, ref: ForwardedRef<H | |
let level = dropState && props.target.type === 'item' ? (dropState.collection.getItem(props.target.key)?.level || 0) + 1 : 1; | ||
|
||
return ( | ||
<TreeDropIndicatorForwardRef | ||
<TreeDropIndicatorForwardRef | ||
{...props} | ||
dropIndicatorProps={dropIndicatorProps} | ||
isDropTarget={isDropTarget} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I've preserved the previous behavior of just focusing the tree if the loader disappears, but ideally we'd do something like useListState and useGridState where it looks for the closest focusable row as a substitute. Will handle that update upon refactoring some of that logic as noted in https://github.com/adobe/react-spectrum/pull/8326/files#r2116825740