From ceb827ec33e8acb126defe4fe36d85adedd0b4f7 Mon Sep 17 00:00:00 2001 From: Radoslaw Szwajkowski Date: Sat, 14 Sep 2024 16:39:14 +0200 Subject: [PATCH] :bug: 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: https://github.com/konveyor/tackle2-ui/pull/2049 Signed-off-by: Radoslaw Szwajkowski Co-authored-by: Scott Dickerson --- .../InfiniteScroller/InfiniteScroller.tsx | 41 +++++++++---------- .../InfiniteScroller/useVisibilityTracker.tsx | 11 +++-- .../task-manager/TaskManagerDrawer.tsx | 7 +++- 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/client/src/app/components/InfiniteScroller/InfiniteScroller.tsx b/client/src/app/components/InfiniteScroller/InfiniteScroller.tsx index 9e4ddfbc6..eb990f39a 100644 --- a/client/src/app/components/InfiniteScroller/InfiniteScroller.tsx +++ b/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,6 +12,7 @@ export interface InfiniteScrollerProps { hasMore: boolean; // number of items currently displayed/known itemCount: number; + pageSize: number; } export const InfiniteScroller = ({ @@ -19,40 +20,38 @@ export const InfiniteScroller = ({ 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 (
{children} {hasMore && ( -
+
{t("message.loadingTripleDot")}
)} diff --git a/client/src/app/components/InfiniteScroller/useVisibilityTracker.tsx b/client/src/app/components/InfiniteScroller/useVisibilityTracker.tsx index c6f8135b2..c30b42d7d 100644 --- a/client/src/app/components/InfiniteScroller/useVisibilityTracker.tsx +++ b/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(null); - const [visible, setVisible] = useState(false); + const [visible, setVisible] = useState(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[]) => diff --git a/client/src/app/components/task-manager/TaskManagerDrawer.tsx b/client/src/app/components/task-manager/TaskManagerDrawer.tsx index 4a1faf69f..10824bc16 100644 --- a/client/src/app/components/task-manager/TaskManagerDrawer.tsx +++ b/client/src/app/components/task-manager/TaskManagerDrawer.tsx @@ -66,7 +66,8 @@ interface TaskManagerDrawerProps { export const TaskManagerDrawer: React.FC = forwardRef( (_props, ref) => { const { isExpanded, setIsExpanded, queuedCount } = useTaskManagerContext(); - const { tasks, hasNextPage, fetchNextPage } = useTaskManagerData(); + const { tasks, hasNextPage, fetchNextPage, pageSize } = + useTaskManagerData(); const [expandedItems, setExpandedItems] = useState([]); const [taskWithExpandedActions, setTaskWithExpandedAction] = useState< @@ -106,6 +107,7 @@ export const TaskManagerDrawer: React.FC = forwardRef( fetchMore={fetchNextPage} hasMore={hasNextPage} itemCount={tasks?.length ?? 0} + pageSize={pageSize} > {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, }; };