fix(web): grant filter ui #17871
fix(web): grant filter ui #17871
Conversation
WalkthroughThe updates adjust the visual layout of the Grants search results page. In the SearchResults.tsx file, an existing Box component displaying the hits message is removed and replaced with a new Box (with updated margin properties) placed below the search filter. Additionally, the variant prop of the GrantsSearchResultsFilter is changed from 'default' to 'dialog' for mobile layouts. In the SearchResultsFilter.tsx file, the label for clearing filters is updated from a category-specific message to a general one. No changes were made to functional logic or public APIs. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
|
View your CI Pipeline Execution ↗ for commit 263265d.
☁️ Nx Cloud last updated this comment at |
![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 (2)
apps/web/screens/Grants/SearchResults/SearchResults.tsx (2)
338-396: Consider extracting mobile view into a separate component.The mobile view logic is quite large and could be extracted into a separate component to improve maintainability and readability.
Example refactor:
+ const MobileSearchResults = ({ + query, + setQuery, + categories, + types, + organizations, + status, + onSearchFilterUpdate, + onResetFilter, + tags, + currentUrl, + totalHits, + hitsMessage, + grants, + locale, + totalPages, + setPage, + }) => ( + <Box padding={[1, 1, 2]} margin={[1, 1, 2]} paddingBottom={3}> + {/* ... mobile view JSX ... */} + </Box> + ) return ( <GrantWrapper> {/* ... */} <Box background="blue100"> {!isMobile && ( <SidebarLayout> {/* ... desktop view ... */} </SidebarLayout> )} - {isMobile && ( - <Box padding={[1, 1, 2]} margin={[1, 1, 2]} paddingBottom={3}> - {/* ... mobile view ... */} - </Box> - )} + {isMobile && ( + <MobileSearchResults + query={query} + setQuery={setQuery} + categories={categories} + types={types} + organizations={organizations} + status={status} + onSearchFilterUpdate={onSearchFilterUpdate} + onResetFilter={onResetFilter} + tags={tags} + currentUrl={currentUrl} + totalHits={totalHits} + hitsMessage={hitsMessage} + grants={grants} + locale={locale} + totalPages={totalPages} + setPage={setPage} + /> + )} </Box> </GrantWrapper> )
96-99: Consider using TypeScript's satisfies operator for better type safety.The GraphQL query variables could benefit from using the
satisfiesoperator to ensure type safety while maintaining type inference.Example:
- getGrants({ - variables: { + const variables = { input: { categories: categories ? [...categories] : null, lang: locale, organizations: organizations ? [...organizations] : null, page, search: searchString, size: PAGE_SIZE, types: types ? [...types] : null, status: status ? status === 'closed' ? GetGrantsInputAvailabilityStatusEnum.Closed : GetGrantsInputAvailabilityStatusEnum.Open : undefined, }, - }, - }) + } satisfies { input: QueryGetGrantsArgs['input'] } + getGrants({ variables })Also applies to: 113-130
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/screens/Grants/SearchResults/SearchResults.tsx(2 hunks)apps/web/screens/Grants/SearchResults/SearchResultsFilter.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/screens/Grants/SearchResults/SearchResultsFilter.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`apps/**/*`: "Confirm that the code adheres to the following...
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."
apps/web/screens/Grants/SearchResults/SearchResults.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: tests (web)
- GitHub Check: linting (web)
- GitHub Check: build (web)
- GitHub Check: formatting
- GitHub Check: linting-workspace
🔇 Additional comments (1)
apps/web/screens/Grants/SearchResults/SearchResults.tsx (1)
354-372: LGTM! Mobile UI improvements look good.The changes improve the mobile layout by:
- Adding consistent vertical spacing with
marginY- Using dialog variant for better mobile interaction
- Moving the hits message below the filter for better visual hierarchy
What
starfatorgmoreScreenshots / Gifs
Checklist:
Summary by CodeRabbit