Skip to content
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

🐛 Fix fetching in InfiniteScroller #2085

Merged
merged 3 commits into from
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 20 additions & 21 deletions client/src/app/components/InfiniteScroller/InfiniteScroller.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { ReactNode, useEffect, useRef } from "react";
import React, { ReactNode, useEffect, useState } from "react";
import { useTranslation } from "react-i18next";
import { useVisibilityTracker } from "./useVisibilityTracker";
import "./InfiniteScroller.css";
Expand All @@ -12,47 +12,46 @@ export interface InfiniteScrollerProps {
hasMore: boolean;
// number of items currently displayed/known
itemCount: number;
pageSize: number;
}

export const InfiniteScroller = ({
children,
fetchMore,
hasMore,
itemCount,
pageSize,
}: InfiniteScrollerProps) => {
const { t } = useTranslation();
// Track how many items were known at time of triggering the fetch.
// This allows to detect edge case when second(or more) fetchMore() is triggered before
// IntersectionObserver is able to detect out-of-view event.
// Initializing with zero ensures that the effect will be triggered immediately
// (parent is expected to display empty state until some items are available).
const itemCountRef = useRef(0);
const [readyForFetch, setReadyForFetch] = useState(false);
const { visible: isSentinelVisible, nodeRef: sentinelRef } =
useVisibilityTracker({
enable: hasMore,
});
useEffect(() => {
// enable or clear the flag depending on the sentinel visibility
setReadyForFetch(!!isSentinelVisible);
}, [isSentinelVisible]);

useEffect(
() => {
if (
isSentinelVisible &&
itemCountRef.current !== itemCount &&
fetchMore() // fetch may be blocked if background refresh is in progress (or other manual fetch)
) {
// fetchMore call was triggered (it may fail but will be subject to React Query retry policy)
itemCountRef.current = itemCount;
}
},
useEffect(() => {
if (readyForFetch) {
// clear the flag if fetch request is accepted
setReadyForFetch(!fetchMore());
}
// reference to fetchMore() changes based on query state and ensures that the effect is triggered in the right moment
// i.e. after fetch triggered by the previous fetchMore() call finished
[isSentinelVisible, fetchMore, itemCount]
);
}, [fetchMore, readyForFetch]);

return (
<div>
{children}
{hasMore && (
<div ref={sentinelRef} className="infinite-scroll-sentinel">
<div
ref={sentinelRef}
// re-create the node with every page change to force new Intersection observer
key={Math.ceil(itemCount / pageSize)}
className="infinite-scroll-sentinel"
>
{t("message.loadingTripleDot")}
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useEffect, useRef, useState, useCallback } from "react";

export function useVisibilityTracker({ enable }: { enable: boolean }) {
const nodeRef = useRef<HTMLDivElement>(null);
const [visible, setVisible] = useState(false);
const [visible, setVisible] = useState<boolean | undefined>(false);
const node = nodeRef.current;

// state is set from IntersectionObserver callbacks which may not align with React lifecycle
Expand All @@ -22,14 +22,19 @@ export function useVisibilityTracker({ enable }: { enable: boolean }) {
}, []);

useEffect(() => {
if (enable && !node) {
// use falsy value different than initial value - state change will trigger render()
// otherwise we need to wait for the next render() to read node ref
setVisibleSafe(undefined);
return undefined;
}

if (!enable || !node) {
return undefined;
}

// Observer with default options - the whole view port used.
// Note that if root element is used then it needs to be the ancestor of the target.
// In case of infinite scroller the target is always within the (scrollable!)parent
// even if the node is technically hidden from the user.
// https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API#root
const observer = new IntersectionObserver(
(entries: IntersectionObserverEntry[]) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ interface TaskManagerDrawerProps {
export const TaskManagerDrawer: React.FC<TaskManagerDrawerProps> = forwardRef(
(_props, ref) => {
const { isExpanded, setIsExpanded, queuedCount } = useTaskManagerContext();
const { tasks, hasNextPage, fetchNextPage } = useTaskManagerData();
const { tasks, hasNextPage, fetchNextPage, pageSize } =
useTaskManagerData();

const [expandedItems, setExpandedItems] = useState<number[]>([]);
const [taskWithExpandedActions, setTaskWithExpandedAction] = useState<
Expand Down Expand Up @@ -106,6 +107,7 @@ export const TaskManagerDrawer: React.FC<TaskManagerDrawerProps> = forwardRef(
fetchMore={fetchNextPage}
hasMore={hasNextPage}
itemCount={tasks?.length ?? 0}
pageSize={pageSize}
>
<NotificationDrawerList>
{tasks.map((task) => (
Expand Down Expand Up @@ -282,6 +284,7 @@ const useTaskManagerData = () => {
[data]
);

// note that the callback will change when query fetching state changes
const fetchMore = useCallback(() => {
// forced fetch is not allowed when background fetch or other forced fetch is in progress
if (!isFetching && !isFetchingNextPage) {
Expand All @@ -297,6 +300,6 @@ const useTaskManagerData = () => {
isFetching,
hasNextPage,
fetchNextPage: fetchMore,
isReadyToFetch: !isFetching && !isFetchingNextPage,
pageSize: PAGE_SIZE,
};
};
Loading