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

Changing state with setState in onRowExpand collapses the expanded row #989

Open
keybraker opened this issue Oct 8, 2019 · 10 comments
Open
Assignees
Labels
bug v4 state/props This issue is related to state and props attempting to control the same values

Comments

@keybraker
Copy link

All three versions try to showcase a general problem in the functionality of expandable rows. I was trying to make the program call for a get request from server and update the state of the data in the current expandable row. But although fetching the data correctly and storing it, and being displayed on screen for a second the setState itself was the cause that collapsed the expanded row.

As shown bellow three different ways that a setState is being called cause the same problem to occur.

[1]
onTableChange: (action, tableState) => { switch (action) { case 'expandRow': this.setState({age: 1}) break; default: break; } }

[2]
onRowsExpand: (curExpanded, allExpanded) => { this.setState({age: 1}) }

[3]
onRowClick: (rowData, rowState) => { this.setState({age: 1}) }

@gabrielliwerant gabrielliwerant self-assigned this Oct 8, 2019
@gabrielliwerant
Copy link
Collaborator

Hey @keybraker, thanks for reporting. It's a bit of a deep issue, as it's related to the table being a bit confused about when an update has occurred vs initializing state from props. I think I have a workable solution, but the tricky bit is making sure it doesn't break anything else, as it requires changing some internals. I'll link here when I have a PR and I'd be happy to have you take a look through it if you want.

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Oct 9, 2019

Also, note that although there is a bug here regarding generic component updates resetting internal state for expanded rows (I also think filters and selected rows might be affected), row selecting and row expanding is not async behavior currently, so you can't cause them to wait for your actions before they will trigger internal state changes (though you can use promises to change props once it resolves).

@keybraker
Copy link
Author

I am using promises, from the example on how to make GET requests, but the problem still remains, as it updates the data of the expandable row but does so by refreshing the whole data table.

@gabrielliwerant
Copy link
Collaborator

Yes, the problem still exists as no fix has been pushed, my note about promises was just to explain that you have two issues:

  1. Changing state is refreshing the table
  2. Async does not work with expandable rows

I'm working on 1. but not 2. so I'm advising you on what will happen if you try to do anything about 2.

Also, as I've been diving into this issue, I'm actually finding it's another manifestation of the issue outlined here: #679, which means there can't be any true fix for the time being. If you update state, the data will be refreshed, and you'll have to keep it up to date yourself, until there's a new set of APIs that will allow you to control what happens. I'm considering adding an experimental API in the mean time, but thinking about the best way to go about this.

@gabrielliwerant gabrielliwerant added the v4 state/props This issue is related to state and props attempting to control the same values label Oct 10, 2019
@keybraker
Copy link
Author

The functionality I need can be understood by the following example:

A list of cars, is rendered by the table, and for every car's details you have to drop the expandable row. But as a more performance oriented approach, you get all the cars and when a certain car is selected/expanded, you make the call to the database, to get the details.

  1. An approach would be to load everything from the beginning, but throwing performance out the window.

  2. I could create a button you have to press to fetch the data, but it would be counter intuitive.

So I have kept it this way, meaning it closes the first time you press it and then you press it once more and it expands, until a proper fix id in place. Do you have a better solution ?

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Oct 11, 2019

If you don't mind expanding the row and having a loading indicator in there until your request is successful, then that will fix problem 2 for you, but it still won't address problem 1, which just can't be done at the moment.

For what it's worth, it's not clear to me on hearing your use case that performance will be automatically improved by sending new requests every time a row is expanded. What I might do in your shoes is pull all the data for two pages of the table up front. You have the data for the first page, and you have the second queued up in anticipation of the user hitting the next page. Then you load the 3rd and 4th in the background, as the user pages forward and it continues. If you're retrieving basic json responses from your server then the limiting factor is most likely the number of requests you make, not the size of data. You need really large data sets before you save on performance by making small requests. In most cases, the penalty of the request itself, going through all the http layers back and forth and hitting the database, would be much worse (you can also benefit from caching strategies more easily). Just food for thought. This approach might also make it easier for you to manage state, as you'd only be making these changes when the user pages forward (though it might reset the expanded rows on previous pages).

@abdu355
Copy link

abdu355 commented Oct 29, 2019

Came up with a workaround that keeps track of the rows open, such that when the data is loaded asynchronously, the expanded rows array ( rowsExpanded ) is used instead, any re-renders will not close the expanded rows, unless you tell it to.

Data table options

const options = { rowsExpanded: this.state.rowsExpanded, expandableRows: true, onRowsExpand: (currentRowsExpanded, allRowsExpanded) => { //console.log(currentRowsExpanded, allRowsExpanded); this.updateRowsExpanded(allRowsExpanded); }, renderExpandableRow: (rowData, rowMeta) => { const colSpan = rowData.length + 1; return ( <TableRow> <TableCell colSpan={colSpan}> <DeviceDetails rowData={rowData} /> </TableCell> </TableRow> ); } };

UpdateRowsExpanded method:

updateRowsExpanded(allRowsExpanded) { var rowsExpanded = _.map(allRowsExpanded, item => { return item.index; }); this.setState({ rowsExpanded }); //simple value }

Monitor row updates:

componentDidUpdate(prevProps, prevState) { if (prevState.rowsExpanded !== this.state.rowsExpanded) { console.log(this.state.rowsExpanded); } }

rowsExpanded

@MikeShobe
Copy link

Any update on this one? I'm having the same issue with setState in onRowsSelect, I'm trying to modify my multiple selection options based on which items are being selected.

@patorjk
Copy link
Collaborator

patorjk commented Dec 4, 2019

This PR would fix the issue: #1086

A current workaround for this would be to save all the table-state information you need in an external object and then plug it into the options object for the table. In this case the rowsExpanded array would need to be saved off and re-placed into the table to avoid the re-render problem.

More discussion on this issue can be found here: #807

@gowthamrajmpuc
Copy link

Hi how to setState onRowClick ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug v4 state/props This issue is related to state and props attempting to control the same values
Projects
None yet
Development

No branches or pull requests

6 participants