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

Can calling with updateValue() in customBodyRender() cause other columns to update? #1405

Closed
cahna opened this issue Jul 10, 2020 · 10 comments
Closed
Labels

Comments

@cahna
Copy link

cahna commented Jul 10, 2020

Expected Behavior

Calling updateValue() in a column's customBodyRender() either (or both):

  • updates the entire row(?)
  • reflects update in both tableMeta.tableData and tableMeta.rowData

(Or am I misusing updateValue()? ie: do I need to move all column value updates into React state so MUIDataTable's data prop triggers updates?)

Current Behavior

In a row, if column A with customBodyRender() depends on the value of column B for its render value:

  • A does not reflect B's calls to updateValue()
  • tableMeta.tableData and tableMeta.rowData reflect different values (see image, below)
    image

Steps to Reproduce (for bugs)

Sandbox: https://codesandbox.io/s/muidatatables-custom-toolbar-28cvu

  1. Configure a column to contain enable/disable buttons that trigger dialogs to confirm the action for the row
  2. Configure a row to render an input if the value of the enable/disable column is set to a specific value
  3. Invoke the enable button and confirm via dialog
  4. The enable/disable column is updated, but the dependent column in the same row does not reflect the update

Your Environment

Tech Version
Material-UI 4.11.0
MUI-datatables 3.1.5
React 16.13.1
browser Chromium Version 83.0.4103.106 (Official Build) snap (64-bit)
@patorjk
Copy link
Collaborator

patorjk commented Jul 11, 2020

So if I understand this correctly, rowData correctly updates, but tableData shows the old data (it looks like it shows the original data in all columns)? Is this the issue?

@cahna
Copy link
Author

cahna commented Jul 11, 2020

I believe that is the issue (if updates to tableData trigger a re-render, then we should see the correct column updates if the correct value is present in tableData)

@patorjk patorjk added the bug label Jul 11, 2020
@patorjk
Copy link
Collaborator

patorjk commented Jul 11, 2020

I will need to think about this some. I'm pretty sure this line is the problem:

https://github.com/gregnb/mui-datatables/blob/master/src/MUIDataTable.js#L1059

Basically, tableData is pointing to the data that's passed into the table. I'll put this on my list to tackle next. In the meantime, one workaround solution would be to use customBodyRenderLite instead (it gives you the dataIndex. You'd update your data, and then the table would update).

@cahna
Copy link
Author

cahna commented Jul 11, 2020

Sorting seems affected, too:
sorting

@patorjk
Copy link
Collaborator

patorjk commented Jul 11, 2020

So after researching this a bit, it looks like tableData was originally supposed to be the processed table rows (the data after the table had transformed the input data). However, there was a bug when object data was used. And it would lead to tableData pointing to the data that was inputted. The previous maintainer suggested this fix: #933

However, that fix kills the performance of the table. I figured out a better fix that doesn't impact performance (the data that needs to be passed is already available), and added a new prop to tableMeta called "currentTableData" which points to the table's current data. It's an array of objects, where each object contains the row of data for the table and the index to the inputted data for that row.

I could process this object and make it an array of arrays, but I figure having the dataIndex for each row could be useful.

I've released a beta version here:

npm install mui-datatables@v3.2.0-beta.1

Codesandbox: https://codesandbox.io/s/muidatatables-custom-toolbar-ffvf2

@cahna
Copy link
Author

cahna commented Jul 11, 2020

This seems to work. I have updated the codesandbox to reflect the desired behavior: https://codesandbox.io/s/muidatatables-custom-toolbar-tx8ft?file=/index.js

@cahna
Copy link
Author

cahna commented Jul 11, 2020

CORRECTION: sorting does not seem to work (see "Example" column):
sorting2

@patorjk
Copy link
Collaborator

patorjk commented Jul 11, 2020

This appears to be a React issue. Check out this version of the sandbox where value is rendered rather than a TextBox:

https://codesandbox.io/s/muidatatables-custom-toolbar-q1nvp?file=/index.js

It sorts correctly and shows that value is matching up with where it should be. Additionally, if you change "defaultValue" to "value", you'll see that it sorts correctly too.

I believe React is diffing the component props and not detecting a difference (I'm guessing defaultValue has no effect for re-renders), so it's not re-rendering them. You can force the TextField to re-render by setting it's key property (I did that in the codesandbox above, you can see it if you comment out the first return).

@cahna
Copy link
Author

cahna commented Jul 11, 2020

Oh, duh, that makes sense. The rows are being rendered as a list, so components need a key. Thank you!

@patorjk
Copy link
Collaborator

patorjk commented Jul 16, 2020

Fixed in version 3.2.0

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

No branches or pull requests

2 participants