fixed-data-table-2 icon indicating copy to clipboard operation
fixed-data-table-2 copied to clipboard

V1.0 pagination support

Open KamranAsif opened this issue 8 years ago • 2 comments

Current Behavior

Currently the rowHeightGetter function looks like a bad pattern. There is no way to tell the table that you have a new for a row, and it only currently works because of some other side effects.

Possible Solution

A cleaner approach is to pass the row heights as an array. However, this has problems when trying to support millions of rows. In addition, we would require a linear scan on every change to detect which row heights have changed.

A better format would be an object that is a map of rowIndex to rowHeight. The user should only need to pass in a subset of rows; the visible rows plus some buffer.

I suggest we add a new callback that returns a 'page' size for the user to focus on. The user can use this to prefetch data, and feed data to the table such as rowHeights. The idea is that we only need the data that is in the page returned by the callback.

This can pave the way for other functionality like rowDataGetter https://github.com/schrodinger/fixed-data-table-2/pull/162

Thoughts? We can introduce the pagination support in 1.0 and support both the rowHeightGetter (with deprecation notice) and new rowHeights object. Then in 1.1+ we can switch to only supporting rowHeights

KamranAsif avatar Jun 02 '17 02:06 KamranAsif

This sounds reasonable to me. Have you done any research into what react-virtualized does to support this? In particular they have a CellMeasurer which handles the common case of the user not knowing the row height, although I don't think it has the best usability. We could probably learn a good amount reading some of what has worked for them and what hasn't.

In addition, the suggested algorithm would still be incorrect wrt to the scrollbar height when we only have a subset of row heights. Have you given any thought to what we can do there to keep a good UX? Should we require all row heights initially and then accept partial updates for changes? Or require a sane default?

wcjordan avatar Jun 05 '17 05:06 wcjordan

This comment discusses the value of DataObject vs DataVersion approaches to this: https://github.com/schrodinger/fixed-data-table-2/pull/162#issuecomment-305547043

wcjordan avatar Jun 14 '17 17:06 wcjordan