feat: add the search bar in courts page #1821
Conversation
WalkthroughThe pull request enhances the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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 (
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![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 (3)
web/src/pages/Courts/TopSearch.tsx (3)
40-48: Accessibility consideration for search input
Consider adding anaria-labelor descriptive label text to the<StyledSearchbar>to improve accessibility for screen readers, especially since the placeholder text alone may not be sufficient.<StyledSearchbar - dir="auto" + aria-label="Search through courts" dir="auto" ... />
70-72: Ensure robust safety checks in flattenCourts
flattenCourtsis straightforward and concise. However, if future adjustments allow unexpected data shapes (e.g., missingchildrenfields), explicitly checking for array-like structures before usingflatMapmight help prevent runtime errors.function flattenCourts(court) { - return court ? [court, ...(court.children || []).flatMap(flattenCourts)] : []; + if (!court) return []; + const childArray = Array.isArray(court.children) ? court.children : []; + return [court, ...childArray.flatMap(flattenCourts)]; }
79-84: Efficient filtering approach
UsingtoLowerCase()to filter courts is fine for simple matching. If you intend to scale this further with more advanced searching (fuzziness, synonyms), consider a library or move logic to a dedicated utility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Courts/TopSearch.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Copy labels from linked issues
- GitHub Check: contracts-testing
- GitHub Check: Copy labels from linked issues
- GitHub Check: Analyze (javascript)
- GitHub Check: dependency-review
- GitHub Check: SonarCloud
🔇 Additional comments (6)
web/src/pages/Courts/TopSearch.tsx (6)
1-4: Well-structured imports and usage of React hooks
The consolidated import statements and the addition of React hooks (useState,useMemo) look consistent and streamlined. Keep it up!
Line range hint
8-19: Suggest verifying CSS fallback
Thegapandflex-wrapstyling are good improvements. However, for older browsers that may not support gap in flex containers, consider verifying if a fallback is needed, or confirm that your targeted browsers fully support this property.
29-39: Praise for component organization
Wrapping the search bar in a dedicated container (SearchBarContainer) keeps the layout modular and easy to maintain. Nicely done!
50-60: Check on overflow styling
The usage ofoverflow-y: autoinSearchResultsContaineris appropriate, but be mindful of horizontally long court names. If that scenario occurs, you may want to limit horizontal overflow or add text truncation for better readability.
62-69: Consistent styling and clickable cards
TheStyledCardeffectively indicates hover states and clickable functionality. This is a clear, intuitive user experience.
89-120: Clear UI flow with conditional rendering
The conditional rendering that shows the search results only when needed is user-friendly. Ensuring that the dropdown and the search bar both appear in the UI simultaneously provides flexibility for different user preferences. Nice work!
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
web/src/pages/Courts/TopSearch.tsx (4)
46-54: Review hardcoded values in StyledSearchbar.Consider replacing hardcoded values with theme variables or responsive units:
- font-size: 16px
- height: 45px
const StyledSearchbar = styled(Searchbar)` width: 100%; input { - font-size: 16px; - height: 45px; + font-size: ${({ theme }) => theme.fontSize.regular}; + height: ${responsiveSize(45, 50)}; padding-top: 0px; padding-bottom: 0px; } `;
56-66: Coordinate z-index usage with other components.The
SearchResultsContainerusesz-index: 1. Consider:
- Documenting z-index layers in a shared constants file
- Ensuring this value doesn't conflict with other overlapping elements
76-78: Add TypeScript types and consider memoization for flattenCourts.The recursive implementation is clean, but could benefit from:
- Type safety
- Memoization for large court hierarchies
- function flattenCourts(court) { + interface Court { + id: string; + name: string; + children?: Court[]; + } + + function flattenCourts(court: Court | undefined): Court[] { return court ? [court, ...(court.children || []).flatMap(flattenCourts)] : []; }
85-90: Consider debouncing search input for performance.For better performance with large datasets, consider debouncing the search input:
+ import { useDebounce } from 'use-debounce'; const TopSearch: React.FC = () => { const [search, setSearch] = useState(""); + const [debouncedSearch] = useDebounce(search, 300); const filteredCourts = useMemo( () => data?.court ? flattenCourts(data.court).filter((c) => c.name.toLowerCase().includes(search.toLowerCase())) : [], - [data, search] + [data, debouncedSearch] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Courts/TopSearch.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Copy labels from linked issues
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
- GitHub Check: dependency-review
🔇 Additional comments (2)
web/src/pages/Courts/TopSearch.tsx (2)
1-14: LGTM! Import statements are well-organized.The imports are logically grouped and all dependencies are properly utilized in the implementation.
35-44: Consider responsive design implications.The
SearchBarContainerusesflex-wrapwhich is good for responsiveness, but ensure the absolute positioning of its childSearchResultsContainerworks well on all screen sizes.
There was a problem hiding this comment.
Functionality wise looks good, but I would modify the dropdown, adding the custom scrollbar instead of the default one, and using another kind of card maybe 🤔 also, maybe we could show the parent court (entire path might be too much)? I can see a situation where there are 2 courts with the same name, but different parent courts.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (3)
93-100: Consider memoizing the flattenCourts function.The
flattenCourtsfunction is called within auseMemohook but isn't memoized itself. Since it performs recursive operations, memoizing it could improve performance for large court hierarchies.-function flattenCourts(court, parent = null) { +const flattenCourts = React.useCallback((court, parent = null) => { const current = { ...court, parentName: parent?.name ?? null, }; const children = (court.children || []).flatMap((child) => flattenCourts(child, current)); return [current, ...children]; -} +}, []);
110-117: Optimize search filtering performance.The current implementation filters courts on every keystroke. Consider debouncing the search input to reduce unnecessary computations.
+import { useDebounce } from 'react-use'; const TopSearch: React.FC = () => { const [search, setSearch] = useState(""); + const debouncedSearch = useDebounce(search, 300); const filteredCourts = useMemo(() => { if (!data?.court) return []; - const courts = flattenCourts(data.court).filter((c) => c.name.toLowerCase().includes(search.toLowerCase())); + const courts = flattenCourts(data.court).filter((c) => c.name.toLowerCase().includes(debouncedSearch.toLowerCase())); const selectedCourt = courts.find((c) => c.id === currentCourtId); if (!selectedCourt) return courts; return [selectedCourt, ...courts.filter((c) => c.id !== currentCourtId)]; - }, [data, search, currentCourtId]); + }, [data, debouncedSearch, currentCourtId]);
128-135: Add aria-label for better accessibility.The search input should have an aria-label for screen readers.
<StyledSearchbar dir="auto" type="text" placeholder="Search" + aria-label="Search courts" value={search} onChange={(e) => setSearch(e.target.value)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/components/Verdict/Answer.tsx(2 hunks)web/src/components/Verdict/FinalDecision.tsx(1 hunks)web/src/pages/Courts/CourtDetails/TopSearch.tsx(1 hunks)web/src/pages/Courts/CourtDetails/index.tsx(2 hunks)web/src/pages/Courts/index.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/Courts/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Copy labels from linked issues
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: dependency-review
- GitHub Check: contracts-testing
- GitHub Check: Mend Security Check
🔇 Additional comments (3)
web/src/components/Verdict/Answer.tsx (1)
5-5: LGTM! Component structure simplified.The changes effectively simplify the component by removing unnecessary wrappers and using
AnswerTitleAndDescriptiondirectly, which improves code maintainability.Also applies to: 20-22
web/src/pages/Courts/CourtDetails/index.tsx (1)
28-28: LGTM! TopSearch component integrated.The TopSearch component is properly integrated into the CourtDetails page, enhancing the user experience with search functionality.
Also applies to: 117-117
web/src/components/Verdict/FinalDecision.tsx (1)
38-44: LGTM! Simplified flex layout.The changes improve the flex layout by:
- Removing unnecessary flex-direction property
- Moving flex-wrap to a new line for better readability
- Removing redundant flex property
|
Code Climate has analyzed commit 605b082 and detected 5 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (1)
139-155: 🛠️ Refactor suggestionAdd keyboard navigation for search results.
The search results can only be navigated using a mouse. Consider adding keyboard navigation for better accessibility.
🧹 Nitpick comments (3)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (3)
60-71: Consider z-index management.The SearchResultsContainer uses a hardcoded z-index value. Consider using a z-index management system to prevent potential stacking context issues.
+const Z_INDICES = { + searchResults: 100, + // ... other z-indices +}; const SearchResultsContainer = styled(OverlayScrollbarsComponent)` position: absolute; margin-top: 45px; max-height: 400px; border: 1px solid ${({ theme }) => theme.stroke}; width: 100%; flex-direction: column; border-radius: 4px; overflow-y: auto; - z-index: 1; + z-index: ${Z_INDICES.searchResults}; background-color: ${({ theme }) => theme.whiteBackground}; `;
96-104: Add TypeScript types for better type safety.The flattenCourts function would benefit from explicit TypeScript interfaces.
+interface Court { + id: string; + name: string; + children?: Court[]; +} + +interface FlattenedCourt extends Court { + parentName: string | null; +} -function flattenCourts(court, parent = null) { +function flattenCourts(court: Court, parent: Court | null = null): FlattenedCourt[] { const current = { ...court, parentName: parent?.name ?? null, }; const children = (court.children || []).flatMap((child) => flattenCourts(child, current)); return [current, ...children]; }
111-120: Optimize search performance with debouncing.The search filtering runs on every keystroke. Consider adding debounce to improve performance, especially with large datasets.
+import { useMemo, useState, useCallback } from "react"; +import debounce from "lodash/debounce"; const TopSearch: React.FC = () => { // ... existing code ... const [search, setSearch] = useState(""); + const [debouncedSearch, setDebouncedSearch] = useState(""); + + const debouncedSetSearch = useCallback( + debounce((value: string) => { + setDebouncedSearch(value); + }, 300), + [] + ); const filteredCourts = useMemo(() => { if (!data?.court) return []; - const courts = flattenCourts(data.court).filter((c) => c.name.toLowerCase().includes(search.toLowerCase())); + const courts = flattenCourts(data.court).filter((c) => + c.name.toLowerCase().includes(debouncedSearch.toLowerCase()) + ); const selectedCourt = courts.find((c) => c.id === currentCourtId); if (!selectedCourt) return courts; return [selectedCourt, ...courts.filter((c) => c.id !== currentCourtId)]; - }, [data, search, currentCourtId]); + }, [data, debouncedSearch, currentCourtId]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Copy labels from linked issues
- GitHub Check: SonarCloud
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
🔇 Additional comments (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (1)
1-22: LGTM! Well-organized imports.The imports are logically grouped and all dependencies are necessary for the component's functionality.
PR-Codex overview
This PR primarily introduces a new component,
TopSearch, to theCourtDetailspage and removes its previous usage in theCourtspage. It also includes various style updates and refactors in theAnswercomponent.Detailed summary
TopSearchfromCourtspage.TopSearchcomponent inCourtDetailspage.Answercomponent to remove unnecessary styled components.TopSearchwith various UI elements likeDropdownCascaderandSearchbar.TopSearch.Summary by CodeRabbit
New Features
TopSearchcomponent for searching and selecting courts.Improvements
AnswerDisplaycomponent for a more streamlined user experience.JuryContainerfor better flexibility.Bug Fixes
TopSearchcomponent from theCourtspage to prevent UI conflicts.