search

facebookarchive / fixed-data-table

A React table component designed to allow presenting thousands of rows of data.
http://facebook.github.io/fixed-data-table/
Other
4.3k stars 553 forks source link

Use "rowIndex" instead of "i" to key rows #440

Closed ekh64 closed 6 years ago

ekh64 commented 8 years ago

Using i as a key for the FixedDataTableRow sometimes resulted in incorrect cells being rendered in the table after scrolling. I found using the rowIndex as a key fixed this bug.

facebook-github-bot commented 8 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

ghost commented 8 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

KamranAsif commented 8 years ago

@ekh64 Can you give some repo steps of where you see the bug?

ekh64 commented 8 years ago

The issue occurs when using input fields inside Cells.

Here's my fiddle to replicate the issue.

Enter text in any input and the value of the input field starts appearing in other rows. Keying by rowIndex seems to maintain the integrity of the input fields when scrolling.

KamranAsif commented 8 years ago

@wcjordan This might be a better solution to https://github.com/schrodinger/fixed-data-table-2/issues/18

KamranAsif commented 8 years ago

@ekh64 Could you make the same PR on our maintained fork? https://github.com/schrodinger/fixed-data-table-2

We haven't seen much activity on this repo (just look at the number of open issues/PRs), but we are trying to actively maintain this library.

wcjordan commented 8 years ago

This would resolve that issue, but I think it will have a negative impact on performance since buffered rows will now only be recycling the React objects, not the DOM elements. @KamranAsif do you want to try this out on our scrolling performance test and see if there's noticeable degradation?

zpao commented 6 years ago

Thanks for your pull request. I apologize for leaving this project in limbo and not addressing your concerns. I have archived this project after doing a final release to make it compatible with React v16.