-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Async loading support for S2 ComboBox/Picker #7938
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c4ca76d
8ee20f9
c2a1cf9
765b757
7708b81
2da7d04
0bb9f21
0049153
76e1a05
68033b2
54fcbaa
4d3bb6c
f6ba68a
3fe09f6
0b06824
da166f2
0ef52ef
51580d7
76dc08a
27260f4
41a28cb
4fb8aeb
f8b1745
b272469
7a2877c
4c5bc69
5c423d4
fdf423f
711dff6
286d8b8
0938fb9
1eeb798
5fc224f
18c65b2
85b7edb
7f57506
d493fc4
a69a794
4523888
3052001
6a98ac5
6fb2c01
97b21a2
e1bd8f3
d4bec98
7f062b8
561d914
dc6e80a
0c3a3b4
2085f8c
a9b7ea6
f60cbe3
0fa17cc
95beec7
fc193c5
cbd91b5
4a62993
f664da0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/* | ||
* Copyright 2024 Adobe. All rights reserved. | ||
* This file is licensed to you under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. You may obtain a copy | ||
* of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software distributed under | ||
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS | ||
* OF ANY KIND, either express or implied. See the License for the specific language | ||
* governing permissions and limitations under the License. | ||
*/ | ||
|
||
import type {AsyncLoadable} from '@react-types/shared'; | ||
import {getScrollParent} from './getScrollParent'; | ||
import {RefObject, useRef} from 'react'; | ||
import {useEffectEvent} from './useEffectEvent'; | ||
import {useLayoutEffect} from './useLayoutEffect'; | ||
|
||
export interface LoadMoreSentinelProps extends AsyncLoadable { | ||
/** | ||
* The amount of offset from the bottom of your scrollable region that should trigger load more. | ||
* Uses a percentage value relative to the scroll body's client height. Load more is then triggered | ||
* when your current scroll position's distance from the bottom of the currently loaded list of items is less than | ||
* or equal to the provided value. (e.g. 1 = 100% of the scroll region's height). | ||
* @default 1 | ||
*/ | ||
scrollOffset?: number | ||
// TODO: Maybe include a scrollRef option so the user can provide the scrollParent to compare against instead of having us look it up | ||
} | ||
|
||
export function UNSTABLE_useLoadMoreSentinel(props: LoadMoreSentinelProps, ref: RefObject<HTMLElement | null>): void { | ||
let {isLoading, onLoadMore, scrollOffset = 1} = props; | ||
|
||
let sentinelObserver = useRef<IntersectionObserver>(null); | ||
|
||
let triggerLoadMore = useEffectEvent((entries: IntersectionObserverEntry[]) => { | ||
// Use "isIntersecting" over an equality check of 0 since it seems like there is cases where | ||
// a intersection ratio of 0 can be reported when isIntersecting is actually true | ||
for (let entry of entries) { | ||
if (entry.isIntersecting && !isLoading && onLoadMore) { | ||
onLoadMore(); | ||
} | ||
} | ||
}); | ||
|
||
useLayoutEffect(() => { | ||
if (ref.current) { | ||
// TODO: problem with S2's Table loading spinner. Now that we use the isLoading provided to the sentinel in the layout to adjust the height of the loader, | ||
// we are getting space reserved for the loadMore spinner when doing initial loading and rendering empty state at the same time. We can somewhat fix this by providing isLoading={loadingState === 'loadingMore'} | ||
// which will mean the layout won't reserve space for the loader for initial loads, but that breaks the load more behavior (specifically, auto load more to fill scrollOffset. Scroll to load more seems broken to after initial load). | ||
// We need to tear down and set up a new IntersectionObserver to force a check if the sentinel is "in view", see https://codesandbox.io/p/sandbox/magical-swanson-dhgp89?file=%2Fsrc%2FApp.js%3A21%2C21 | ||
// I've actually fixed this via a ListLayout change (TableLayout extends this) where I check "collection?.size === 0 || (collection.size === 1 && collection.getItem(collection.getFirstKey()!)!.type === 'loader')" | ||
// as well as isLoading, but it feels pretty opinionated/implementation specifc | ||
Comment on lines
+48
to
+53
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. A bit unfortunate that fixing this involved layout level changes. Also a bit unfortunate that we can't just create single instance of a IntersectionObserver upfront, but we need to do this setup/teardown every time isLoading changes so that we can properly trigger the loadMore if the sentinel remains within the rootMargin after the items are loaded. |
||
sentinelObserver.current = new IntersectionObserver(triggerLoadMore, {root: getScrollParent(ref?.current) as HTMLElement, rootMargin: `0px ${100 * scrollOffset}% ${100 * scrollOffset}% ${100 * scrollOffset}%`}); | ||
sentinelObserver.current.observe(ref.current); | ||
} | ||
|
||
return () => { | ||
if (sentinelObserver.current) { | ||
sentinelObserver.current.disconnect(); | ||
} | ||
}; | ||
}, [isLoading, triggerLoadMore, ref, scrollOffset]); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
"actionbar.selectedAll": "تم تحديد الكل", | ||
"breadcrumbs.more": "المزيد من العناصر", | ||
"button.pending": "قيد الانتظار", | ||
"combobox.noResults": "لا توجد نتائج", | ||
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. From S1 |
||
"contextualhelp.help": "مساعدة", | ||
"contextualhelp.info": "معلومات", | ||
"dialog.alert": "تنبيه", | ||
|
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.
leaning towards not adding it for now, the hook is unstable as of now so we could always add it later. Open to opinions if anyone feels strongly about the approach