🐛 Fix fetching in InfiniteScroller (#2085)

Fixes:
1. initial delay in first fetch - root cause was delayed initialization
of sentinel reference

2. extra page fetch request - algorithm based on items count was not
sufficient

Reference-Url:  #2049

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
Co-authored-by: Scott Dickerson <[email protected]>
Signed-off-by: DvoraShechter1 <[email protected]>

client/src/app/components/InfiniteScroller/InfiniteScroller.tsx

@@ -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";
@@ -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>
       )}

client/src/app/components/InfiniteScroller/useVisibilityTracker.tsx

@@ -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
@@ -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[]) =>

client/src/app/components/task-manager/TaskManagerDrawer.tsx

@@ -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<
@@ -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) => (
@@ -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) {
@@ -297,6 +300,6 @@ const useTaskManagerData = () => {
     isFetching,
     hasNextPage,
     fetchNextPage: fetchMore,
-    isReadyToFetch: !isFetching && !isFetchingNextPage,
+    pageSize: PAGE_SIZE,
   };
 };