gregnb / mui-datatables

Datatables for React using Material-UI
MIT License
2.7k stars 931 forks source link

onFilterChange displays data with no chip while making a server side filtering #721

Open MahmoudSoliman922 opened 5 years ago

MahmoudSoliman922 commented 5 years ago

When using onFilterChange to make a request to the server side the chip don't got displayed as well as the filter dropdown box loses the filter option.

Expected Behavior

If you want to reproduce the issue just try to create a server side filtration.

the expected behavior is that the chip as well as the value inside the dropdown box to remain the same as with no server side request.

Current Behavior

The app is working with redux so the table data comes from redux store, Pagination and filtering should take place in the server side so I'm dispatching an action in the onFilterChange function that replaces the data with the filtered data and it works but the chip that represents the filtered data as well as the place holders don't show anything.

Steps to Reproduce (for bugs)

1.set onFilterChange to make a request

  1. whether you're working with redux or not just update the state with the new data
  2. execute filter
  3. check for the placeholder and the filter chip

Your Environment

Tech Version
Material-UI 3.9.2
MUI-datatables 2.5.1
React 16.8.2
browser Chrome
etc
sadika9 commented 5 years ago

This is not limited to filtering. Sorting also not working when using with remote data.

Click column to sort. fetch new data on onTableChange sorting arrow does not change (along with the sort order/column)

patorjk commented 5 years ago

I can also confirm that the sorting arrows are not working for remote data. I'm not using filters yet so can't confirm the original posting.

gabrielliwerant commented 5 years ago

I'm afraid I'm going to need a specific example of this not working, as I played around with the serverside examples and couldn't verify this. Remember to use the correct version of material ui (3.5.1).

patorjk commented 5 years ago

I can't speak for the other two, but after investigating further I found it to be a bug on my end. The sort arrows now work correctly in my table that uses remote data.

I was reading the sort information from the onTableChange function and passing it to my data fetch routine. However, I wasn't updating the sort information in my reducer with the new value, so when I re-rendered the table, I was passing in props that had stale information for the sort direction.

MahmoudSoliman922 commented 5 years ago

@gabrielliwerant It seems that the issue is on my side as I've created a sample on sandbox and it worked, I don't need the sorting feature but it didn't work with me maybe It's my mistake as well, Anyway I'm leaving this url as a verification that it's working and also If you want to take a look at the sorting part.

@patorjk If you can check what's wrong with the sorting part in this sample it'll be perfect.

https://codesandbox.io/s/charming-wood-fus31?fontsize=14

I'll leave my aws lambda function working as well so feel free to use this sample.

Sorry for wasting your time and Thank you for your fast replay.

patorjk commented 5 years ago

For sorting to work with serverSide data you need to handle the sorting in the "onTableChange" function. Basically you'd add a case like this:

case 'sort':
    console.log(action, tableState);
    // code to re-fetch data from server in sorted order using info in "tableState"
    break;

And then you'd fetch the data in sorted order from the server. It has to be done on the server side because the server could have 1000's of items for the table, and the table wouldn't be able to accurately sort the data with just 1 page worth of data.

MahmoudSoliman922 commented 5 years ago

@gabrielliwerant I fount that issue on my side while using version 2.5.1 (mui-datatables version), I've updated the version in the sample, versions till 2.5.0 don't have this issue

gabrielliwerant commented 5 years ago

Hm, interesting, I can't find a version of sorting that works with serverside: true even as far back as the beta version of this library. @patorjk's answer should work, but I also think sorting should work with serverside: true by default, as pagination will still work with whatever the table has available. So I think the sorting problem deserves its own issue, if someone wants to create that.

As far as the original issue with the filtering and the chip, @MahmoudSoliman922 you did not waste time at all, this looks like a legitimate bug to me, and you've helped me narrow it down to the point where I know where the code changes are that are responsible, so thank you for that!

gabrielliwerant commented 5 years ago

@MahmoudSoliman922 It seems I may have spoken a little too soon. I looked into it more deeply, and this isn't a bug, per say, though it is changed behavior in 2.5.1.

What was happening before was that the table was making an assumption on update, that if internal column state existed (from the perspective of mui datatables), column state took precedence over column props. The change in 2.5.1 prevents that assumption from taking place, instead favoring changes to props (again from the perspective of mui datatable) for columns. The reason this change was made is because doing it the other way also caused issues, but they were not as easily fixable by anyone using the table. The longer description of this issue is cataloged here: https://github.com/gregnb/mui-datatables/issues/679.

The way that it works now, if you are making changes to the table state, your code must take responsibility for all the state you wish to preserve/change. I've forked your example to show what I mean: https://codesandbox.io/s/nameless-tree-swgp7?fontsize=14. You'll now notice that the column data is managed in your example in such a way that you can now update/maintain the filter changes.

MahmoudSoliman922 commented 5 years ago

@gabrielliwerant yes now it works perfectly, and I'm not sure why but sorting also works as well. I suggest adding this to the documentation, it'll be super helpful. Thank you for your time and effort.