fix(web): grants pagination reset page count #17665
Conversation
WalkthroughThe pull request modifies the search functionality in the Grants Search Results screen by introducing a new Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/screens/Grants/SearchResults/SearchResults.tsx (1)
89-89: Consider deriving searchString from query parameterThe
searchStringstate duplicates information already present in the URL query parameter. Consider deriving it fromqueryto avoid state synchronization issues.-const [searchString, setSearchString] = useState<string | null>() +const searchString = query
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/screens/Grants/SearchResults/SearchResults.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/web/screens/Grants/SearchResults/SearchResults.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (5)
apps/web/screens/Grants/SearchResults/SearchResults.tsx (5)
120-120: LGTM!The search parameter is correctly passed to the GraphQL query.
161-166: LGTM!The debounced search update correctly resets pagination and updates the search string.
192-194: LGTM!The reset functionality correctly clears all filters and resets pagination.
154-157: Review ESLint dependency warning suppressionThe ESLint disable comment suggests potential missing dependencies. Consider including all dependencies or extracting the fetch logic to avoid the warning.
✅ Verification successful
ESLint dependency warning suppression is appropriate
The current implementation using useCallback with the ESLint warning suppression is correct. The effect's dependency array properly includes all relevant dependencies (categories, locale, organizations, page, searchString, types, status) while avoiding unnecessary re-renders from the stable fetchGrants callback.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other useEffect hooks with disabled exhaustive-deps rg -A 2 "useEffect.*\(" | rg "eslint-disable.*exhaustive-deps" -B 2Length of output: 3256
Script:
#!/bin/bash # Look for fetchGrants definition in SearchResults.tsx ast-grep --pattern 'const fetchGrants = async () => { $$$ }'Length of output: 62
Script:
#!/bin/bash # Search for fetchGrants with context rg "fetchGrants" apps/web/screens/Grants/SearchResults/SearchResults.tsx -B 2 -A 5Length of output: 486
240-240: LGTM! Verify similar pagination patternsThe pagination reset on filter change is correctly implemented. Consider checking other similar patterns in the codebase.
✅ Verification successful
Pagination reset pattern is correctly implemented
The implementation follows the established pattern in the codebase where pagination state is reset when filters change. This is consistently implemented across multiple components including WorkMachinesOverview, DentistRegistration, MedicineCalculator, and others.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for similar pagination patterns that might need the same fix ast-grep --pattern 'setPage($value)' | rg -A 5 -B 5 'setPage'Length of output: 61
Script:
#!/bin/bash # Search for page-related state declarations and their usage rg -g '*.{ts,tsx,js,jsx}' -i 'const.*\[.*page.*,.*set.*page.*\].*useState' # Search for pagination-related components and their implementations rg -g '*.{ts,tsx,js,jsx}' -i '(pagination|paginate)' -A 3 -B 3 # Look for filter-related components with potential pagination rg -g '*.{ts,tsx,js,jsx}' -i '(filter.*page|page.*filter)' -A 3 -B 3Length of output: 66996
What
Why
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit