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.

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

Conversation

svenvandescheur
Copy link
Collaborator

No description provided.

@svenvandescheur svenvandescheur merged commit 88035be into main Jun 10, 2024
7 checks passed
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.

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.

@svenvandescheur
Copy link
Collaborator Author

Merged in as considered hotfix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants