Add row based virtualization to pivot table #4059
Conversation
|
|
||
| export let pivotDataStore: PivotDataStore; | ||
|
|
||
| const OVERSCAN = 80; | ||
| const ROW_HEIGHT = 24; |
There was a problem hiding this comment.
24 seems a bit too tight for row height. The mock has height as 28 for each row?
There was a problem hiding this comment.
Where are you seeing 28? The cells in Figma are 24.
|
|
||
| export let pivotDataStore: PivotDataStore; | ||
|
|
||
| const OVERSCAN = 80; |
There was a problem hiding this comment.
Overscan of 80 seems excessive? Probably can make it 30-40
There was a problem hiding this comment.
I've spent a good bit of time tinkering with this value. There is no downside to setting it at 80. Any browser is more than capable of stably rendering ~200 rows. What they can't do, within the context of TanStack's virtualization paradigm, is quickly mount/unmount/rerender huge chunks rows, which is what happens when you have a lower overscan. Virtualization, frankly, is not needed on views that have less than 1500 (rough estimate) total cells, so this is a way of bypassing virtualization for those smaller tables. Setting it at 80 means that you get very little flashing even when scrolling quickly. This will vary from browser to browser, of course, but there's a rough example in the screenshots below. Additionally, setting it higher means that your cell widths are being derived from a large set of values, which will cause less column size snapping in the short term.
Scrolling with OVERSCAN at 80:
Scrolling with OVERSCAN at 30:
| </div> | ||
| </th> | ||
| {/each} | ||
| <td colspan={headerGroups.length} style:height="{before}px"> </td> |
There was a problem hiding this comment.
Why do we need this? Wondering if we can follow Svelte virtual's example for sticky rows as well?
https://github.com/TanStack/virtual/blob/main/examples/svelte/sticky/src/App.svelte
There was a problem hiding this comment.
The rendering approach in this example uses absolute positioning of rows, which is not performant enough for our use case and does not work with well with rendering table elements as opposed to divs.
See: TanStack/virtual#476
See: TanStack/virtual#585 (comment)
See: TanStack/virtual#656
nishantmonu51
added
the
blocker
|
@djbarnwal Responded to your questions and updated the comments. |
nishantmonu51
added
blocker
* add row based virtualization to table * variable cleanup * add row padding description
This PR adds row based virtualization to the pivot table. Column virtualization will be added separately.
TanStack's virtualization library and flexRenderer are not performant enough for our use case and will need to be replaced.