feat: Grid table virtualization #93

stephenh · 2021-05-31T15:43:26Z

See comments but I played around with:

react-window (looks like it was the best, but maintainer has moved on, and I couldn't immediately get it to play nicely with display: contents),
react-virtual (headless just-a-hook, looks really small and tight but right away was pretty finicky), and ended up on
react-virtuoso (seems like the new "best" and is actively maintained, tons of features, but also largest bundle size I've heard but not actually checked/compared)

Also, specifically react-virtuoso seems to be the only library that does measured per item sizing. react-window let's you provide a size variable (like we would have a per-kind RowStyle[kind].height) but AFAIU (could be wrong, but I think?) you have to know it up-front without measuring it from the DOM. Same with react-aria's useVirtualization afaict (I looked into the source code and eventually got to "these table rows are X height, those are Y height"). So, dunno, I think know-height-ahead-of-time-based-on-row-kind would be an okay limitation for us if we had to go that way, but the "meh just measure it and it magically works" is pretty great imo.

I extended the as from table | div to table | div | virtual, which ended up being a pretty nice/cute slice point.

I did have to patch react-virtuoso to work around "display: contents shows up as height zero":

petyosi/react-virtuoso@master...homebound-team:probe-item-size

And am working on upstreaming it:

petyosi/react-virtuoso#375

UPDATE: ^ is now merged into the latest react-virtuoso link, and I've updated the PR to use that instead of our fork.

stephenh · 2021-05-31T15:45:03Z

src/components/GridTable.tsx

@@ -245,7 +243,6 @@ export function GridTable<R extends Kinded, S = {}, X extends Only<GridTableXss,
            isCollapsed,
            toggleCollapsedId,
            ...sortProps,
-            persistCollapse,


This prop was being passed but not used

stephenh · 2021-05-31T15:51:47Z

src/components/GridTable.tsx

    ...((rowStyle?.rowLink || rowStyle?.onClick) && {
      // Even though backgroundColor is set on the cellCss (due to display: content), the hover target is the row.
      "&:hover > *": Css.cursorPointer.bgColor(maybeDarken(rowStyleCellCss?.backgroundColor, style.rowHoverColor)).$,
    }),
    ...maybeApplyFunction(row, rowStyle?.rowCss),
  };

-  const TableRow = as === "div" ? "div" : "tr";
+  const Row = as === "table" ? "tr" : "div";


Most of the changes from here down are flipping logic from if div ... else table to be if table ... else div because the "else div" applies to both as div as well as as virtual.

stephenh · 2021-06-01T17:39:54Z

src/components/GridTable.tsx

        ...Css
          // Apply the between-row styling with `div + div > *` so that we don't have to have conditional
          // `if !lastRow add border` CSS applied via JS that would mean the row can't be React.memo'd.
          // The `div + div` is also the "owl operator", i.e. don't apply to the 1st row.
-          .addIn("& > div + div > *, & > tbody > tr ", style.betweenRowsCss)


This renderCssGrid got simpler b/c the table logic got moved into renderTable, which also made things like maybeWrapWith go away, so I think turned out.

KoltonG

🍣 I've checked out the Storybook and it's neat to see the virtualization. I was surprised that it did not keep the already rendered div's though as the work has been done (maybe for really really big tables it's worth removing them).

KoltonG · 2021-06-02T14:36:18Z

src/components/GridTable.tsx

@@ -120,7 +124,7 @@ export interface GridTableProps<R extends Kinded, S, X> {
   * @example
   *  { header: "Name", data: ({ name }) => name, w: "75px", align: "right" }
   */
-  as?: TableAs;
+  as?: RenderAs;


Thoughts on defaulting to Virtual all the time? Is there any benefit not to? And Keeping as the way it was?

I think that the as=virtual needs its container to be pinned to "effectively 100% - chrome/padding/whatever", so I don't think we can just flip existing usages over.

Also I think virtual has a slightly worse UX for things like "find in page"...

But, dunno. I'd thought of something like "if number of rows is over 100, flip to virtual by default if as isn't explicitly set to div or table", but I think that'd be even trickier to get right on "my DOM parent sometimes / sometimes doesn't need to set the height in this specific way".

KoltonG · 2021-06-02T14:47:22Z

I see that they mentioned they work with Grid (they are headless) but wondering if the API gives us anything to get rid of the display: content?

🔗 https://virtuoso.dev/grid-responsive-columns/

stephenh · 2021-06-02T14:48:05Z

I was surprised that it did not keep the already rendered div's though as the work has been done

Yeah, afaict basically none of the virtualization libraries do that, but they each have issues requesting it:

bvaughn/react-window#234

petyosi/react-virtuoso#298

I've also tried to google around about how to "keep DOM nodes that you don't technically want in the DOM" and it seems like React just doesn't like doing that. I.e. tricking the reconciler to keep around the DOM node that is already rendered is really hard to do, at least with a provided-by-React API/approach. So I think you'd have to resort to like "keeping it in the DOM but display: none" or something like that. Which ... would probably work? Dunno.

I agree it seems like an obvious thing to try and do... 🤷 Maybe we can try and do a PR for react-virtuoso on next hack day. :-)

KoltonG · 2021-06-02T14:48:22Z

I think we worked on a few tickets to get our tables to scroll to a certain sections, this would be nice to use https://virtuoso.dev/scroll-to-index/

KoltonG · 2021-06-02T14:49:59Z

