Introduce useColumnFilter #1074
Conversation
|
Hi @jassmith, I'm still not sure—do you guys accept PRs? Sorry for just dropping it here. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new useColumnFilter hook to support filtering data grid columns, similar to the existing useColumnSort implementation. The hook provides a way to filter rows based on custom filter callbacks applied to specific columns.
- Adds
useColumnFilterhook with support for single or multiple column filters - Includes comprehensive test coverage for multi-column filtering scenarios
- Provides a Storybook example demonstrating the hook's usage with a simple number input filter
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/source/src/use-column-filter.ts | Core hook implementation with filtering logic and row mapping |
| packages/source/test/use-column-filter.test.tsx | Test coverage for multi-column filtering functionality |
| packages/source/src/stories/use-data-source.stories.tsx | Storybook example with filter input and integration |
| packages/source/src/index.ts | Export statement for the new hook |
|
|
||
| export type ColumnFilter = { | ||
| column: GridColumn; | ||
| filterCallback: (cellValue: any) => boolean; |
There was a problem hiding this comment.
The cellValue parameter should be typed as GridCell instead of any since the hook passes the entire cell object (line 53) to the filter callback, not just the cell's data value.
| filterCallback: (cellValue: any) => boolean; | |
| filterCallback: (cellValue: GridCell) => boolean; |
Copilot uses AI. Check for mistakes.
| for (let sIndex = 0; sIndex < activeFilters.length; sIndex++) { | ||
| const { filter: colFilter } = activeFilters[sIndex]; | ||
| const v = values[sIndex][r]; | ||
| if (!colFilter.filterCallback(v)) { |
There was a problem hiding this comment.
The filter callback receives the entire GridCell object v, but the type definition suggests it should receive cellValue: any. This inconsistency could cause runtime errors if consumers expect just the cell data rather than the full cell object.
| if (!colFilter.filterCallback(v)) { | |
| if (!colFilter.filterCallback(v.data)) { |
Copilot uses AI. Check for mistakes.
| const [filterValue, setFilterValue] = React.useState<number | undefined>(undefined); | ||
|
|
||
| const filterHandler = (value: number) => { | ||
| return filterValue > value; |
There was a problem hiding this comment.
The filter callback expects a GridCell object but treats the parameter as a number. This will cause a runtime error since filterValue > value will be comparing a number to a GridCell object. Should be return filterValue > Number(value.data).
| return filterValue > value; | |
| const filterHandler = (value) => { | |
| return filterValue > Number(value.data); |
Copilot uses AI. Check for mistakes.
| getCellContent, | ||
| filter: [ | ||
| { column: columns[0], filterCallback: (v) => v !== "2" }, | ||
| { column: columns[1], filterCallback: (v) => v !== "b" }, |
There was a problem hiding this comment.
The test filter callback expects a string but will receive a GridCell object. This should be (v) => v.data !== "2" to access the cell's data property.
| { column: columns[1], filterCallback: (v) => v !== "b" }, | |
| { column: columns[0], filterCallback: (v) => v.data !== "2" }, | |
| { column: columns[1], filterCallback: (v) => v.data !== "b" }, |
Copilot uses AI. Check for mistakes.
| getCellContent, | ||
| filter: [ | ||
| { column: columns[0], filterCallback: (v) => v !== "2" }, | ||
| { column: columns[1], filterCallback: (v) => v !== "b" }, |
There was a problem hiding this comment.
The test filter callback expects a string but will receive a GridCell object. This should be (v) => v.data !== "b" to access the cell's data property.
| { column: columns[1], filterCallback: (v) => v !== "b" }, | |
| { column: columns[1], filterCallback: (v) => v.data !== "b" }, |
Copilot uses AI. Check for mistakes.
Hi!
Introduced
useColumnFilterto support filtering similarly asuseColumnSortis implemented.I have also created a Storybook example demonstrating how to use
useColumnFilter.Currently, I am not 100% sure how to implement a filter menu. I can only see menu props being available which simple icon that can triggered. Ideally, I would like to add a UI component to work alongside useColumnFilter.
Any suggestions on how I could implement this?
Also, if the hook itself is valuable to merge without the ui component, I can work on the filter UI component in a separate PR.