-
Notifications
You must be signed in to change notification settings - Fork 532
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
Added debouncing for APIs #9801
Changes from 14 commits
ce241c3
80e48d6
3f5fcd3
4cc9892
9a2ac6f
2323a97
6f9e820
97d3e71
50fa549
dd876e0
7934651
64ecadf
0ff0e86
26a4850
9615a9b
1afa00d
a22eaa7
4bf3f64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { useQuery } from "@tanstack/react-query"; | ||
import { Link } from "raviger"; | ||
import { useEffect } from "react"; | ||
|
||
import CareIcon from "@/CAREUI/icons/CareIcon"; | ||
|
||
|
@@ -9,6 +10,7 @@ import { Input } from "@/components/ui/input"; | |
|
||
import { Avatar } from "@/components/Common/Avatar"; | ||
|
||
import useDebouncedState from "@/hooks/useDebouncedState"; | ||
import useFilters from "@/hooks/useFilters"; | ||
|
||
import routes from "@/Utils/request/api"; | ||
|
@@ -30,15 +32,21 @@ export default function OrganizationFacilities({ | |
const { qParams, Pagination, advancedFilter, resultsPerPage, updateQuery } = | ||
useFilters({ limit: 14, cacheBlacklist: ["facility"] }); | ||
|
||
const [debouncedParams, setDebouncedParams] = useDebouncedState(qParams, 500); | ||
|
||
useEffect(() => { | ||
setDebouncedParams(qParams); | ||
}, [qParams]); | ||
|
||
const { data: facilities, isLoading } = useQuery({ | ||
queryKey: ["organizationFacilities", id, qParams], | ||
queryKey: ["organizationFacilities", id, debouncedParams], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Update query parameters to match the debouncing split. If implementing the suggested split of debounced parameters, update the query parameters accordingly. queryKey: ["organizationFacilities", id, debouncedParams],
queryFn: query(routes.facility.list, {
queryParams: {
- page: debouncedParams.page,
+ page: qParams.page,
limit: resultsPerPage,
- offset: (debouncedParams.page - 1) * resultsPerPage,
+ offset: (qParams.page - 1) * resultsPerPage,
organization: id,
- name: debouncedParams.name,
+ name: debouncedSearch || undefined,
...advancedFilter.filter,
},
}), Also applies to: 45-49 |
||
queryFn: query(routes.facility.list, { | ||
queryParams: { | ||
page: qParams.page, | ||
page: debouncedParams.page, | ||
limit: resultsPerPage, | ||
offset: (qParams.page - 1) * resultsPerPage, | ||
offset: (debouncedParams.page - 1) * resultsPerPage, | ||
organization: id, | ||
name: qParams.name, | ||
name: debouncedParams.name, | ||
...advancedFilter.filter, | ||
}, | ||
}), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { useQuery } from "@tanstack/react-query"; | ||
import { Link } from "raviger"; | ||
import { useState } from "react"; | ||
import { useEffect, useState } from "react"; | ||
|
||
import CareIcon from "@/CAREUI/icons/CareIcon"; | ||
|
||
|
@@ -11,6 +11,8 @@ import { Input } from "@/components/ui/input"; | |
|
||
import Pagination from "@/components/Common/Pagination"; | ||
|
||
import useDebouncedState from "@/hooks/useDebouncedState"; | ||
|
||
import query from "@/Utils/request/query"; | ||
import { Organization, getOrgLabel } from "@/types/organization/organization"; | ||
import organizationApi from "@/types/organization/organizationApi"; | ||
|
@@ -25,16 +27,26 @@ interface Props { | |
export default function OrganizationView({ id, navOrganizationId }: Props) { | ||
const [page, setPage] = useState(1); | ||
const [searchQuery, setSearchQuery] = useState(""); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be debounced instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
i tried this one but problem is If searchQuery is debounced directly, the input's
To fix this, we need two states but for distinct purposes: A searchQuery state to handle the immediate user input. |
||
|
||
const [debouncedParams, setDebouncedParams] = useDebouncedState( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this okay ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nope. i don't see any purpose for having two states for debouncing 1 variable |
||
searchQuery, | ||
500, | ||
); | ||
|
||
useEffect(() => { | ||
setDebouncedParams(searchQuery); | ||
}, [searchQuery]); | ||
|
||
const limit = 12; // 3x4 grid | ||
|
||
const { data: children, isLoading } = useQuery({ | ||
queryKey: ["organization", id, "children", page, limit, searchQuery], | ||
queryKey: ["organization", id, "children", page, limit, debouncedParams], | ||
queryFn: query(organizationApi.list, { | ||
queryParams: { | ||
parent: id, | ||
offset: (page - 1) * limit, | ||
limit, | ||
name: searchQuery || undefined, | ||
name: debouncedParams || undefined, | ||
}, | ||
}), | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,4 +7,3 @@ export interface Message { | |
created_by: UserBase; | ||
updated_by: UserBase; | ||
} | ||
|
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.
🛠️ Refactor suggestion
Reconsider debouncing the entire qParams object.
While debouncing works well for search queries, debouncing pagination parameters might lead to a degraded user experience. Consider splitting the debouncing logic to only apply to the search parameters.