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

Custom Filter doesn't accept filterList (needed for serverside) #737

Closed
Avd6977 opened this issue Jul 2, 2019 · 7 comments
Closed

Custom Filter doesn't accept filterList (needed for serverside) #737

Avd6977 opened this issue Jul 2, 2019 · 7 comments
Labels
needs verification For issues that can't be reproduced or are otherwise unclear

Comments

@Avd6977
Copy link
Contributor

Avd6977 commented Jul 2, 2019

Expected Behavior

Passing in filterList with the column will default the filter to a value. For server-side, this is how to keep the filter between loads.

Current Behavior

deepClone clears out the array when it has string keys

Steps to Reproduce (for bugs)

const customFilterList = [];
customFilterList["min"] = "20"
Pass customFilterList in as the column's filterList and see that it isn't loaded.

Your Environment

Tech Version
MUI-datatables 2.6.0
Avd6977 pushed a commit to Avd6977/mui-datatables that referenced this issue Jul 2, 2019
@Avd6977 Avd6977 mentioned this issue Jul 2, 2019
@gabrielliwerant
Copy link
Collaborator

Hm, I'm a little confused by this one. Maybe a more fleshed out example would help. This doesn't look like supported behavior. If you want to keep the filterList between loads, you should be able to update it as part of state change on your end. I don't see the need to pass in an object.

@gabrielliwerant gabrielliwerant added the needs verification For issues that can't be reproduced or are otherwise unclear label Jul 3, 2019
@Avd6977
Copy link
Contributor Author

Avd6977 commented Jul 3, 2019

I think saying it was needed for just serverside is confusing. If you take the change to the custom-filter example I made and revert the Object.assign, then it doesn't use the default filter because deepClone doesn't clone it to the new array.

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Jul 3, 2019

I think what I'm saying is that if your problem is maintaining filter state between serverside updates, I don't believe you need to do this the way you're doing it. I'm pretty sure there's a way to do this with existing functionality, and I'd be happy to demonstrate it to you, but I might need you to give a more in-depth example of what you're trying to accomplish first.

In general, the direction the table is moving is for you to manage more of the state yourself rather than rely on the internal derived state of the table, so I'm reluctant to go the opposite direction.

@Avd6977
Copy link
Contributor Author

Avd6977 commented Jul 3, 2019

From what I'm seeing happen though, the chip is disappearing despite maintaining the state on our end. Have you looked at the PR? The main issue is that deepClone has known issues (especially with stringed-key arrays) so the fix is just cloning with the Object.assign method.

Unless I'm missing what you're meaning by being able to do this already. I'm passing in the filterList prop with the column. Is there a better way to maintain the filter state and pass it down so the Chips appear still?

@gabrielliwerant
Copy link
Collaborator

I'd like to verify that the issue is blocking people from being able to manage filter state, and I don't yet have that verification because I don't think it's being done in the recommended way.

const customFilterList = [];
customFilterList["min"] = "20"
Pass customFilterList in as the column's filterList and see that it isn't loaded.

This treats an array (customFilterList) like an object, presumably because filterList is an array, but you're looking for object-like behavior. That's not something I plan to support. If necessary, we would change filterList to an object instead, but again, I'm 95% certain that this just isn't the right way to handle filter state in this library. If you like, I can show you how to play by the rules :), but I'd need to see your code.

@Avd6977
Copy link
Contributor Author

Avd6977 commented Jul 3, 2019

I mean....I don't think it's against the ruuuules for a string named array.... :) but fine. I just fixed the example to show how it would work.

gabrielliwerant pushed a commit that referenced this issue Aug 1, 2019
* Custom Filter doesn't accept filterList (needed for serverside) #737

* Fixing the example
@gabrielliwerant
Copy link
Collaborator

Should be fixed now in 2.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs verification For issues that can't be reproduced or are otherwise unclear
Projects
None yet
Development

No branches or pull requests

2 participants