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

Resolve infinite scroll issue in Resource page #9267

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open
Changes from 1 commit
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
Prev Previous commit
Next Next commit
useInfiniteQuery hook implemented
NikhilA8606 committed Dec 11, 2024
commit d3b81025c9f01259fab545e1e1903092ebff8f08
61 changes: 61 additions & 0 deletions src/Utils/request/useInfiniteQuery.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { useCallback, useState } from "react";

import { RESULTS_PER_PAGE_LIMIT } from "@/common/constants";

import { PaginatedResponse, QueryRoute } from "@/Utils/request/types";
import useQuery, { QueryOptions } from "@/Utils/request/useQuery";

export interface InfiniteQueryOptions<TItem>
extends QueryOptions<PaginatedResponse<TItem>> {
deduplicateBy: (item: TItem) => string | number;
}

export function useInfiniteQuery<TItem>(
route: QueryRoute<PaginatedResponse<TItem>>,
options?: InfiniteQueryOptions<TItem>,
) {
const [items, setItems] = useState<TItem[]>([]);
const [totalCount, setTotalCount] = useState<number>();
const [offset, setOffset] = useState(0);

const { refetch, loading, ...queryResponse } = useQuery(route, {
...options,
query: {
...(options?.query ?? {}),
offset,
},
onResponse: ({ data }) => {
Comment on lines +21 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential Overwrite of onResponse and query Options

By spreading ...options and then overriding query and onResponse, any query or onResponse properties provided in the options object will be overwritten. This could lead to unintended behavior if the caller supplies their own onResponse or query parameters.

Consider merging the query parameters and chaining the onResponse callbacks to preserve the functionality provided by the caller:

const { refetch, loading, ...queryResponse } = useQuery(route, {
  ...options,
  query: {
    ...(options?.query ?? {}),
    offset,
  },
- onResponse: ({ data }) => {
+ onResponse: (response) => {
+   options?.onResponse?.(response);
    if (!response.data) return;
-   if (!data) return;
-   const allItems = items.concat(data.results);
+   const allItems = items.concat(response.data.results);

    const deduplicatedItems = options?.deduplicateBy
      ? Array.from(
          allItems
            .reduce((map, item) => {
              const key = options.deduplicateBy(item);
              return map.set(key, item);
            }, new Map<string | number, TItem>())
            .values(),
        )
      : allItems;

    setItems(deduplicatedItems);
    setTotalCount(response.data.count);
  },
});

This ensures that any onResponse logic provided in options is still executed.

Committable suggestion skipped: line range outside the PR's diff.

if (!data) return;
const allItems = items.concat(data.results);

const deduplicatedItems = options?.deduplicateBy
? Array.from(
allItems
.reduce((map, item) => {
const key = options.deduplicateBy(item);
return map.set(key, item);
}, new Map<string | number, TItem>())
.values(),
)
: allItems;

setItems(deduplicatedItems);
setTotalCount(data.count);
},
});

const fetchNextPage = useCallback(() => {
if (loading) return;
setOffset((prevOffset) => prevOffset + RESULTS_PER_PAGE_LIMIT);
}, [loading]);
NikhilA8606 marked this conversation as resolved.
Show resolved Hide resolved

return {
items,
loading,
fetchNextPage,
refetch,
totalCount,
hasMore: items.length < (totalCount ?? 0),
NikhilA8606 marked this conversation as resolved.
Show resolved Hide resolved
...queryResponse,
};
}
89 changes: 16 additions & 73 deletions src/components/Kanban/Board.tsx
Original file line number Diff line number Diff line change
@@ -4,15 +4,14 @@ import {
Droppable,
OnDragEndResponder,
} from "@hello-pangea/dnd";
import { ReactNode, RefObject, useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { ReactNode, RefObject, useEffect, useRef } from "react";

import CareIcon from "@/CAREUI/icons/CareIcon";

import useDebounce from "@/hooks/useDebounce";

import request from "@/Utils/request/request";
import { QueryRoute } from "@/Utils/request/types";
import { PaginatedResponse, QueryRoute } from "@/Utils/request/types";
import { useInfiniteQuery } from "@/Utils/request/useInfiniteQuery";
import { QueryOptions } from "@/Utils/request/useQuery";

export interface KanbanBoardProps<T extends { id: string }> {
@@ -25,7 +24,7 @@ export interface KanbanBoardProps<T extends { id: string }> {
id: string,
...args: unknown[]
) => {
route: QueryRoute<unknown>;
route: QueryRoute<PaginatedResponse<T>>;
options?: QueryOptions<unknown>;
};
}[];
@@ -83,58 +82,21 @@ function KanbanSection<T extends { id: string }>(
},
) {
const { section } = props;
const [offset, setOffset] = useState(0);
const [pages, setPages] = useState<T[][]>([]);
const [fetchingNextPage, setFetchingNextPage] = useState(false);
const [hasMore, setHasMore] = useState(true);
const [totalCount, setTotalCount] = useState<number>();

const options = section.fetchOptions(section.id);
const sectionRef = useRef<HTMLDivElement>(null);
const defaultLimit = 14;
const { t } = useTranslation();

const fetchNextPage = async (refresh: boolean = false) => {
if (!refresh && (fetchingNextPage || !hasMore)) return;
if (refresh) {
setPages([]);
setOffset(0);
}
const offsetToUse = refresh ? 0 : offset;
setFetchingNextPage(true);
const res = await request(options.route, {
...options.options,
query: {
...options.options?.query,
offset: offsetToUse,
limit: defaultLimit,
},
});
if (res.error) return;
const newPages = refresh ? [] : [...pages];
const page = Math.floor(offsetToUse / defaultLimit);
newPages[page] = (res.data as any).results;
setPages(newPages);
setHasMore(!!(res.data as any)?.next);
setTotalCount((res.data as any)?.count);
setOffset(offsetToUse + defaultLimit);
setFetchingNextPage(false);
};

const items = pages.flat();
const { items, loading, fetchNextPage, hasMore } = useInfiniteQuery<T>(
section.fetchOptions(section.id).route,
{
...section.fetchOptions(section.id).options,
deduplicateBy: (item) => item.id,
},
);

const debouncedScroll = useDebounce(() => {
if (
!sectionRef.current ||
!props.boardRef.current ||
fetchingNextPage ||
!hasMore
)
return;
if (!props.boardRef.current || loading || !hasMore) return;

const scrollTop = props.boardRef.current.scrollTop;
const visibleHeight = props.boardRef.current.offsetHeight;
const sectionHeight = sectionRef.current.offsetHeight;
const sectionHeight = props.boardRef.current.scrollHeight;

if (scrollTop + visibleHeight >= sectionHeight - 100) {
fetchNextPage();
@@ -145,11 +107,7 @@ function KanbanSection<T extends { id: string }>(
props.boardRef.current?.addEventListener("scroll", debouncedScroll);
return () =>
props.boardRef.current?.removeEventListener("scroll", debouncedScroll);
}, [fetchingNextPage, hasMore, props.boardRef, debouncedScroll]);

useEffect(() => {
fetchNextPage(true);
}, [props.section]);
}, [loading, hasMore, debouncedScroll, props.boardRef]); // Add props.boardRef here

return (
<Droppable droppableId={section.id}>
@@ -158,22 +116,7 @@ function KanbanSection<T extends { id: string }>(
ref={provided.innerRef}
className="relative mr-2 w-[300px] shrink-0 rounded-xl bg-secondary-200"
>
<div className="sticky top-0 rounded-xl bg-secondary-200 pt-2">
<div className="mx-2 flex items-center justify-between rounded-lg border border-secondary-300 bg-white p-4">
<div>{section.title}</div>
<div>
<span className="ml-2 rounded-lg bg-secondary-300 px-2">
{typeof totalCount === "undefined" ? "..." : totalCount}
</span>
</div>
</div>
</div>
<div ref={sectionRef}>
{!fetchingNextPage && totalCount === 0 && (
<div className="flex items-center justify-center py-10 text-secondary-500">
{t("no_results_found")}
</div>
)}
<div ref={props.boardRef}>
{items.map((item, i) => (
<Draggable draggableId={item.id} key={i} index={i}>
{(provided) => (
@@ -188,7 +131,7 @@ function KanbanSection<T extends { id: string }>(
)}
</Draggable>
))}
{fetchingNextPage && (
{loading && (
<div className="mt-2 h-[300px] w-[284px] animate-pulse rounded-lg bg-secondary-300" />
)}
</div>