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

Fix property "scrollToRow" not working for the added rows when "rowsCount" grows #435

Closed VladimirTechMan closed 6 years ago

VladimirTechMan commented 8 years ago

Currently, inside the _calculateState() method of the FixedDataTable class, the implementation code first applies the updated value of the 'scrollToRow' property (if it has changed). And then the code checks if the number of rows in the table has changed – in that case a new scroll helper object is created. In the practical cases, when the table grows (thus, the row count increases) and it is required to scroll the updated table to a row beyond the old row count (i.e. beyond the old max row index), the above code flow does not work properly:

First, the new value of scrollToRow is passed to the previously created scroll helper object. Which cannot correctly calculate the new index and offset for the first visible table row as the scrollToRow index passed to it lies beyond the (old) row count that that helper object knows of. And it is only after that, that the code creates a new scroll helper for the updated row count. But at that point, the values for the first visible row index and its offset have already been updated (incorrectly) and are not properly reflecting the row index requested with 'scrollToRow'.

This bug makes it very inconvenient to use 'scrollToRow' with dynamically changing table data – an ugly workaround has to be applied in the external code, to force re-rendering the table once again immediately after rendering it with an updated number of rows. Thus, resulting in totally unnecessary re-render cycles (not to mention that figuring out that workaround is not that obvious at all, without diving deep into the implementation code).

The fix is simple here: The code block for checking and applying the value of 'scrollToRow' (if that value has changed) needs to be invoked after the code block responsible for the internal state updates and the creation of new scroll helper, being done on a changed number of table rows.

Hopefully, we can see a quick update of the official repository with this fix included. @zpao, thanks for your efforts on reviving the official support for this project!

KamranAsif commented 8 years ago

We've had this fix in our fork for a while: https://github.com/schrodinger/fixed-data-table-2 We have a bunch of new features, bug fixes, unit testing, etc. Feel free to check it out

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.