added infinite scroll in search results #383
Conversation
WalkthroughThe changes integrate infinite scrolling into the Search component. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchComp as Search Component
participant Observer as IntersectionObserver
participant API as Server API
User->>SearchComp: Scrolls down
SearchComp->>Observer: Observe bottom sentinel div
Observer-->>SearchComp: Sentinel visible & more results available
SearchComp->>SearchComp: Set isLoading = true
SearchComp->>API: Fetch next page of results
API-->>SearchComp: Return additional results
SearchComp->>SearchComp: Append results and set isLoading = false
SearchComp->>Observer: Disconnect if no more results
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @MayankBansal2004, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request by MayankBansal2004 introduces infinite scrolling to the search results page. The primary goal is to enhance the user experience by automatically loading more results as the user scrolls down, eliminating the need for manual pagination. The changes involve adding an intersection observer to detect when the user reaches the bottom of the result list, triggering the loading of additional results. The PR also includes logic to manage the loading state and prevent redundant requests.
Highlights
- Infinite Scroll Implementation: Implemented infinite scrolling using an Intersection Observer to detect when the user reaches the bottom of the search results.
- Loading State Management: Added a loading state to indicate when more results are being fetched and prevent multiple simultaneous requests.
- Conditional Rendering of Loading Indicator: Implemented conditional rendering of a loading indicator at the bottom of the search results while new data is being fetched.
- Filter-Aware Pagination: Ensured that the infinite scroll functionality respects the applied filters and only loads more results within the filtered subset.
Changelog
- frontend/src/routes/_authenticated/search.tsx
- Removed unused
ChevronDownimport from line 39. - Added
bottomRefto track the bottom of the search results for the intersection observer on line 122. - Added
isLoadingstate to manage the loading state during infinite scroll on line 123. - Implemented a
useEffecthook with an Intersection Observer to trigger loading more results when the bottom of the list is visible, with checks to ensure results are available, the filter page size is greater than the current page, the results length is less than the filter page size, and that loading is not already in progress, starting on line 153. - Added logic to reset the loading state after results are received on line 374.
- Added logic to reset the loading state in case of an error during the search request on line 392.
- Removed
totalCountandfilterPageSizedeclarations from the main component body and moved them to the top of the component on lines 113 and 114. - Replaced the 'More Results' button with a loading indicator that appears during infinite scroll on lines 541-550.
- Removed unused
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
The scroll extends, a boundless quest,
For data sought, put to the test.
No click required,
New knowledge acquired,
Infinite content, truly blessed.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces infinite scrolling to the search results page, which should improve the user experience. The implementation appears to be well-structured, utilizing IntersectionObserver for efficient loading. However, there are a few areas that could be improved for clarity and robustness.
Summary of Findings
- Potential Race Condition: There's a potential race condition where
handleNextmight be called multiple times in quick succession if the intersection observer triggers rapidly. This could lead to multiple requests being fired off before theisLoadingstate has a chance to update, potentially causing unexpected behavior or data duplication. - Error Handling in Intersection Observer: The intersection observer's callback doesn't have explicit error handling. If
handleNextfails, theisLoadingstate might not be reset, preventing further loading. This could lead to a broken infinite scroll experience. - Redundant Condition in Loading Indicator: The loading indicator's visibility condition includes
filterPageSize > page, which seems redundant sinceresults.length < filterPageSizealready implies thatpageis not yet at the maximum.
Merge Readiness
The pull request is a good step towards improving the search results page. However, the potential race condition and lack of error handling in the intersection observer should be addressed before merging. I am unable to approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the high severity issues should be addressed before merging.
| setIsLoading(true) | ||
| handleNext() |
There was a problem hiding this comment.
Consider adding a check within handleNext to see if isLoading is already true to prevent multiple calls in quick succession. This can prevent race conditions where multiple requests are fired off before the state has a chance to update.
if (isLoading) {
return; // Prevent multiple calls
}
setIsLoading(true)
handleNext()| ) { | ||
| // Load more results when bottom is visible | ||
| setIsLoading(true) | ||
| handleNext() |
There was a problem hiding this comment.
Add a .finally() block to handleNext to ensure setIsLoading(false) is always called, even if there's an error. This prevents the loading indicator from getting stuck if the request fails.
setIsLoading(true)
handleNext()
.finally(() => setIsLoading(false));![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/routes/_authenticated/search.tsx (2)
113-119: Consider using useMemo for derived valuesThe variables
totalCountandfilterPageSizeare calculated on every render. Since they're used in the IntersectionObserver effect's dependency array and might be expensive to recalculate, consider memoizing them.-const totalCount = searchMeta?.totalCount || 0 -const filterPageSize = - filter.app && filter.entity - ? groups - ? groups[filter.app][filter.entity] - : totalCount - : totalCount +const totalCount = useMemo(() => searchMeta?.totalCount || 0, [searchMeta]) +const filterPageSize = useMemo(() => { + if (filter.app && filter.entity) { + return groups ? groups[filter.app][filter.entity] : totalCount + } + return totalCount +}, [filter.app, filter.entity, groups, totalCount])
541-551: Add feedback when all results are loadedThe infinite scroll implementation works well, but consider adding a message when all results have been loaded to provide better feedback to users.
{/* Infinite scroll loading indicator and bottom reference */} {results.length > 0 && ( <div ref={bottomRef} className="py-4 flex justify-center"> {isLoading && filterPageSize > page && results.length < filterPageSize ? ( <div className="flex items-center space-x-2"> <div className="w-4 h-4 rounded-full border-2 border-t-transparent border-blue-500 animate-spin"></div> <span className="text-gray-500">Loading more results...</span> </div> + ) : results.length >= filterPageSize ? ( + <div className="text-gray-500">All results loaded</div> ) : null} </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/routes/_authenticated/search.tsx(6 hunks)
🔇 Additional comments (3)
frontend/src/routes/_authenticated/search.tsx (3)
121-123: LGTM! Good implementation of infinite scroll state variablesThe approach of using
useReffor the bottom element reference anduseStatefor loading state is appropriate for implementing infinite scroll functionality.
373-375: LGTM! Proper loading state managementGood addition to reset the loading state after results are successfully received.
392-392: LGTM! Error state handlingGood addition to reset the loading state when an error occurs, preventing the UI from being stuck in a loading state.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/routes/_authenticated/search.tsx (2)
545-545: Simplify the loading indicator condition.The condition for showing the loading indicator can be simplified for better readability.
-{isLoading && filterPageSize > page && results.length < filterPageSize ? ( +{isLoading && results.length < filterPageSize ? (If
results.length < filterPageSize, it already implies that there are more results to load, making thefilterPageSize > pagecheck redundant.
172-174:⚠️ Potential issuePrevent race conditions in loading logic.
The current implementation could trigger multiple requests if the user scrolls quickly or the network is slow.
Implement proper loading state management by adding a check before triggering
handleNextand ensuring loading state is always reset:- setIsLoading(true) - handleNext() + if (isLoading) { + return; // Prevent multiple calls + } + setIsLoading(true) + handleNext() + .finally(() => setIsLoading(false));Since
handleNextdoesn't return a Promise, consider modifying it as suggested in the previous comment so that.finally()works correctly.
🧹 Nitpick comments (1)
frontend/src/routes/_authenticated/search.tsx (1)
158-177: Inconsistency in the IntersectionObserver threshold comment.The comment states "Trigger when 10% of the element is visible" but the threshold is set to 0.5 (50%).
- { threshold: 0.5 } // Trigger when 10% of the element is visible + { threshold: 0.5 } // Trigger when 50% of the element is visible
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/routes/_authenticated/search.tsx(6 hunks)
🔇 Additional comments (5)
frontend/src/routes/_authenticated/search.tsx (5)
113-124: Good job centralizing key variables for better readability!Moving
totalCountandfilterPageSizecalculations to the top of the component makes the code more maintainable, and adding clear comments about the purpose of the new variables helps with code clarity.
158-186: Missing handleNext in the useEffect dependency array.The dependency array for the IntersectionObserver useEffect should include
handleNextto avoid potential stale closure issues.Good job including
handleNextin the dependency array! This prevents stale closure issues ifhandleNextreferences state that changes over time.
378-381: Good practice setting loading state after success.Explicitly resetting the loading state after results are received is a good practice to ensure the UI accurately reflects the application state.
397-397: Good error handling with loading state reset.Resetting the loading state in the error case is crucial to prevent the UI from being stuck in a loading state when errors occur.
542-552: Well-implemented infinite scroll UI element.The loading indicator is clearly visible and provides good feedback to users. The implementation with a separate div element for the observer reference is clean.
| <div ref={bottomRef} className="py-4 flex justify-center"> | ||
| {isLoading && filterPageSize > page && results.length < filterPageSize ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-4 h-4 rounded-full border-2 border-t-transparent border-blue-500 animate-spin"></div> |
There was a problem hiding this comment.
Instead of this div, use LoaderContent component from common.tsx
| ) // State for debug info visibility, initialized from env var | ||
| const [traceData, setTraceData] = useState<any | null>(null) // State for trace data | ||
| // close autocomplete if clicked outside | ||
| ) |
There was a problem hiding this comment.
Why are we removing important comments and todos? Add them again everywhere.
| observerRef.current = new IntersectionObserver( | ||
| (entries) => { | ||
| if (entries[0].isIntersecting && !isLoading) { | ||
| const totalCount = searchMeta?.totalCount || 0 |
There was a problem hiding this comment.
totalCount and filterPageSize consts are derived twice in this file. Just derive them once.
| const autocompleteRef = useRef<HTMLDivElement | null>(null) | ||
| const [autocompleteQuery, setAutocompleteQuery] = useState("") | ||
| const observerRef = useRef<IntersectionObserver | null>(null) |
There was a problem hiding this comment.
Don't use ref for the observer itself. Go back to the previous version what you had done before.
| /> | ||
| ))} | ||
| </div> | ||
| {results.length > 0 && results.length < filterPageSize && ( | ||
| <div ref={loadMoreRef} className="h-10"></div> |
There was a problem hiding this comment.
Are we sure we want to remove Loading animation? What happens when user has reached the bottom of search results and still the next page results are being fetched. Check with slow internet.
Description
added the infinite scroll in the search results.
Testing
i have tested visually it's working well.
Additional Notes
Summary by CodeRabbit
Summary by CodeRabbit