-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fides table column utilities #4569
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Passing run #6085 ↗︎
Details:
Review all test suite changes for PR #4569 ↗︎ |
? flexRender(cell.column.columnDef.cell, cell.getContext()) | ||
? flexRender(cell.column.columnDef.cell, { | ||
...cell.getContext(), | ||
isDisplayAll, |
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.
this is a way to pass a prop from the table environment to the cell. most of the time we can pass props during the column definition, but in this case where we want a column to toggle between displaying all and grouping all, we don't know that value at definition time. so we need to pass a prop at render time instead (taken from TanStack/table#4391 (reply in thread))
@@ -159,57 +159,39 @@ export const ConsentManagementTable = () => { | |||
id: "name", | |||
cell: (props) => <DefaultCell value={props.getValue()} />, | |||
header: (props) => <DefaultHeaderCell value="Vendor" {...props} />, | |||
meta: { | |||
maxWidth: "350px", | |||
}, |
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.
these widths don't work too well when column resizing is enabled
@@ -218,7 +212,13 @@ export const AddMultipleSystems = ({ redirectRoute }: Props) => { | |||
pagination: { | |||
pageSize: PAGE_SIZES[0], | |||
}, | |||
columnSizing: { | |||
select: 0, |
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.
this allows these two columns to start off initially small (it doesn't start at 0, and admittedly I don't quite know how this works, but 0 seems to work ok)
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.
Thank you for the quick turnaround, this looks amazing! Everything works as expected and your implementation looks solid (as far as a backend engineer like myself can tell 😄). It makes sense how we would add the expand and collapse feature to other tables so this looks good to me. Let's give @mfbrown a chance to provide some more UAT before we merge.
Closes https://ethyca.atlassian.net/browse/PROD-1505
Description Of Changes
Screen.Recording.2024-01-24.at.3.43.42.PM.mov
Code Changes
FidesTableV2
FidesTableV2
cells that use theGroupCountBadgeCell
componentSteps to Confirm
/add-systems/multiple
,/reporting/datamap
, and/consent/configure
by resizing the columnsPre-Merge Checklist
CHANGELOG.md