-
Notifications
You must be signed in to change notification settings - Fork 1
Add infinite scroll #30
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
base: main
Are you sure you want to change the base?
Conversation
src/hooks/useInfiniteScroll.ts
Outdated
| */ | ||
| const useInfiniteScroll = (callback: () => void) => { | ||
| useEffect(() => { | ||
| const target = document.getElementById('skeleton'); |
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.
Could you refactor this to take in a ref instead, and make useInfiniteScroll generic to any scrollable container that can pass a ref into it?
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 did it that way initially, but I changed it because it made it much more complex to send the reference from the layout to the page, and because the scrollable area is always the same I left it that way.
But if you prefer, I can do it that way.
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.
The other thing I can do is to make the results table scrollable (but 100% height), so it is visually the same but I can control the scroll in the scope of the page
src/hooks/useInfiniteScroll.ts
Outdated
| const scrollHeight = target.scrollHeight; | ||
| const clientHeight = target.clientHeight; | ||
|
|
||
| if (scrollHeight - scrollTop <= clientHeight + DEFAULT_THRESHOLD) { |
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.
if it's called DEFAULT_THRESHOLD, then please also expose an optional threshold?: number arg that callers can override
This PR introduces a useInfiniteScroll hook to trigger data loading when the user scrolls near the bottom of the page
related issue: #18