feat(learn): enhance search and article fetching functionality #5461
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThis pull request updates the article fetching and pagination logic across several learn-related pages. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CMS as CMS Service
Client->>CMS: getArticles({ page: 0, pageSize: 100, fetchAll: true })
Note right of CMS: Fetch articles for page 0
loop While more pages exist
CMS->>CMS: Recursive call: getArticles({ page: nextPage, ... })
Note right of CMS: Aggregating articles from each page
end
CMS-->>Client: Combined article list response
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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
CodeRabbit Configuration File (
|
![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 (6)
apps/cow-fi/app/(learn)/learn/topics/page.tsx (1)
9-9: Consider addingfetchAll: trueparameter for consistency with other pages.You've specified a
pageSize: 200here, but other pages like/learn/[article]/page.tsxand/learn/page.tsxare usingfetchAll: trueto ensure all articles are retrieved for search functionality. For consistency in search behavior across the app, consider adding thefetchAllparameter here as well.- const articlesResponse = await getArticles({ pageSize: 200 }) + const articlesResponse = await getArticles({ pageSize: 200, fetchAll: true })apps/cow-fi/components/TopicPageComponent.tsx (1)
97-97: Consider using a more specific type thanany[].Using
any[]reduces type safety. If there's a specific article type defined in the codebase, consider using that instead for better type checking and IDE assistance.- allArticles?: any[] + allArticles?: Article[] // Replace with your actual article typeapps/cow-fi/components/SearchBar.tsx (4)
125-134: Consider cross-browser support for line clamping.
Whileline-clamp: 2; box-orient: vertical;works in some browsers, consider prefixed properties (e.g.-webkit-line-clamp) if older browsers need support.+/* Add prefixed properties for broader browser support */ +display: -webkit-box; +-webkit-line-clamp: 2; +-webkit-box-orient: vertical; line-clamp: 2; box-orient: vertical; overflow: hidden;
233-297: Complex multi-criteria search logic may become expensive for large datasets.
This chunk implements multi-layered filtering (substrings, prefixes, multi-word checks) per article, which is fine for moderate volumes. However, if the article set grows significantly, or if scaling is anticipated, consider an indexing-based approach (e.g., Elasticlunr, Fuse.js, or server-side search) for efficiency and maintainability.
305-322: Phrase-generation logic is correct but might be simplified or cached.
Generating all possible phrases is quadratic in the number of words, which is acceptable for short content. If performance becomes a concern, consider memoizing or refactoring.
325-379: Complex highlight logic could benefit from unit tests.
This multi-word, prefix-based highlighting is robust but intricate. To ensure correctness and guard against regressions, consider adding automated tests for diverse inputs (single word, multi-word, special characters, etc.).Would you like help generating such tests or a new issue to track them?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/cow-fi/app/(learn)/learn/[article]/page.tsx(1 hunks)apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx(3 hunks)apps/cow-fi/app/(learn)/learn/page.tsx(2 hunks)apps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx(3 hunks)apps/cow-fi/app/(learn)/learn/topics/page.tsx(1 hunks)apps/cow-fi/components/ArticlesPageComponents.tsx(1 hunks)apps/cow-fi/components/SearchBar.tsx(4 hunks)apps/cow-fi/components/TopicPageComponent.tsx(2 hunks)apps/cow-fi/services/cms/index.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (22)
apps/cow-fi/app/(learn)/learn/[article]/page.tsx (1)
65-67: LGTM! Appropriate implementation for fetching all articles.The comment clearly indicates the purpose, and the implementation correctly uses the
fetchAll: trueparameter to ensure comprehensive search functionality.apps/cow-fi/app/(learn)/learn/page.tsx (3)
7-9: Function rename and commenting enhances code clarity.Good job renaming from
Pageto the more descriptiveLearnPageand adding explanatory comments about the article fetching logic.
12-20: Increased pageSize for featured articles.You've increased the featured articles limit from 6 to 7. This matches the PR objective of enhancing article retrieval capabilities.
8-9:✅ Verification successful
Verify if both pageSize and fetchAll are necessary.
You're setting both
pageSize: 200andfetchAll: true. IffetchAllis designed to recursively fetch all articles regardless of total count, thepageSizeparameter might be redundant. Consider verifying if both parameters are needed or if one might override the other.
🏁 Script executed:
#!/bin/bash # Find and examine the getArticles implementation to understand how these parameters interact rg -A 10 "getArticles" --glob "*.ts" | grep -A 10 "function getArticles"Length of output: 854
Clarification on the use of both
pageSizeandfetchAllin thegetArticlescallAfter verifying the implementation in
apps/cow-fi/services/cms/index.ts, it’s clear that:
pageSizecontrols how many articles are fetched per individual request.fetchAlltriggers recursive fetching to gather all articles across pages.Thus, when you call:
const articlesResponse = await getArticles({ pageSize: 200, fetchAll: true })passing both parameters is intentional. The
pageSizeisn’t redundant even withfetchAllenabled—it still sets the batch size for each fetch iteration. If your goal is to explicitly control the batch size (200 articles per request), then retaining both is appropriate. Alternatively, if you want to rely on the default batch size, you could omit the explicitpageSize.apps/cow-fi/components/TopicPageComponent.tsx (2)
100-100: LGTM! Clean parameter implementation.The function signature has been updated correctly to include the new optional parameter.
109-109: Effective implementation of article fallback logic.The search bar now correctly uses all articles when available, falling back to topic-specific articles otherwise. This enhances search functionality as intended.
apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx (3)
7-7: Increased page size aligns with improved pagination strategy.Doubling the
ITEMS_PER_PAGEfrom 24 to 48 will reduce the number of pagination requests needed, enhancing performance and user experience while handling a growing article library.
41-47: Well-implemented article fetching strategy for search functionality.Good separation of concerns between paginated display articles and comprehensive search data. This implementation aligns with the PR objective of ensuring all articles are accessible for search, regardless of the total count.
75-75: LGTM - SearchBar now has access to the complete article collection.Passing
allArticlesto theArticlesPageComponentscorrectly implements the enhanced search functionality.apps/cow-fi/components/ArticlesPageComponents.tsx (3)
64-64: LGTM - Interface correctly updated with optional property.The optional
allArticlesproperty allows for backward compatibility while supporting the new enhanced search functionality.
67-73: Function signature properly updated to match interface changes.The destructuring pattern is maintained with the addition of the new parameter, keeping the code consistent.
80-80: Good fallback implementation for the SearchBar component.Using
allArticles || articlesprovides a clean fallback mechanism whenallArticlesis not provided, ensuring backward compatibility.apps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx (4)
43-44: Clear variable naming improves readability.The comment and renaming to
topicArticlesResponsebetter clarifies the purpose of this API call for topic-specific articles.
56-59: Appropriate implementation of all-articles fetching for search.This addition ensures the search functionality has access to the complete article collection, consistent with the implementation in other page components.
61-62: Clear variable naming for distinct article collections.Separating
topicArticlesandallArticlesclearly distinguishes between the two datasets and their purposes.
71-78: Component props updated correctly to support enhanced search.The updated return statement properly passes both topic-specific articles for display and all articles for search functionality.
apps/cow-fi/services/cms/index.ts (3)
8-8: Increased PAGE_SIZE improves efficiency of article fetching.Doubling the page size from 50 to 100 reduces the number of API calls needed to fetch all articles, which is a good optimization.
116-121: Function signature correctly updated with fetchAll parameter.Adding the optional
fetchAllparameter with a default value offalsemaintains backward compatibility while enabling the new recursive pagination functionality.
150-170:❓ Verification inconclusive
Well-implemented recursive pagination strategy.
The recursive approach effectively handles pagination to fetch all articles when needed. This solves the limitation mentioned in the PR objectives by ensuring all articles are accessible for search regardless of their total count.
Consider monitoring the performance as the article count grows beyond 500, as recursive API calls may impact load times.
🌐 Web query:
What are the best practices for implementing pagination in NextJS when dealing with large datasets?💡 Result:
Implementing efficient pagination in Next.js for large datasets requires a combination of server-side data handling, optimized client-side navigation, and strategic use of Next.js features. Here are key best practices:
Core Implementation Strategies
Server-Side Pagination
Fetch only required data chunks using APIs supportingpage/pageSizeor cursor-based parameters[1][6]. Example Strapi implementation:export async function getSummaries(queryString: string, currentPage: number) { const PAGE_SIZE = 4; const query = qs.stringify({ pagination: { pageSize: PAGE_SIZE, page: currentPage } }); // API call }URL Parameter Management
Use Next.js hooks for state synchronization:const pathname = usePathname(); const searchParams = useSearchParams(); const router = useRouter(); const createPageURL = (pageNumber: number) => { const params = new URLSearchParams(searchParams); params.set('page', pageNumber.toString()); router.push(`${pathname}?${params.toString()}`); };Advanced Techniques
Cursor-Based Pagination
Preferred for massive datasets (>10k records) due to:
- Better performance with indexed queries[3][6]
- Stable results during data mutations[3]
Shallow Routing
Update URL without full page reloads:router.push(`?page=${newPage}`, undefined { shallow: true });UI/UX Considerations
Pagination Component Features
- Previous/Next buttons with disabled states[1][7]
- Current page indicator
- Responsive page number display
Metadata Handling
IncludepageCountfrom API responses to control navigation:<PaginationComponent pageCount={meta.pagination.pageCount} />Performance Optimization Table
Technique Benefit Implementation Example Server-Side Filter Reduces transferred data size Combine with search params[4][9] Cursor Pagination Efficient deep navigation Use after/beforequery params[3][6]Static Generation Cache common pages getStaticPropswith revalidate[8]Anti-Patterns to Avoid
- Client-side filtering of large datasets
- Offset pagination beyond ~500 pages[3]
- Loading full dataset before paginating
When combining with search/filters:
// Handle both search and pagination const handleSearch = (term: string) => { const params = new URLSearchParams(searchParams); params.set('query', term); params.delete('page'); // Reset to first page router.push(`${pathname}?${params.toString()}`); };For datasets exceeding 100k records, consider hybrid approaches combining cursor pagination with temporal segmentation (e.g., date ranges)[6]. Always validate performance using tools like Lighthouse or WebPageTest during implementation[5].
Citations:
- 1: https://strapi.io/blog/epic-next-js-14-tutorial-part-8-search-and-pagination-in-next-js
- 2: https://docs.directus.io/blog/implementing-pagination-and-infinite-scrolling-in-next-js
- 3: https://codedamn.com/news/backend/rest-api-pagination-handling-large-data-sets
- 4: https://www.dhiwise.com/post/ultimate-guide-to-nextjs-pagination-for-beginners
- 5: https://clouddevs.com/next/react-paginate/
- 6: https://pagepro.co/blog/5-lessons-for-next-js-performance-optimization-in-large-projects/
- 7: https://bejamas.com/hub/guides/building-pagination-component-nextjs-strapi
- 8: https://strapi.io/integrations/nextjs-cms/topics/next-js-pagination
- 9: https://nextjs.org/learn/dashboard-app/adding-search-and-pagination
- 10: How to solve "Large Page Data" issue with Nextjs? vercel/next.js#39880
Pagination Strategy Implementation – Effective, with a Note on Scalability
The recursive pagination approach currently ensures that all articles are fetched for comprehensive search functionality, which meets the PR objectives. However, given that recursive API calls can introduce performance overhead when the dataset grows (e.g., beyond 500 articles), please monitor load times closely. In the long term, you might consider alternative strategies (like server-side or cursor-based pagination) as recommended best practices for handling very large datasets in Next.js.
- Actionable points:
- Continue using the recursive approach for now.
- Keep an eye on performance metrics as the article count increases.
- Evaluate alternative pagination methods if scaling impacts performance.
apps/cow-fi/components/SearchBar.tsx (3)
99-100: Layout direction change looks good.
Switching to a column layout with left-aligned content for result items is clear and consistent with the new design requirement.
119-123: New component for result titles is well-defined.
Implementing a separate styled component for the title improves readability and makes the layout more maintainable.
461-464: Usage of new styled components for title and description is on point.
The integration ofResultTitleandResultDescriptionwithhighlightQueryis consistent, ensuring both readability and highlight functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cow-fi/components/TopicPageComponent.tsx (1)
21-21: Fix import ordering.The import statement for
Articlefrom '../services/cms' should be placed before the local import from '@/components/CategoryLinks' to follow the standard import ordering convention.-import Link from 'next/link' -import { Article } from '../services/cms' +import Link from 'next/link' + +import { Article } from '../services/cms'🧰 Tools
🪛 ESLint
[error] 21-21:
../services/cmsimport should occur before import of@/components/CategoryLinks(import/order)
apps/cow-fi/util/textHighlighting.tsx (1)
1-3: Fix import spacing.Add an empty line between import statements to follow the standard import ordering convention.
import React from 'react' import styled from 'styled-components/macro' +🧰 Tools
🪛 ESLint
[error] 1-1: There should be at least one empty line between import groups
(import/order)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/cow-fi/app/(learn)/learn/topics/page.tsx(1 hunks)apps/cow-fi/components/SearchBar.tsx(6 hunks)apps/cow-fi/components/TopicPageComponent.tsx(3 hunks)apps/cow-fi/util/textHighlighting.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cow-fi/app/(learn)/learn/topics/page.tsx
🧰 Additional context used
🪛 ESLint
apps/cow-fi/util/textHighlighting.tsx
[error] 1-1: There should be at least one empty line between import groups
(import/order)
apps/cow-fi/components/TopicPageComponent.tsx
[error] 21-21: ../services/cms import should occur before import of @/components/CategoryLinks
(import/order)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (14)
apps/cow-fi/components/TopicPageComponent.tsx (2)
96-98: Type enhancement improves code robustness.Replacing
any[]with the properArticle[]type and adding the optionalallArticlesproperty enhances type safety and documents the component's expected inputs.
101-101: Enhanced search with comprehensive article access.The function signature now accepts
allArticlesand properly passes it to theSearchBarcomponent, enabling search across all articles rather than just the topic-specific ones. This supports the PR's objective to ensure all articles are accessible for search.Also applies to: 110-110
apps/cow-fi/components/SearchBar.tsx (7)
8-8: Updated import forhighlightQuery.The import has been updated to use the new text highlighting utility file.
100-102: Improved search result item layout.Changed from row to column layout with items aligned to the left, which improves readability of search results.
120-141: Enhanced search result display with title and description components.Adding separate styled components for result titles and descriptions improves the UI by:
- Making titles stand out with medium font weight
- Limiting descriptions to 2 lines with ellipsis overflow
- Using appropriate styling for better visual hierarchy
The implementation correctly includes cross-browser support with prefixed properties.
195-212: Performance optimization with debouncing and caching.The implementation adds:
- A cache for phrases to avoid recomputation
- Constants for optimization parameters
- Debounced query state to reduce processing frequency
This is a good pattern for optimizing search performance, especially as the article count grows beyond the current 89 articles.
252-335: Comprehensive search algorithm implementation.The improved search algorithm provides multiple matching strategies:
- Full-term matches in title, description, or slug
- Prefix matches for any word in the content
- Multi-word search support with partial matching
- Phrase-based matching with performance optimizations
This directly addresses the PR objective of enhancing search functionality across all articles, properly handling the growing content library.
344-373: Efficient phrase generation with caching.The
findPhraseshelper function efficiently generates all possible phrases from text using:
- Caching to avoid redundant computation
- Limited phrase length for performance
- Early returns for empty inputs
This optimization is essential for maintaining good performance as the article count grows.
414-417: Enhanced search result presentation.The implementation:
- Displays result titles with proper highlighting
- Conditionally renders descriptions with highlighting
- Uses the new styled components for consistent styling
This improves user experience by making search results more readable and visually appealing.
apps/cow-fi/util/textHighlighting.tsx (5)
4-6: Simple highlighting implementation.The
HighlightedTextstyled component provides a clear visual indicator for matched text.
14-26: Well-documented utility functions for text matching.The
wordStartsWithandwordExactlyMatchesfunctions are:
- Well-documented with JSDoc comments
- Properly handling case-insensitive matching
- Focused on a single responsibility
These functions provide a clean abstraction for the text highlighting system.
36-53: Robust substring highlighting implementation.The
highlightSubstringfunction:
- Includes proper validation of indices
- Handles edge cases (highlighting at start, middle, or end)
- Returns appropriate React elements
This function correctly implements the core highlighting functionality needed for the search experience.
63-105: Comprehensive word processing for highlighting.The
processWordForHighlightingfunction handles various search scenarios:
- Multi-word searches with prefix matching for the last word
- Exact matching for previous words in multi-word searches
- Substring highlighting for single-word searches
- Proper handling of whitespace
This implementation correctly supports all the search patterns needed by the enhanced search functionality.
113-141: Well-structured text highlighting algorithm.The
highlightQueryfunction:
- Properly handles empty inputs
- Has distinct logic paths for multi-word and single-word searches
- Efficiently processes text by splitting on whitespace
- Correctly preserves whitespace in the output
This implementation effectively supports the enhanced search experience across the application.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/cow-fi/components/SearchBar.tsx (3)
120-141: Enhanced search result presentation with semantic structureThe new
ResultTitleandResultDescriptionstyled components provide better semantic structure and visual hierarchy compared to the previous implementation.Note: The non-standard
box-orientproperty (line 140) has limited browser support. The prefixed versions should work in most browsers but consider monitoring for any display issues.
195-212: Performance optimization with caching and debouncingGood implementation of performance optimizations:
- Phrase caching prevents redundant computations
- Debouncing reduces unnecessary processing during rapid typing
Consider adding a cache size limit to prevent potential memory issues with very large datasets:
// Cache for memoized phrases to avoid recomputation const phrasesCache = useRef<Record<string, string[]>>({}) +const MAX_CACHE_ENTRIES = 100 // Limit cache size +// Helper to manage cache size +const addToCache = (key: string, value: string[]) => { + // If cache is full, remove oldest entry + const cacheKeys = Object.keys(phrasesCache.current) + if (cacheKeys.length >= MAX_CACHE_ENTRIES) { + delete phrasesCache.current[cacheKeys[0]] + } + phrasesCache.current[key] = value +}Update line 370 to use this helper:
-phrasesCache.current[text] = phrases +addToCache(text, phrases)
252-335: Comprehensive search algorithm with multi-strategy approachThe enhanced search algorithm provides a significantly better user experience by:
- Supporting full-term matches in multiple fields
- Enabling prefix matching for words
- Handling multi-word searches intelligently
- Implementing phrase matching for more natural language searches
The performance considerations comment is valuable for future maintenance.
One optimization opportunity: The redundant conditions check on lines 312-319 repeats the same conditions from lines 268-283. Consider refactoring to avoid this duplication:
// 4. Check if the entire search term is a prefix of any phrase in the content // Only perform expensive phrase checking if other checks fail and the content is not too long -if ( - !title.includes(searchTerm) && - !description.includes(searchTerm) && - !slug.includes(searchTerm) && - !titleWords.some((word) => word.startsWith(searchTerm)) && - !descriptionWords.some((word) => word.startsWith(searchTerm)) && - !slugWords.some((word) => word.startsWith(searchTerm)) -) { +// Only perform phrase checking if previous checks didn't match +if (!foundMatch) { // Lazy evaluation - only generate phrases if needed const titlePhrases = title.length < 1000 ? findPhrases(title) : [] const descriptionPhrases = description.length < 1000 ? findPhrases(description) : [] const slugPhrases = findPhrases(slug) // Slugs are typically shortThis would require adding a
foundMatchvariable earlier in the function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cow-fi/components/SearchBar.tsx(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Cypress
🔇 Additional comments (5)
apps/cow-fi/components/SearchBar.tsx (5)
8-8: Good addition of highlighting functionalityThe import of
highlightQueryimproves the search user experience by visually emphasizing matching terms within search results.
95-95: Improved spacing for search resultsThe additional bottom padding (54px) provides better visual separation and ensures content isn't cut off when scrolling through search results.
100-101: Better layout for search resultsChanging from row to column layout with left alignment improves readability and provides a more natural reading flow for search result items.
343-373: Efficient phrase generation with cachingThe
findPhrasesfunction is well-implemented with:
- Early returns for empty input
- Cache utilization to avoid redundant processing
- Reasonable phrase length limits
The algorithm efficiently generates all possible consecutive word combinations up to MAX_PHRASE_LENGTH, enhancing the search capability without excessive computational cost.
414-417: Improved search result display with highlighted matchesThe enhanced search result display now shows:
- Highlighted title that visually indicates matching terms
- Conditional rendering of description with highlighted terms
- Better visual structure with the new semantic components
This significantly improves the search experience by providing more context and making matches more visible to users.
…ed search functionality
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/cow-fi/components/TopicPageComponent.tsx (2)
21-21: Reorder the imports to satisfy the linter.ESLint suggests placing the relative import (
../services/cms) before the alias imports (e.g.,@/components/CategoryLinks). Reordering import statements can help keep imports organized as indicated by style rules.-import { CategoryLinks } from '@/components/CategoryLinks' -import { Article } from '../services/cms' +import { Article } from '../services/cms' +import { CategoryLinks } from '@/components/CategoryLinks'🧰 Tools
🪛 ESLint
[error] 21-21:
../services/cmsimport should occur before import of@/components/CategoryLinks(import/order)
101-108: Validate fallback usage for SearchBar.You’re passing
allArticles || articlesto theSearchBar, but sinceallArticlesis no longer optional, this fallback may be redundant. IfallArticlesis always defined, consider removing the fallback logic to simplify the code.- <SearchBar articles={allArticles || articles} /> + <SearchBar articles={allArticles} />apps/cow-fi/components/SearchBar/index.tsx (3)
77-113: Consider stricter input validation.The component sets and debounces the query, but extreme or malicious input might lead to unexpected rendering or performance overhead. Consider validating or sanitizing user input (e.g., length or special characters) before searching.
114-148: Server-side search vs. local state.Your approach immediately fetches articles from the server on every new query (after the debounce delay). This is fine for moderate datasets. However, if the dataset grows significantly or if usage scales up, you might need caching or advanced rate-limiting to avoid flooding the server with requests.
194-202: Optionally refine click-outside behavior for small screens.You close search results on click outside only for medium screens and up. You might want consistent behavior across all screen sizes, depending on your UX requirements.
apps/cow-fi/services/cms/index.ts (1)
121-122: Optional recursion is a versatile enhancement.Allowing
fetchAllkeeps it flexible for one-page or multi-page fetching. Ensure calling code is aware of its cost if the total number of articles grows significantly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cow-fi/components/ArticlesPageComponents.tsx(1 hunks)apps/cow-fi/components/SearchBar.tsx(0 hunks)apps/cow-fi/components/SearchBar/index.tsx(1 hunks)apps/cow-fi/components/SearchBar/styled.ts(1 hunks)apps/cow-fi/components/TopicPageComponent.tsx(3 hunks)apps/cow-fi/services/cms/index.ts(3 hunks)
💤 Files with no reviewable changes (1)
- apps/cow-fi/components/SearchBar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cow-fi/components/ArticlesPageComponents.tsx
🧰 Additional context used
🪛 ESLint
apps/cow-fi/components/SearchBar/styled.ts
[error] 1-1: There should be at least one empty line between import groups
(import/order)
[error] 2-2: @cowprotocol/ui import should occur before import of styled-components/macro
(import/order)
apps/cow-fi/components/TopicPageComponent.tsx
[error] 21-21: ../services/cms import should occur before import of @/components/CategoryLinks
(import/order)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (11)
apps/cow-fi/components/TopicPageComponent.tsx (2)
96-98: Type-check improvement is appreciated.Replacing
any[]withArticle[]and addingallArticlesclarifies the data shape and provides better type safety. This is a solid improvement.
149-166: Guard the consumable fields ofcategory.You’re directly accessing
category.name,category.imageUrl, andcategory.description. Ensure these fields always exist or introduce graceful handling whencategorymight be missing a validattributesobject. This can prevent runtime errors if the CMS data changes unexpectedly.apps/cow-fi/components/SearchBar/index.tsx (5)
1-2: Confirm that these external hooks are robust for SSR usage.You’re importing hooks (
useMediaQuery,useOnClickOutside,useDebounce) from@cowprotocol/common-hooks. Verify that they handle server-side rendering gracefully, or confirm they won’t be invoked server-side. This helps avoid hydration mismatches and ensures consistent behavior across different rendering environments.
28-71: Good separation of concerns for keyboard navigation.Creating a custom hook for arrow key navigation keeps the search logic simpler. The logic for resetting the index on deactivation is well handled. No immediate issues found.
150-172: Parallel "load more" calls.
loadMoreResultsrelies onisLoadingMoreto prevent overlapping requests. This is a good approach. Just remember to reset any flags properly if an error occurs so subsequent “load more” attempts can proceed.
184-192: Clear function logic is correct.Resetting state and refocusing the input is a thoughtful UX detail. Confirm you want to preserve the search results if the user re-enters the same query. Currently, clearing resets all states, effectively discarding any prior results.
210-299: Robust rendering and ARIA attributes.Providing ARIA attributes like
aria-expandedandrole="listbox"promotes accessibility. Also, highlighting the query in results is a nice UX touch. Ensure tested with screen readers for compliance.apps/cow-fi/services/cms/index.ts (2)
8-10: Increased page size and added limit are aligned with the feedback.Raising
PAGE_SIZEto 100 and introducingARTICLES_FETCHING_PAGE_LIMITto 10 matches previous code review suggestions, preventing an unbounded recursion scenario.
151-175: Thoughtful recursion for multi-page fetch.Your logic correctly merges results across pages and updates the
meta.pagination.totalto reflect the overall count. Verify you don’t break if an intermediate call fails (e.g., partial success handling).apps/cow-fi/components/SearchBar/styled.ts (2)
4-257: Well-structured and responsive styled components.The styled components are well-organized with comprehensive styling for different states (hover, focus, active) and include proper responsive design using media queries. The scrollbar customization is thorough, covering multiple browsers including Firefox, Webkit browsers, and iOS. The components provide excellent visual feedback for user interactions.
259-261: Constants align with pagination enhancement goals.The PAGE_SIZE constant (100) is significantly larger than the CMS API's default page size (25 articles per request) mentioned in the PR objectives, which will help reduce the number of API calls needed when fetching all articles. The DEBOUNCE_DELAY helps optimize the search experience by preventing excessive API calls during typing.
apps/cow-fi/services/cms/index.ts
Outdated
| /** | ||
| * Search for articles containing a search term across multiple fields. | ||
| * Uses Strapi's filtering capabilities to perform the search server-side. | ||
| * | ||
| * @param searchTerm The term to search for | ||
| * @param page The page number (0-indexed) | ||
| * @param pageSize The number of articles per page | ||
| * @returns Articles matching the search term with pagination info | ||
| */ | ||
| export async function searchArticles({ | ||
| searchTerm, | ||
| page = 0, | ||
| pageSize = PAGE_SIZE, | ||
| }: { | ||
| searchTerm: string | ||
| page?: number | ||
| pageSize?: number | ||
| }): Promise<ArticleListResponse> { | ||
| // Trim the search term to remove leading and trailing whitespace | ||
| const trimmedSearchTerm = searchTerm.trim() | ||
|
|
||
| if (!trimmedSearchTerm) { | ||
| return { data: [], meta: { pagination: { page, pageSize, pageCount: 0, total: 0 } } } | ||
| } | ||
|
|
||
| try { | ||
| const articlesResponse = await getArticles({ page, pageSize }) | ||
|
|
||
| // Filter articles using the trimmed search term | ||
| const filteredArticles = articlesResponse.data.filter((article) => { | ||
| const attrs = article.attributes | ||
| if (!attrs) return false | ||
|
|
||
| const title = attrs.title?.toLowerCase() || '' | ||
| const description = attrs.description?.toLowerCase() || '' | ||
| const slug = attrs.slug?.toLowerCase() || '' | ||
|
|
||
| const searchTermLower = trimmedSearchTerm.toLowerCase() | ||
|
|
||
| return title.includes(searchTermLower) || description.includes(searchTermLower) || slug.includes(searchTermLower) | ||
| }) | ||
|
|
||
| return { | ||
| data: filteredArticles, | ||
| meta: { | ||
| pagination: { | ||
| page, | ||
| pageSize, | ||
| pageCount: Math.ceil(filteredArticles.length / pageSize), | ||
| total: filteredArticles.length, | ||
| }, | ||
| }, | ||
| } | ||
| } catch (error) { | ||
| console.error('Error in searchArticles:', error) | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Server-side filtering is recommended to handle large datasets.
This method filters in-memory after fetching a full page, which may strain performance if the dataset is large and many pages are fetched. Consider harnessing Strapi’s $containsi or $or filters to narrow results server-side.
| import styled from 'styled-components/macro' | ||
| import { Font, Color, Media } from '@cowprotocol/ui' |
There was a problem hiding this comment.
Fix import order to comply with linting rules.
Adjust the import ordering to follow the project's linting rules:
-import styled from 'styled-components/macro'
-import { Font, Color, Media } from '@cowprotocol/ui'
+import { Font, Color, Media } from '@cowprotocol/ui'
+
+import styled from 'styled-components/macro'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import styled from 'styled-components/macro' | |
| import { Font, Color, Media } from '@cowprotocol/ui' | |
| import { Font, Color, Media } from '@cowprotocol/ui' | |
| import styled from 'styled-components/macro' |
🧰 Tools
🪛 ESLint
[error] 1-1: There should be at least one empty line between import groups
(import/order)
[error] 2-2: @cowprotocol/ui import should occur before import of styled-components/macro
(import/order)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cow-fi/services/cms/index.ts (1)
206-258: Consider removing debug logging statements for production.The function contains numerous debug logging statements that are helpful during development but should be removed or conditioned on a debug flag before deploying to production. This will improve performance and reduce noise in the logs.
- // 1. Log the raw search term - console.log('Searching for:', JSON.stringify(trimmedSearchTerm)) // [...] - // 5. Debug output - console.log('Final query string:', decodeURIComponent(queryString)) - console.log('Full URL:', `/articles?${queryString}`) // [...] - // 6. Response inspection - console.log('API response status:', response.status) - console.log('API response data:', JSON.stringify(data, null, 2)) // [...] - // 7. Result validation - const foundIds = data.data.map((article) => article.id) - console.log('Found article IDs:', foundIds)Consider adding a debug utility instead:
const DEBUG = process.env.NODE_ENV !== 'production'; const debug = (...args: any[]) => { if (DEBUG) console.log(...args); }; // Then use debug() instead of console.log()🧰 Tools
🪛 ESLint
[error] 209-209: 'filters' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cow-fi/services/cms/index.ts(3 hunks)
🧰 Additional context used
🪛 ESLint
apps/cow-fi/services/cms/index.ts
[error] 209-209: 'filters' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Cypress
🔇 Additional comments (4)
apps/cow-fi/services/cms/index.ts (4)
8-10: Well-configured pagination constants.The increased
PAGE_SIZEand newARTICLES_FETCHING_PAGE_LIMITare well-balanced for your use case. With 100 articles per page and a maximum of 10 pages, you can now fetch up to 1000 articles recursively, which should be more than adequate for your current needs (89 articles) and future growth (500+ articles).
121-122: LGTM! Backward-compatible API design.The optional
fetchAllparameter with a default value offalseensures backward compatibility with existing code. This is a clean way to introduce the new functionality.
151-175: Great recursive implementation for comprehensive pagination.Your recursive fetching implementation elegantly handles the CMS pagination limitations, ensuring all articles are accessible for search. The checks for
fetchAll, more pages availability, and the page limit provide robust safeguards against excessive API calls.
189-264: Excellent implementation of server-side filtering.I appreciate that you've implemented server-side filtering using Strapi's query capabilities (
$startsWithi,$containsi). This is much more efficient than client-side filtering, especially as your article count grows. The explicit query structure with array indices ensures proper encoding.🧰 Tools
🪛 ESLint
[error] 209-209: 'filters' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
apps/cow-fi/services/cms/index.ts
Outdated
| const filters = { | ||
| $or: [ | ||
| { title: { $startsWithi: trimmedSearchTerm } }, | ||
| { title: { $containsi: trimmedSearchTerm } }, | ||
| { description: { $containsi: trimmedSearchTerm } }, | ||
| ], | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused variable.
The filters object is defined but never used in the code. Instead, the query parameters are manually constructed in the queryParams object below.
- // 2. Explicit filter structure with indices
- const filters = {
- $or: [
- { title: { $startsWithi: trimmedSearchTerm } },
- { title: { $containsi: trimmedSearchTerm } },
- { description: { $containsi: trimmedSearchTerm } },
- ],
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filters = { | |
| $or: [ | |
| { title: { $startsWithi: trimmedSearchTerm } }, | |
| { title: { $containsi: trimmedSearchTerm } }, | |
| { description: { $containsi: trimmedSearchTerm } }, | |
| ], | |
| } |
🧰 Tools
🪛 ESLint
[error] 209-209: 'filters' is assigned a value but never used. Allowed unused vars must match /^_/u.
(unused-imports/no-unused-vars)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/cow-fi/components/SearchBar/index.tsx (4)
28-71: Consider improving the keyboard navigation logic with more robust event handling.
While the current approach is functional, you might consider factoring out repeated logic (e.g.,ArrowUpvs.ArrowDown) into smaller helpers for clarity. Additionally, verifying that focus stays within the component for assistive technologies would further enhance accessibility.
77-114: Unify navigation approach for consistency and possible performance benefits.
Currently, the code useswindow.location.href(line 99) to navigate, whereas the result links at line 264 rely on HTML<a>tags. Consider using Next.js’useRouteror<Link>for both to ensure consistent navigation, route transitions, and potential client-side performance optimizations.- window.location.href = `/learn/${selectedArticle.attributes?.slug}?ref=learn_search` + import { useRouter } from 'next/router' + ... + const router = useRouter() + router.push(`/learn/${selectedArticle.attributes?.slug}?ref=learn_search`)
115-148: Gracefully handle empty or partial query inputs.
The search logic provides a robust path for error handling, but for partial queries (e.g., a single character), consider offering user guidance or immediate local filtering to improve responsiveness. You might restrict external searches until the query surpasses a certain length threshold (e.g., 2 or 3 characters) to reduce backend load and false positives.
247-298: Nice job with accessibility attributes, but watch for potential complexity.
The use ofrole="listbox"andaria-selectedis appropriate. However, the dynamic addition of a “Load more results” button under the same listbox may confuse some screen readers. You might consider splitting the “Load more results” functionality into a separate element or clearly reflecting its action for screen readers to improve clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cow-fi/components/SearchBar/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Cypress
🔇 Additional comments (3)
apps/cow-fi/components/SearchBar/index.tsx (3)
1-27: Good use of reusable hooks and external libraries.
All imports appear clear and purposeful, and the mixture of custom and third-party hooks is well-structured. The references to@cowprotocol/common-hooksand@cowprotocol/uilook appropriate for the codebase.
74-75: Verify necessity of thearticlesprop.
TheSearchBarPropsinterface requiresarticles, but they don’t appear to be used (beyond the comment referencing initial rendering). If these articles are not intended to be preloaded or used in fallback mode, consider removing or repurposing this prop to keep the component interface minimal.
150-172: Verify edge cases for paginated loading.
WhenPAGE_SIZEis small, repetitive calls toloadMoreResultscould lead to a high number of requests. Ensure the server’s pagination logic efficiently supports frequent incremental fetches. As a follow-up, you could batch additional pages if multiple pages remain.
… search validation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cow-fi/components/SearchBar/index.tsx (1)
41-93: Consider refining Tab navigation for accessibility.Preventing default Tab navigation and manually cycling through items may disrupt expected keyboard behavior, particularly for users relying on tab-based focus movement.
You could consider allowing normal Tab navigation when focus leaves the results region, making sure arrow keys (Up/Down) remain the main navigation mechanism within the search results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cow-fi/components/SearchBar/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (7)
apps/cow-fi/components/SearchBar/index.tsx (7)
145-191: Verify concurrency handling during rapid input changes.Although React transitions handle many concurrency patterns gracefully, confirm that
searchArticlesActionremains consistent if multiple search requests are triggered in quick succession. Ensure results are never overwritten with stale data.
193-215: Efficient handling of “Load more” logic.This approach correctly prevents concurrent loads when
isLoadingMoreis true. It also updates the pagination state carefully.
228-237: Clear search logic looks good.The function cleanly resets all relevant states and re-focuses the input to resume user interaction immediately.
240-248: Confirm design choice for smaller screens.The results close on click outside only for medium screens and above. If this is intentional for mobile usability, no issue; otherwise, consider applying it consistently across all screen sizes.
249-253: Logical check for result visibility.Conditionally showing the results for smaller screens when the user has an active query aligns well with typical mobile UI patterns.
255-351: Commendable ARIA and accessibility attributes.Using appropriate roles, labels, and
aria-selectedfor the results significantly enhances accessibility. Great job!
7-7:✅ Verification successful
Ensure safe HTML output from the highlight function.
If
highlightQueryisn’t escaping potentially unsafe characters, malicious or unexpected HTML could be rendered.To confirm safe usage, please examine the implementation of
highlightQueryfor any possible injection routines or unsafe actions. You may run a script like:
🏁 Script executed:
#!/bin/bash # Description: Search for highlightQuery definition and check for any unsafe usage or dangerouslySetInnerHTML rg 'function highlightQuery|export function highlightQuery|dangerouslySetInnerHTML' -A 5Length of output: 503
Verification Complete: No Issues Found with
highlightQueryHTML EscapingAfter running the verification script, no evidence was found that links the
highlightQueryimplementation to any unsafe HTML output. The only instance ofdangerouslySetInnerHTMLappears in theapps/cow-fi/components/TokenDetails/index.tsxfile, which is separate from theSearchBarcomponent that useshighlightQuery.
- No occurrence of
highlightQueryor unsanitized HTML routines was detected in our grep results.- The isolated use of
dangerouslySetInnerHTMLin another component does not affect the safety ofhighlightQueryas imported inSearchBar.Based on the available evidence, please ensure that
highlightQueryis internally sanitizing input as expected. However, the current verification does not reveal any concerning patterns, so no changes appear to be required.
|
Now the build can be triggered, but it fails. |
Summary
This PR enhances the search functionality in the cow-fi app by implementing a recursive pagination solution that ensures all articles from the CMS are available for search, regardless of the total number of articles.
Previously, the app was only fetching a limited number of articles (200 at most) from the CMS, which meant that as content grows beyond this limit, some articles wouldn't be included in search results. The screenshot shows we currently have 89 articles across 4 pages, but this solution future-proofs our search as content grows to 500+ articles.
To Test
Open the learn page (
/learn)Test search on other pages
/learn/topic/defi)Test with partial words
Background
The CMS API uses pagination to limit the number of articles returned in a single request. The default page size is 25 articles, and we currently have 89 articles total (4 pages).
This solution adds a new
fetchAllparameter to thegetArticlesfunction that recursively fetches all pages of articles until it has the complete set. This ensures that the search functionality always has access to all articles, making it future-proof as content grows.I've updated all relevant pages (
/learn,/learn/articles,/learn/[article], and/learn/topic/[topicSlug]) to use this new parameter, ensuring consistent search functionality throughout the app.Summary by CodeRabbit
New Features
SearchBarcomponent for a responsive and interactive search interface.Style
Bug Fixes
Chores
SearchBarcomponent to streamline the search functionality.