Skip to content
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.

Sign up for GitHub

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

♻️ - refactor: move filterTransform to DataGrid as prop #83

Merged
merged 1 commit into from
Jun 10, 2024
Merged

♻️ - refactor: move filterTransform to DataGrid as prop #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/components/data/datagrid/datagrid.stories.tsx
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe good to have a story where multiple fields need to be transformed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in general we better stories are useful. TBH I kinda rushed this in to fix our broken PR but better documentation is a good idea.

Original file line number Diff line number Diff line change
Expand Up @@ -416,17 +416,17 @@ export const DateRangeFilter: Story = {
type: "string",
},
{
filterTransform: (data) => {
const { dateOfBirth, _data } = data;
const [dateOfBirth__gte = "", dateOfBirth__lte = ""] =
String(dateOfBirth).split("/");
return { dateOfBirth__gte, dateOfBirth__lte, ..._data };
},
name: "dateOfBirth",
type: "daterange",
},
],
filterable: true,
filterTransform: (data) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering: if I understand correctly, now we will have one function that has to handle the filters transform for all the fields. If that's the case, it could be harder to maintain than specifying the filters transform on the fields themselves 🤔

Maybe the filter transforms could still be specified per field and then when they need to be applied we could iterate over the fields that are active and pass to a filter the output of the previous one?

Also, how does this behave in the case of selectable columns when a particular field is disabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering: if I understand correctly, now we will have one function that has to handle the filters transform for all the fields. If that's the case, it could be harder to maintain than specifying the filters transform on the fields themselves 🤔

Yes, as I there might be (rare) cases where another filter's value needs to be used as well.

Maybe the filter transforms could still be specified per field and then when they need to be applied we could iterate over the fields that are active and pass to a filter the output of the previous one?

Wouldn't this mean that the order the fields is affect the outcome? The order which is used for presentational purposes (and in the future maybe dynamic?

Also, how does this behave in the case of selectable columns when a particular field is disabled?

I'm not sure what you mean here but I think this is up to the user.

const { dateOfBirth, ..._data } = data;
const [dateOfBirth__gte = "", dateOfBirth__lte = ""] =
String(dateOfBirth).split("/");
return { dateOfBirth__gte, dateOfBirth__lte, ..._data };
},
objectList: [
{
firstName: "Albert",
Expand Down
21 changes: 15 additions & 6 deletions src/components/data/datagrid/datagrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ export type DataGridProps = {
*/
filterable?: boolean;

/**
* A function transforming the filter values.
* This can be used to adjust filter input to an API spec.
*/
filterTransform?: (value: AttributeData) => AttributeData;

/** Whether to allow sorting/the field to sort on. */
sort?: boolean | string;

Expand Down Expand Up @@ -195,6 +201,7 @@ export const DataGrid: React.FC<DataGridProps> = ({
editable = undefined,
fields,
filterable = undefined,
filterTransform,
paginatorProps,
showPaginator = Boolean(paginatorProps),
pProps,
Expand Down Expand Up @@ -401,6 +408,7 @@ export const DataGrid: React.FC<DataGridProps> = ({
<DataGridHeading
dataGridId={id}
filterable={Boolean(filterable)}
filterTransform={filterTransform}
id={id}
labelFilterField={labelFilterField || ""}
renderableFields={renderableFields}
Expand Down Expand Up @@ -648,6 +656,7 @@ export type DataGridHeadingProps = {
selectable: boolean;
dataGridId: string;
filterable: boolean;
filterTransform?: (value: AttributeData) => AttributeData;
id: string;
labelFilterField: string;
renderableFields: TypedField[];
Expand All @@ -664,6 +673,7 @@ export type DataGridHeadingProps = {
export const DataGridHeading: React.FC<DataGridHeadingProps> = ({
dataGridId,
filterable,
filterTransform,
id,
labelFilterField,
onFilter,
Expand Down Expand Up @@ -711,8 +721,7 @@ export const DataGridHeading: React.FC<DataGridHeadingProps> = ({
const placeholder = field2Caption(field.name);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { options, filterTransform, valueTransform, ...context } =
field;
const { options, valueTransform, ...context } = field;

const _labelFilterField = labelFilterField
? formatMessage(labelFilterField, context)
Expand Down Expand Up @@ -755,11 +764,11 @@ export const DataGridHeading: React.FC<DataGridHeadingProps> = ({
e.preventDefault();
const data = serializeForm(
e.target.form as HTMLFormElement,
);
const _data = field.filterTransform
? field.filterTransform(data as AttributeData)
) as AttributeData;
const _data = filterTransform
? filterTransform(data)
: data;
onFilter(_data as AttributeData);
onFilter(_data);
}}
/>
)}
Expand Down
6 changes: 0 additions & 6 deletions src/lib/data/attributedata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,6 @@ export type TypedField<T = Attribute> = {
/** The value for this field's filter. */
filterValue?: string | number;

/**
* A function transforming the filter values.
* This can be used to adjust filter input to an API spec.
*/
filterTransform?: (value: AttributeData) => AttributeData;

/**
* The "lookup" (dot separated) to use for this field while filtering (e.g.
* "._expand.zaaktype.omschrijving").
Expand Down