-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Tree multiple level loading support and only count "items" for collection size #8349
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
Conversation
… the top most collection
Build successful! 🎉 |
Build successful! 🎉 |
also the aria row index and pos in set from empty/isLoading state wrapper since it might be confusing to hear that there are items when the table/tree is empty
Build successful! 🎉 |
Build successful! 🎉 |
looks like you have some conflicts, happy to help resolve |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
Build successful! 🎉 |
* add docs for gridlist * add async loading docs for GridList and Listbox adjacent components * add docs example for table * add prop tables * rename sentinel component and add prop tables * adding tree docs for loading spinners * fix docs lint * typo and left over stuff * export the loadMoreItem props from the monopackage * review comments * fix the tree example in docs * review comments * review comments * forgot to remove a delay
Build successful! 🎉 |
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.
couple small comments, looks like you have some merge conflicts too
let sentinelRef = useRef(null); | ||
let memoedLoadMoreProps = useMemo(() => ({ | ||
onLoadMore, | ||
// TODO: this collection will update anytime a row is expanded/collapsed becaused the flattenedRows will change. |
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.
Still a TODO before merge?
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.
I'll remove the todo here but keep the comment in case it turns out to be an issue down the line
sentinelRef, | ||
scrollOffset | ||
}), [onLoadMore, scrollOffset, state?.collection]); | ||
UNSTABLE_useLoadMoreSentinel(memoedLoadMoreProps, sentinelRef); |
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.
are we removing UNSTABLE for all of them?
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.
Was debating whether or not to do that for this util, but I think I'll go ahead and remove it since it is just a util with a pretty specific purpose (not sure if people will use it tbh)
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:UNSTABLE_GridListLoadingSentinel-UNSTABLE_GridListLoadingSentinel <T extends {}> {
- children?: ReactNode
- className?: string
- isLoading?: boolean
- onLoadMore?: () => any
- scrollOffset?: number = 1
- style?: CSSProperties
-} /react-aria-components:UNSTABLE_ListBoxLoadingSentinel-UNSTABLE_ListBoxLoadingSentinel <T extends {}> {
- children?: ReactNode
- className?: string
- isLoading?: boolean
- onLoadMore?: () => any
- scrollOffset?: number = 1
- style?: CSSProperties
-} /react-aria-components:UNSTABLE_TableLoadingSentinel-UNSTABLE_TableLoadingSentinel <T extends {}> {
- children?: ReactNode
- className?: string
- isLoading?: boolean
- onLoadMore?: () => any
- scrollOffset?: number = 1
- style?: CSSProperties
-} /react-aria-components:UNSTABLE_TreeLoadingIndicator-UNSTABLE_TreeLoadingIndicator <T extends {}> {
- children?: ChildrenOrFunction<UNSTABLE_TreeLoadingIndicatorRenderProps>
- className?: ClassNameOrFunction<UNSTABLE_TreeLoadingIndicatorRenderProps>
- style?: StyleOrFunction<UNSTABLE_TreeLoadingIndicatorRenderProps>
-} /react-aria-components:GridListLoadMoreItem+GridListLoadMoreItem <T extends {}> {
+ children?: ReactNode
+ className?: string
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: CSSProperties
+} /react-aria-components:ListBoxLoadMoreItem+ListBoxLoadMoreItem <T extends {}> {
+ children?: ReactNode
+ className?: string
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: CSSProperties
+} /react-aria-components:TableLoadMoreItem+TableLoadMoreItem <T extends {}> {
+ children?: ReactNode
+ className?: string
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: CSSProperties
+} /react-aria-components:TreeLoadMoreItem+TreeLoadMoreItem <T extends {}> {
+ children?: ReactNode | ((TreeLoadMoreItemRenderProps & {
+ defaultChildren: ReactNode | undefined
+})) => ReactNode
+ className?: ClassNameOrFunction<TreeLoadMoreItemRenderProps>
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: StyleOrFunction<TreeLoadMoreItemRenderProps>
+} /react-aria-components:GridListLoadMoreItemProps+GridListLoadMoreItemProps {
+ children?: ReactNode
+ className?: string
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: CSSProperties
+} /react-aria-components:ListBoxLoadMoreItemProps+ListBoxLoadMoreItemProps {
+ children?: ReactNode
+ className?: string
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: CSSProperties
+} /react-aria-components:TableLoadMoreItemProps+TableLoadMoreItemProps {
+ children?: ReactNode
+ className?: string
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: CSSProperties
+} /react-aria-components:TreeLoadMoreItemProps+TreeLoadMoreItemProps {
+ children?: ReactNode | ((TreeLoadMoreItemRenderProps & {
+ defaultChildren: ReactNode | undefined
+})) => ReactNode
+ className?: ClassNameOrFunction<TreeLoadMoreItemRenderProps>
+ isLoading?: boolean
+ onLoadMore?: () => any
+ scrollOffset?: number = 1
+ style?: StyleOrFunction<TreeLoadMoreItemRenderProps>
+} /react-aria-components:TreeLoadMoreItemRenderProps+TreeLoadMoreItemRenderProps {
+ level: number
+} @react-aria/utils/@react-aria/utils:UNSTABLE_useLoadMoreSentinel-UNSTABLE_useLoadMoreSentinel {
- props: LoadMoreSentinelProps
- ref: RefObject<HTMLElement | null>
- returnVal: undefined
-} /@react-aria/utils:useLoadMoreSentinel+useLoadMoreSentinel {
+ props: LoadMoreSentinelProps
+ ref: RefObject<HTMLElement | null>
+ returnVal: undefined
+} |
Closes RSP Component Milestones (view)
✅ Pull Request Checklist:
📝 Test Instructions:
Test Tree multiloading stories in RAC storybook. Also test all collections that support loading indicators (Table, Listbox, GridList, Tree, ComboBox, Picker, etc) and make sure that the virtualized items have the proper aria-rowcount/posinset/etc. Loaders shouldn't be counted as items/rows. Sanity check that other collections work fine virtualized/non-virtualized and that empty states/initial load states still render as expected
🧢 Your Project:
RSP