-
Notifications
You must be signed in to change notification settings - Fork 339
feat: add protocol search filter to chain overview #2408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe PR adds search and filtering functionality to a table component. New imports include getFilteredRowModel and ColumnFiltersState from React Table. Two state variables—columnFilters and searchQuery—were introduced. A useEffect hook enables debounced filtering on the name column. The table configuration now includes getFilteredRowModel() and onColumnFiltersChange callbacks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/containers/ChainOverview/Table.tsx (2)
358-365: Consider moving column lookup inside the timeout for robustness.The current implementation captures
nameColumnbefore the timeout fires. If the table instance changes during the 200ms delay (e.g., columns reconfigure), the reference could be stale. MovinggetColumninside the callback ensures you always get the current column.♻️ Suggested improvement
useEffect(() => { - const nameColumn = instance.getColumn('name') const id = setTimeout(() => { + const nameColumn = instance.getColumn('name') nameColumn?.setFilterValue(searchQuery) }, 200) return () => clearTimeout(id) }, [searchQuery, instance])
439-455: Good implementation; considertype="search"for better semantics.The search input is accessible with proper labeling. Consider adding
type="search"for semantic HTML, which also enables native browser clear buttons in some browsers.♻️ Optional enhancement
<input + type="search" value={searchQuery} onChange={(e) => { setSearchQuery(e.target.value) }} placeholder="Search protocols..." className="w-full rounded-md border border-(--form-control-border) bg-white p-1 pl-7 text-black max-sm:py-0.5 dark:bg-black dark:text-white" />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/containers/ChainOverview/Table.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/containers/ChainOverview/Table.tsx (1)
src/components/Icon.tsx (1)
Icon(108-114)
🔇 Additional comments (3)
src/containers/ChainOverview/Table.tsx (3)
1-16: LGTM!The imports are correctly added to support the new filtering functionality.
getFilteredRowModelandColumnFiltersStateare the standard TanStack Table APIs for client-side filtering.
214-215: LGTM!Good separation of concerns:
searchQueryfor immediate UI feedback,columnFiltersfor debounced table filtering.
316-356: LGTM!The TanStack Table filtering is correctly configured with
columnFiltersstate,onColumnFiltersChangecallback, andgetFilteredRowModel(). The existingfilterFromLeafRows: trueensures child protocols are also searchable.
What
Why
Screenshots
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.