I see that they mentioned they work with Grid (they are headless) but wondering if the API gives us anything to get rid of the display: content?

🔗 https://virtuoso.dev/grid-responsive-columns/

This way we can get rid of the table approach!

stephenh · 2021-06-02T14:54:11Z

https://virtuoso.dev/grid-responsive-columns/

Yeah I saw that, but didn't think it was for "rows in a table", vs. just "a long list of items". Granted, that is what CSS grid is... And yeah, they do mention "css grid" there, which I'd missed... (good catch)

That said, the reason we added the extra "div display: contents in the middle" in the first place was to do row-level hovers and row-levels click targets. So I think we need to keep the display: contents approach to keep supporting those features.

stephenh · 2021-06-02T14:55:19Z

this would be nice to use https://virtuoso.dev/scroll-to-index/

Yep! Wanted to leave it out of this initial PR but saw that / agreed we should figure out how to use it.

## [1.35.0](v1.34.0...v1.35.0) (2021-06-03) ### Features * Grid table virtualization ([#93](#93)) ([3af84a5](3af84a5))

homebound-team-bot · 2021-06-03T12:52:31Z

🎉 This PR is included in version 1.35.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

stephenh requested review from bdow, KoltonG, ksk5280 and lucyconklin as code owners May 31, 2021 15:43

stephenh commented May 31, 2021

View reviewed changes

stephenh changed the title ~~Grid table virtualization~~ feat: Grid table virtualization May 31, 2021

stephenh commented May 31, 2021

View reviewed changes

stephenh force-pushed the grid-table-virtualization branch 2 times, most recently from a6ce36b to 7088074 Compare June 1, 2021 17:35

stephenh commented Jun 1, 2021

View reviewed changes

KoltonG approved these changes Jun 2, 2021

View reviewed changes

stephenh added 13 commits June 2, 2021 19:30

Proof of concept with react-virtuoso.

faf2cbc

Don't need AutoSizer.

f689ee0

Add 'virtual' to TableAs, rename to RenderAs.

8e47430

Add comments, throw in stickyHeader.

38ac985

Fix story not memoizing columns.

07dfa3c

Add objectId.

ce20d09

Make renders top-level.

59dc991

Working with auto sizing.

5aa8f48

Working with auto-sizing and no Item.

ab92068

Remove react-window and auto-sizer.

1e9792e

Undo the nodeFn approach.

5d1f589

Remove now unused refs.

d6053ef

Add comments.

c30298e

stephenh added 11 commits June 2, 2021 19:30

Update sticky story.

adc0fc7

Use our react-virtuoso fork until patch is upstream.

4127afc

Remove console.log.

8a02e55

Update snapshots.

208fe8b

Bump back to react-virtuoso which was itemSize merged.

e2211d7

Remove stale comment.

e23d262

Pin the firstRowMessage too b/c it's easy.

a119c0a

Remove link to resolved/patched issue.

959cdfd

Dedup copy/pasted gridTemplateColumns calc.

86f666e

Use Css.gtc and inline the vars.

dffbb14

Fix up after rebases.

3ae470f

stephenh force-pushed the grid-table-virtualization branch from 12ea5ce to 3ae470f Compare June 3, 2021 00:31

stephenh merged commit 3af84a5 into main Jun 3, 2021

stephenh deleted the grid-table-virtualization branch June 3, 2021 12:49

homebound-team-bot pushed a commit that referenced this pull request Jun 3, 2021

chore(release): 1.35.0 [skip ci]

b125efd

## [1.35.0](v1.34.0...v1.35.0) (2021-06-03) ### Features * Grid table virtualization ([#93](#93)) ([3af84a5](3af84a5))

homebound-team-bot added the released label Jun 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Grid table virtualization #93

feat: Grid table virtualization #93

stephenh commented May 31, 2021 •

edited

Loading

stephenh May 31, 2021

stephenh May 31, 2021

stephenh Jun 1, 2021

KoltonG left a comment

KoltonG Jun 2, 2021

stephenh Jun 2, 2021 •

edited

Loading

KoltonG commented Jun 2, 2021

stephenh commented Jun 2, 2021

KoltonG commented Jun 2, 2021

KoltonG commented Jun 2, 2021

stephenh commented Jun 2, 2021 •

edited

Loading

stephenh commented Jun 2, 2021

homebound-team-bot commented Jun 3, 2021

feat: Grid table virtualization #93

feat: Grid table virtualization #93

Conversation

stephenh commented May 31, 2021 • edited Loading

stephenh May 31, 2021

Choose a reason for hiding this comment

stephenh May 31, 2021

Choose a reason for hiding this comment

stephenh Jun 1, 2021

Choose a reason for hiding this comment

KoltonG left a comment

Choose a reason for hiding this comment

KoltonG Jun 2, 2021

Choose a reason for hiding this comment

stephenh Jun 2, 2021 • edited Loading

Choose a reason for hiding this comment

KoltonG commented Jun 2, 2021

stephenh commented Jun 2, 2021

KoltonG commented Jun 2, 2021

KoltonG commented Jun 2, 2021

stephenh commented Jun 2, 2021 • edited Loading

stephenh commented Jun 2, 2021

homebound-team-bot commented Jun 3, 2021

stephenh commented May 31, 2021 •

edited

Loading

stephenh Jun 2, 2021 •

edited

Loading

stephenh commented Jun 2, 2021 •

edited

Loading