feat: filter live services from drop-down menu in admin dashboard #4344
Conversation
| } | ||
| return getDoubleFilterFn(filterItem); | ||
| }, | ||
| InputComponent: filteringOne ? SingleInputFilter : DoubleInputFilter, |
There was a problem hiding this comment.
Ideally, it seems like this would/should be select-one or select-many (rather than select-two). Out of curiousity, have you tried passing in our existing SelectMultiple component here?
| @@ -38,7 +38,7 @@ export const mockTeams: AdminPanelData[] = [ | |||
| liveFlows: [ | |||
| "Notify completion of planning application", | |||
| "Find out if you need planning permission", | |||
| "Apply for prior approval", | |||
| "apply for prior approval ", | |||
There was a problem hiding this comment.
To test the whitespace and casing processes
| getRowHeight={() => "auto"} | ||
| getRowClassName={(params) => | ||
| params.indexRelativeToCurrentPage % 2 === 0 ? "even" : "odd" | ||
| } | ||
| slots={{ | ||
| toolbar: GridToolbar, |
There was a problem hiding this comment.
Adds the additional toolbar at the top of the table
There was a problem hiding this comment.
Decided to make a separate component from the SelectMultiple since a lot of the props were superfluous, and the styling a bit different!
There was a problem hiding this comment.
Some of these moved across from DataTable.tsx
There was a problem hiding this comment.
Took a quick pass through the code - only a few very minor comments, otherwise looking great !
Happy to do final review & approval today once pizza is up 👍
Agree that if this is "working" now, then best way to test current re-useability-ness and spot any further prop tweaking is to just implement it again soon for another case 🙂
| } | ||
| : { | ||
| ...baseColDef, | ||
| width: column.width || baseColDef.width, |
There was a problem hiding this comment.
nit: it looks like width could be part of shared baseColDef? Otherwise this feels clear considering level of complexity ➕
|
|
||
| const containsItem = (item: string, value: Pick<GridFilterItem, "value">) => { | ||
| if (typeof value === "string") { | ||
| return item.toLowerCase().includes((value as string).toLowerCase()); |
There was a problem hiding this comment.
nit: do we actually need (value as string) casting still if we're within this conditional if (typeof value === "string") block?
No worries if so, understand MUI types can be fiddly sometimes - but this looks like might be leftover from another iteration?
| options: columnValueOptions, | ||
| }, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Super clear, easy to revisit 💯
| ))} | ||
| </Box> | ||
| ), | ||
| }; |
There was a problem hiding this comment.
nit: took me a minute to unpack what componentRegistry meant here - is something like cellValueRegistry or valueOptionsRegistry more specific maybe?
In this PR
utilsfilePlease thoroughly test the filtering of all the columns and check it behaves as expected (note the Article 4s column is an outstanding issue to come back to).
Mild concerns
There are a few types I need to come back to - substituted a lot ofanys while I was hacking/prototyping.I think I've got a bit too down in the weeds with this to properly see if it remains readable/easy to use the generic component. Hopefully fresh eyes/time will help, and implementing it in the next few features!