From faf2cbc54f3dff697ce5d62a25e7896ccd76763a Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 13:55:55 -0500 Subject: [PATCH 01/24] Proof of concept with react-virtuoso. --- package.json | 12 +- src/components/GridTable.stories.tsx | 31 +++-- src/components/GridTable.tsx | 200 ++++++++++++++++++++------- yarn.lock | 91 +++++++++++- 4 files changed, 264 insertions(+), 70 deletions(-) diff --git a/package.json b/package.json index 45084d77a..27d4e28f6 100644 --- a/package.json +++ b/package.json @@ -40,12 +40,22 @@ "change-case": "^4.1.2", "date-fns": "^2.21.3", "framer-motion": "^4.1.11", + "@types/react-virtualized-auto-sizer": "^1.0.0", + "@types/react-window": "^1.8.3", + "change-case": "^4.1.2", + "date-fns": "^2.21.3", + "framer-motion": "^4.1.11", + "memoize-one": "^5.2.1", "react-aria": "^3.6.0", "react-day-picker": "^7.4.10", "react-stately": "^3.5.0", + "react-virtualized-auto-sizer": "^1.0.5", + "react-virtuoso": "^1.8.6", + "react-window": "^1.8.6", "tributejs": "^5.1.3", "trix": "^1.3.1", - "react-popper": "^2.2.5" + "react-popper": "^2.2.5", + "trix": "^1.3.1" }, "peerDependencies": { "@emotion/react": ">=11", diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index 57fcdddd8..61a515267 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -91,26 +91,31 @@ export function Filtering() { const rows: GridDataRow[] = useMemo( () => [ { kind: "header", id: "header" }, - { kind: "data", id: "1", name: "c", value: 1 }, - { kind: "data", id: "2", name: "b", value: 2 }, - { kind: "data", id: "3", name: "a", value: 3 }, + ...zeroTo(1_000).map((i) => ({ kind: "data" as const, id: String(i), name: `c ${i}`, value: i })), ], [], ); const [filter, setFilter] = useState(); return ( -
- setFilter(e.target.value)} css={Css.ba.bGray900.$} /> - +
+
+ setFilter(e.target.value)} css={Css.ba.bGray900.$} /> +
+
+ +
); } +// .sb-main-padded adds 1rem on top/bottom +const heightWithoutStorybookPadding = "calc(100vh - 2rem)"; + export function NoRowsFallback() { const nameColumn: GridColumn = { header: "Name", data: ({ name }) => name }; const valueColumn: GridColumn = { header: "Value", data: ({ value }) => value }; @@ -214,14 +219,14 @@ export function StickyHeader() { sort: false, }; return ( -
+
some other top of page content ({ kind: "data" as const, id: "1", name: "c", value: 1 })), + ...zeroTo(2_000).map((i) => ({ kind: "data" as const, id: "1", name: "c", value: 1 })), ]} />
diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 7a9a97c04..eb6ec7e4a 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -1,7 +1,8 @@ +import memoizeOne from "memoize-one"; import { Observer } from "mobx-react"; import React, { - Fragment, MutableRefObject, + ReactElement, ReactNode, useCallback, useContext, @@ -10,6 +11,8 @@ import React, { useState, } from "react"; import { Link } from "react-router-dom"; +import AutoSizer from "react-virtualized-auto-sizer"; +import { Components, Virtuoso } from "react-virtuoso"; import { navLink } from "src/components/CssReset"; import { Icon } from "src/components/Icon"; import { Css, Margin, Only, Palette, Properties, px, Xss } from "src/Css"; @@ -106,6 +109,8 @@ export function setDefaultStyle(style: GridStyle): void { type TableAs = "div" | "table"; +type RowTuple = [GridDataRow, string[], ReactElement, Array]; + export interface GridTableProps { id?: string; /** @@ -204,11 +209,6 @@ export function GridTable (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) - .join(" "); - const [sortFlags, setSortValue] = useSortFlags(columns, sort, onSort); const maybeSorted = useMemo(() => { if (sorting === "client-side" && sortFlags) { @@ -219,9 +219,8 @@ export function GridTable row.kind !== "header"); - // For each row, keep it's ReactNode (React.memo'd) + its filter values, so we can re-filter w/o re-evaluating this useMemo. - type RowTuple = [GridDataRow, string[], ReactNode, Array]; - const allRows: RowTuple[] = useMemo(() => { + // For each row, keep it's ReactElement (React.memo'd) + its filter values, so we can re-filter w/o re-evaluating this useMemo. + const allRows: RowTuple[] = useMemo(() => { return maybeSorted.map((row) => { // We only pass sortProps to header rows, b/c non-headers rows shouldn't have to re-render on sorting // changes, and so by not passing the sortProps, it means the data rows' React.memo will still cache them. @@ -334,43 +333,163 @@ export function GridTable( + style: GridStyle, + id: string, + columns: GridColumn[], + headerRows: RowTuple[], + filteredRows: RowTuple[], + firstRowMessage: string | undefined, + xss: any, +): ReactElement { + const gridTemplateColumns = columns + // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string + .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) + .join(" "); return ( - *` 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) + .addIn("& > div + div > *", style.betweenRowsCss) // removes border between header and second row - .addIn("& > div:nth-of-type(2) > *, & > tbody > tr:first-of-type", Css.add("borderTopStyle", "none").$).$, + .addIn("& > div:nth-of-type(2) > *", Css.add({ borderTopStyle: "none" }).$).$, ...style.rootCss, ...xss, }} data-testid={id} > - {maybeWrapWith( - headerRows.map(([, , node]) => node), - as, - "thead", + {headerRows.map(([, , node]) => node)} + {/* Show an all-column-span info message if it's set. */} + {firstRowMessage && ( +
+
{firstRowMessage}
+
)} - {maybeWrapWith( - - {/* Show an all-column-span info message if it's set. */} - {firstRowMessage && renderFirstRowMessage(columns.length, firstRowMessage, style.firstRowMessageCss, as)} - {filteredRows.map(([, , node]) => node)} - , - as, - "tbody", + {filteredRows.map(([, , node]) => node)} + + ); +} + +function renderTable( + style: GridStyle, + id: string, + columns: GridColumn[], + headerRows: RowTuple[], + filteredRows: RowTuple[], + firstRowMessage: string | undefined, + xss: any, +): ReactElement { + return ( +
tbody > tr ", style.betweenRowsCss) + // removes border between header and second row + .addIn("& > tbody > tr:first-of-type", Css.add({ borderTopStyle: "none" }).$).$, + ...style.rootCss, + ...xss, + }} + data-testid={id} + > + {headerRows.map(([, , node]) => node)} + + {/* Show an all-column-span info message if it's set. */} + {firstRowMessage && ( + + + + )} + {filteredRows.map(([, , node]) => node)} + +
+ {firstRowMessage} +
+ ); +} + +function renderVirtual( + style: GridStyle, + id: string, + columns: GridColumn[], + headerRows: RowTuple[], + filteredRows: RowTuple[], + firstRowMessage: string | undefined, + xss: any, +): ReactElement { + const gridTemplateColumns = columns + // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string + .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) + .join(" "); + + return ( + + {({ height, width }) => ( + { + let i = index; + if (i < headerRows.length) { + return headerRows[i][2]; + } + i -= headerRows.length; + if (firstRowMessage) { + if (i === 0) { + return firstRowMessage; + } + i -= 1; + } + return filteredRows[i][2]; + }} + totalCount={headerRows.length + (firstRowMessage ? 1 : 0) + filteredRows.length} + /> )} - + ); } +const List = memoizeOne<(gs: GridStyle, gridTemplateColumns: string, id: string, xss: any) => Components["List"]>( + (gs, gridTemplateColumns, id, xss) => + React.forwardRef(({ style, children }, ref) => { + // This re-renders each time we have new children in the view port + return ( +
*` 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 > div > *", gs.betweenRowsCss) + // Flatten out the Item + .addIn("& > div", Css.display("contents").$).$, + ...gs.rootCss, + ...xss, + }} + data-testid={id} + > + {children} +
+ ); + }), +); + /** * Given an ADT of type T, performs a look up and returns the type of kind K. * @@ -969,7 +1088,6 @@ function getKinds(columns: GridColumn[]): R[] { /** GridTable as Table utility to apply element override styles */ const tableRowStyles = (as: TableAs, column?: GridColumn) => { const thWidth = column?.w; - return as === "div" ? {} : { @@ -977,27 +1095,3 @@ const tableRowStyles = (as: TableAs, column?: GridColumn) => { ...(thWidth ? Css.w(thWidth).$ : {}), }; }; - -/** GridTable as Table utility to handle rendering a full spanning first row */ -const renderFirstRowMessage = ( - spanWidth: number, - firstRowMessage: string, - firstRowMessageCss: Properties, - as: TableAs, -) => { - if (as === "div") { - return ( -
-
{firstRowMessage}
-
- ); - } - - return ( - - - {firstRowMessage} - - - ); -}; diff --git a/yarn.lock b/yarn.lock index fa670cad1..aa632dcc8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4010,6 +4010,20 @@ dependencies: "@types/react" "*" +"@types/react-virtualized-auto-sizer@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@types/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.0.tgz#fc32f30a8dab527b5816f3a757e1e1d040c8f272" + integrity sha512-NMErdIdSnm2j/7IqMteRiRvRulpjoELnXWUwdbucYCz84xG9PHcoOrr7QfXwB/ku7wd6egiKFrzt/+QK4Imeeg== + dependencies: + "@types/react" "*" + +"@types/react-window@^1.8.3": + version "1.8.3" + resolved "https://registry.yarnpkg.com/@types/react-window/-/react-window-1.8.3.tgz#14f74b144b4e3df9421eb31182dc580b7ccc7617" + integrity sha512-Xf+IR2Zyiyh/6z1CM8kv1aQba3S3X/hBXt4tH+T9bDSIGwFhle0GZFZGTSU8nw2cUT3UNbCHDjhxVQVZPtE8cA== + dependencies: + "@types/react" "*" + "@types/react@*": version "17.0.3" resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.3.tgz#ba6e215368501ac3826951eef2904574c262cc79" @@ -4199,6 +4213,18 @@ "@typescript-eslint/types" "4.21.0" eslint-visitor-keys "^2.0.0" +"@virtuoso.dev/react-urx@^0.2.5": + version "0.2.6" + resolved "https://registry.yarnpkg.com/@virtuoso.dev/react-urx/-/react-urx-0.2.6.tgz#e1d8bc717723b2fc23d80ea4e07703dbc276448b" + integrity sha512-+PLQ2iWmSH/rW7WGPEf+Kkql+xygHFL43Jij5aREde/O9mE0OFFGqeetA2a6lry3LDVWzupPntvvWhdaYw0TyA== + dependencies: + "@virtuoso.dev/urx" "^0.2.6" + +"@virtuoso.dev/urx@^0.2.5", "@virtuoso.dev/urx@^0.2.6": + version "0.2.6" + resolved "https://registry.yarnpkg.com/@virtuoso.dev/urx/-/urx-0.2.6.tgz#0028c49e52037e673993900d32abea83262fbd53" + integrity sha512-EKJ0WvJgWaXIz6zKbh9Q63Bcq//p8OHXHbdz4Fy+ruhjJCyI8ADE8E5gwSqBoUchaiYlgwKrT+sX4L2h/H+hMg== + "@webassemblyjs/ast@1.9.0": version "1.9.0" resolved "https://registry.yarnpkg.com/@webassemblyjs/ast/-/ast-1.9.0.tgz#bd850604b4042459a5a41cd7d338cbed695ed964" @@ -4778,7 +4804,7 @@ arrify@^2.0.1: resolved "https://registry.yarnpkg.com/arrify/-/arrify-2.0.1.tgz#c9655e9331e0abcd588d2a7cad7e9956f66701fa" integrity sha512-3duEwti880xqi4eAMN8AyR4a0ByT90zoYdLlevfrvU43vb0YZwZVfxOgxWrLXXXpyugL0hNZc9G6BiB5B3nUug== -asap@^2.0.0, asap@~2.0.3: +asap@^2.0.0, asap@~2.0.3, asap@~2.0.6: version "2.0.6" resolved "https://registry.yarnpkg.com/asap/-/asap-2.0.6.tgz#e50347611d7e690943208bbdafebcbc2fb866d46" integrity sha1-5QNHYR1+aQlDIIu9r+vLwvuGbUY= @@ -6188,6 +6214,11 @@ core-js@^3.0.1, core-js@^3.0.4, core-js@^3.6.5, core-js@^3.8.2: resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.10.1.tgz#e683963978b6806dcc6c0a4a8bd4ab0bdaf3f21a" integrity sha512-pwCxEXnj27XG47mu7SXAwhLP3L5CrlvCB91ANUkIz40P27kUcvNfSdvyZJ9CLHiVoKSp+TTChMQMSKQEH/IQxA== +core-js@^3.5.0: + version "3.13.1" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.13.1.tgz#30303fabd53638892062d8b4e802cac7599e9fb7" + integrity sha512-JqveUc4igkqwStL2RTRn/EPFGBOfEZHxJl/8ej1mXJR75V3go2mFF4bmUYkEIT1rveHKnkUlcJX/c+f1TyIovQ== + core-util-is@1.0.2, core-util-is@~1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7" @@ -10756,6 +10787,11 @@ memfs@^3.1.2: dependencies: fs-monkey "1.0.3" +"memoize-one@>=3.1.1 <6", memoize-one@^5.2.1: + version "5.2.1" + resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.2.1.tgz#8337aa3c4335581839ec01c3d594090cebe8f00e" + integrity sha512-zYiwtZUcYyXKo/np96AGZAckk+FWWsUdJ3cHGGmld7+AhvcWmQyGCYUh1hc4Q/pkOhb65dQR/pqCyK0cOaHz4Q== + memoizerific@^1.11.3: version "1.11.3" resolved "https://registry.yarnpkg.com/memoizerific/-/memoizerific-1.11.3.tgz#7c87a4646444c32d75438570905f2dbd1b1a805a" @@ -12451,6 +12487,13 @@ promise@^7.1.1: dependencies: asap "~2.0.3" +promise@^8.0.3: + version "8.1.0" + resolved "https://registry.yarnpkg.com/promise/-/promise-8.1.0.tgz#697c25c3dfe7435dd79fcd58c38a135888eaf05e" + integrity sha512-W04AqnILOL/sPRXziNicCjSNRruLAuIHEOVBazepu0545DDNGYHz7ar9ZgZ1fMU8/MA4mVxp5rkBWRi6OXIy3Q== + dependencies: + asap "~2.0.6" + prompts@2.4.0: version "2.4.0" resolved "https://registry.yarnpkg.com/prompts/-/prompts-2.4.0.tgz#4aa5de0723a231d1ee9121c40fdf663df73f61d7" @@ -12622,6 +12665,13 @@ quick-lru@^4.0.1: resolved "https://registry.yarnpkg.com/quick-lru/-/quick-lru-4.0.1.tgz#5b8878f113a58217848c6482026c73e1ba57727f" integrity sha512-ARhCpm70fzdcvNQfPoy49IaanKkTlRWF2JMzqhcJbhSFRZv7nPTvZJdcY7301IPmvW+/p0RgIWnQDLJxifsQ7g== +raf@^3.4.1: + version "3.4.1" + resolved "https://registry.yarnpkg.com/raf/-/raf-3.4.1.tgz#0742e99a4a6552f445d73e3ee0328af0ff1ede39" + integrity sha512-Sq4CW4QhwOHE8ucn6J34MqtZCeWFP2aQSmrlroYgqAV1PjStIhJXxYuTgUIfkEk7zTLjmIjLmU5q+fbD1NnOJA== + dependencies: + performance-now "^2.1.0" + ramda@^0.21.0: version "0.21.0" resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.21.0.tgz#a001abedb3ff61077d4ff1d577d44de77e8d0a35" @@ -12684,6 +12734,18 @@ react-addons-create-fragment@^15.6.2: loose-envify "^1.3.1" object-assign "^4.1.0" +react-app-polyfill@^1.0.6: + version "1.0.6" + resolved "https://registry.yarnpkg.com/react-app-polyfill/-/react-app-polyfill-1.0.6.tgz#890f8d7f2842ce6073f030b117de9130a5f385f0" + integrity sha512-OfBnObtnGgLGfweORmdZbyEz+3dgVePQBb3zipiaDsMHV1NpWm0rDFYIVXFV/AK+x4VIIfWHhrdMIeoTLyRr2g== + dependencies: + core-js "^3.5.0" + object-assign "^4.1.1" + promise "^8.0.3" + raf "^3.4.1" + regenerator-runtime "^0.13.3" + whatwg-fetch "^3.0.0" + react-aria@^3.6.0: version "3.6.0" resolved "https://registry.yarnpkg.com/react-aria/-/react-aria-3.6.0.tgz#8252596f61c0a2a3522bccc80266a4277843b12b" @@ -13020,6 +13082,29 @@ react-textarea-autosize@^8.3.0: use-composed-ref "^1.0.0" use-latest "^1.0.0" +react-virtualized-auto-sizer@^1.0.5: + version "1.0.5" + resolved "https://registry.yarnpkg.com/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.5.tgz#9eeeb8302022de56fbd7a860b08513120ce36509" + integrity sha512-kivjYVWX15TX2IUrm8F1jaCEX8EXrpy3DD+u41WGqJ1ZqbljWpiwscV+VxOM1l7sSIM1jwi2LADjhhAJkJ9dxA== + +react-virtuoso@^1.8.6: + version "1.8.6" + resolved "https://registry.yarnpkg.com/react-virtuoso/-/react-virtuoso-1.8.6.tgz#dc7d79ee9e309c13700296ff65b04f6ebecdcbaa" + integrity sha512-WFSI4YzdyAFrr3CuZnQ8cmubGMJjgmgyuhx/OpWLls1uK9O43s+6DMr9oBXHAgvCDjS3Nd3vkIJAhsJBJzS1nQ== + dependencies: + "@virtuoso.dev/react-urx" "^0.2.5" + "@virtuoso.dev/urx" "^0.2.5" + react-app-polyfill "^1.0.6" + resize-observer-polyfill "^1.5.1" + +react-window@^1.8.6: + version "1.8.6" + resolved "https://registry.yarnpkg.com/react-window/-/react-window-1.8.6.tgz#d011950ac643a994118632665aad0c6382e2a112" + integrity sha512-8VwEEYyjz6DCnGBsd+MgkD0KJ2/OXFULyDtorIiTz+QzwoP94tBoA7CnbtyXMm+cCeAUER5KJcPtWl9cpKbOBg== + dependencies: + "@babel/runtime" "^7.0.0" + memoize-one ">=3.1.1 <6" + react@^16.11.0, react@^16.12.0, react@^16.14.0, react@^16.8.3: version "16.14.0" resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" @@ -13195,7 +13280,7 @@ regenerate@^1.4.0: resolved "https://registry.yarnpkg.com/regenerate/-/regenerate-1.4.2.tgz#b9346d8827e8f5a32f7ba29637d398b69014848a" integrity sha512-zrceR/XhGYU/d/opr2EKO7aRHUeiBI8qjtfHqADTwZd6Szfy16la6kqD0MIUs5z5hx6AaKa+PixpPrR289+I0A== -regenerator-runtime@^0.13.4, regenerator-runtime@^0.13.7: +regenerator-runtime@^0.13.3, regenerator-runtime@^0.13.4, regenerator-runtime@^0.13.7: version "0.13.7" resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.13.7.tgz#cac2dacc8a1ea675feaabaeb8ae833898ae46f55" integrity sha512-a54FxoJDIr27pgf7IgeQGxmqUNYrcV338lf/6gH456HZ/PhX+5BcwHXG9ajESmwe6WRO0tAzRUrRmNONWgkrew== @@ -15628,7 +15713,7 @@ whatwg-encoding@^1.0.5: dependencies: iconv-lite "0.4.24" -whatwg-fetch@>=0.10.0: +whatwg-fetch@>=0.10.0, whatwg-fetch@^3.0.0: version "3.6.2" resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-3.6.2.tgz#dced24f37f2624ed0281725d51d0e2e3fe677f8c" integrity sha512-bJlen0FcuU/0EMLrdbJ7zOnW6ITZLrZMIarMUVmdKtsGvZna8vxKYaexICWPfZ8qwf9fzNq+UEIZrnSaApt6RA== From f689ee09d951a7885754ea81f81ce07f2015cd2f Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 13:56:27 -0500 Subject: [PATCH 02/24] Don't need AutoSizer. --- src/components/GridTable.tsx | 50 ++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index eb6ec7e4a..4c46cde04 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -11,7 +11,6 @@ import React, { useState, } from "react"; import { Link } from "react-router-dom"; -import AutoSizer from "react-virtualized-auto-sizer"; import { Components, Virtuoso } from "react-virtuoso"; import { navLink } from "src/components/CssReset"; import { Icon } from "src/components/Icon"; @@ -432,33 +431,28 @@ function renderVirtual( .join(" "); return ( - - {({ height, width }) => ( - { - let i = index; - if (i < headerRows.length) { - return headerRows[i][2]; - } - i -= headerRows.length; - if (firstRowMessage) { - if (i === 0) { - return firstRowMessage; - } - i -= 1; - } - return filteredRows[i][2]; - }} - totalCount={headerRows.length + (firstRowMessage ? 1 : 0) + filteredRows.length} - /> - )} - + { + let i = index; + if (i < headerRows.length) { + return headerRows[i][2]; + } + i -= headerRows.length; + if (firstRowMessage) { + if (i === 0) { + return firstRowMessage; + } + i -= 1; + } + return filteredRows[i][2]; + }} + totalCount={headerRows.length + (firstRowMessage ? 1 : 0) + filteredRows.length} + /> ); } From 8e474301e211f1a35a113560d7271954979d5f41 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 14:06:44 -0500 Subject: [PATCH 03/24] Add 'virtual' to TableAs, rename to RenderAs. --- src/components/GridTable.stories.tsx | 1 + src/components/GridTable.tsx | 80 ++++++++++++++-------------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index 61a515267..470b3fa5d 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -107,6 +107,7 @@ export function Filtering() { sorting={"client-side"} filter={filter} rows={rows} + as="virtual" />
diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 4c46cde04..cebe65780 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -106,7 +106,7 @@ export function setDefaultStyle(style: GridStyle): void { defaultStyle = style; } -type TableAs = "div" | "table"; +type RenderAs = "div" | "table" | "virtual"; type RowTuple = [GridDataRow, string[], ReactElement, Array]; @@ -124,7 +124,7 @@ export interface GridTableProps { * @example * { header: "Name", data: ({ name }) => name, w: "75px", align: "right" } */ - as?: TableAs; + as?: RenderAs; /** The column definitions i.e. how each column should render each row kind. */ columns: GridColumn[]; /** The rows of data (including any header/footer rows), to be rendered by the column definitions. */ @@ -332,9 +332,12 @@ export function GridTable = { + table: renderTable, + div: renderCssGrid, + virtual: renderVirtual, + }; + const render = renders[as]; return render(style, id, columns, headerRows, filteredRows, firstRowMessage, xss); } @@ -584,7 +587,7 @@ export type GridDataRow = { } & DiscriminateUnion; interface GridRowProps { - as: TableAs; + as: RenderAs; columns: GridColumn[]; row: GridDataRow; style: GridStyle; @@ -626,7 +629,7 @@ function GridRow(props: GridRowProps) { // root-element > row-element > cell-elements, so that we can have a hook for // hovers and styling. In theory this would change with subgrids. // Only enable when using div as elements - ...(as === "div" ? Css.display("contents").$ : {}), + ...(as === "table" ? {} : Css.display("contents").$), ...((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)).$, @@ -634,10 +637,10 @@ function GridRow(props: GridRowProps) { ...maybeApplyFunction(row, rowStyle?.rowCss), }; - const TableRow = as === "div" ? "div" : "tr"; + const Row = as === "table" ? "tr" : "div"; const div = ( - + {columns.map((column, idx) => { const maybeContent = applyRowFn(column, row); @@ -677,7 +680,7 @@ function GridRow(props: GridRowProps) { return renderFn(idx, cellCss, content, row, rowStyle); })} - +
); // Because of useToggleIds, this provider should basically never trigger a re-render, which is @@ -737,8 +740,8 @@ function applyRowFn(column: GridColumn, row: GridDataRow } /** Renders our default cell element, i.e. if no row links and no custom renderCell are used. */ -const defaultRenderFn: (as: TableAs) => RenderCellFn = (as: TableAs) => (key, css, content) => { - const Row = as === "div" ? "div" : "td"; +const defaultRenderFn: (as: RenderAs) => RenderCellFn = (as: RenderAs) => (key, css, content) => { + const Row = as === "table" ? "td" : "div"; return ( {content} @@ -780,7 +783,7 @@ const headerRenderFn: ( column: GridColumn, sortFlags: SortFlags | undefined, setSortValue: Function | undefined, - as: TableAs, + as: RenderAs, ) => RenderCellFn = (columns, column, sortFlags, setSortValue, as) => (key, css, content) => { const [currentValue, direction] = sortFlags || []; const newValue = column.sortValue || columns.indexOf(column); @@ -788,8 +791,7 @@ const headerRenderFn: ( sorted: newValue === currentValue ? direction : undefined, toggleSort: () => setSortValue!(newValue), }; - const Row = as === "div" ? "div" : "th"; - + const Row = as === "table" ? "th" : "div"; return ( {content} @@ -798,27 +800,27 @@ const headerRenderFn: ( }; /** Renders a cell element when a row link is in play. */ -const rowLinkRenderFn: (as: TableAs) => RenderCellFn = (as: TableAs) => (key, css, content, row, rowStyle) => { +const rowLinkRenderFn: (as: RenderAs) => RenderCellFn = (as: RenderAs) => (key, css, content, row, rowStyle) => { const to = rowStyle!.rowLink!(row); - if (as === "div") { + if (as === "table") { return ( - - {content} - + + + {content} + + ); } return ( - - - {content} - - + + {content} + ); }; /** Renders a cell that will fire the RowStyle.onClick. */ -const rowClickRenderFn: (as: TableAs) => RenderCellFn = (as: TableAs) => (key, css, content, row, rowStyle) => { - const Row = as === "div" ? "div" : "tr"; +const rowClickRenderFn: (as: RenderAs) => RenderCellFn = (as: RenderAs) => (key, css, content, row, rowStyle) => { + const Row = as === "table" ? "tr" : "div"; return ( rowStyle!.onClick!(row)}> {content} @@ -837,12 +839,13 @@ const alignmentToTextAlign: Record = right: "right", }; -function getJustification(column: GridColumn, maybeContent: ReactNode | GridCellContent, as: TableAs) { +// For alignment, use: 1) user-specified, else 2) center if non-1st header, else 3) left. +function getJustification(column: GridColumn, maybeContent: ReactNode | GridCellContent, as: RenderAs) { const alignment = (isContentAndSettings(maybeContent) && maybeContent.alignment) || column.align || "left"; - if (as === "div") { - return Css.justify(alignmentToJustify[alignment]).$; + if (as === "table") { + return Css.add("textAlign", alignmentToTextAlign[alignment]).$; } - return Css.add("textAlign", alignmentToTextAlign[alignment]).$; + return Css.justify(alignmentToJustify[alignment]).$; } // TODO This is very WIP / proof-of-concept and needs flushed out a lot more to handle nested batches. @@ -1068,11 +1071,6 @@ export function observableColumns(cols: GridColumn[]): Grid }) as any; } -/** GridTable as Table utility to wrap table header or body with thead or tbody */ -function maybeWrapWith(children: ReactNode, as: TableAs, Node: "thead" | "tbody") { - return as === "div" ? children : {children}; -} - function getKinds(columns: GridColumn[]): R[] { // Use the 1st column to get the runtime list of kinds const nonKindKeys = ["w", "sort", "sortValue", "align"]; @@ -1080,12 +1078,12 @@ function getKinds(columns: GridColumn[]): R[] { } /** GridTable as Table utility to apply element override styles */ -const tableRowStyles = (as: TableAs, column?: GridColumn) => { +const tableRowStyles = (as: RenderAs, column?: GridColumn) => { const thWidth = column?.w; - return as === "div" - ? {} - : { + return as === "table" + ? { ...Css.dtc.$, ...(thWidth ? Css.w(thWidth).$ : {}), - }; + } + : {}; }; From 38ac985aa0b21d0071ae01345186c365946c8586 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 14:19:59 -0500 Subject: [PATCH 04/24] Add comments, throw in stickyHeader. --- src/components/GridTable.stories.tsx | 1 + src/components/GridTable.tsx | 78 ++++++++++++++++------------ 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index 470b3fa5d..d9dd41dc7 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -106,6 +106,7 @@ export function Filtering() { columns={[nameColumn, valueColumn, actionColumn]} sorting={"client-side"} filter={filter} + stickyHeader={true} rows={rows} as="virtual" /> diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index cebe65780..f76dbf9c5 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -338,7 +338,7 @@ export function GridTable( @@ -348,6 +348,7 @@ function renderCssGrid( headerRows: RowTuple[], filteredRows: RowTuple[], firstRowMessage: string | undefined, + stickyHeader: boolean, xss: any, ): ReactElement { const gridTemplateColumns = columns @@ -389,6 +390,7 @@ function renderTable( headerRows: RowTuple[], filteredRows: RowTuple[], firstRowMessage: string | undefined, + stickyHeader: boolean, xss: any, ): ReactElement { return ( @@ -426,6 +428,7 @@ function renderVirtual( headerRows: RowTuple[], filteredRows: RowTuple[], firstRowMessage: string | undefined, + stickyHeader: boolean, xss: any, ): ReactElement { const gridTemplateColumns = columns @@ -435,11 +438,16 @@ function renderVirtual( return ( { let i = index; if (i < headerRows.length) { @@ -459,33 +467,39 @@ function renderVirtual( ); } -const List = memoizeOne<(gs: GridStyle, gridTemplateColumns: string, id: string, xss: any) => Components["List"]>( - (gs, gridTemplateColumns, id, xss) => - React.forwardRef(({ style, children }, ref) => { - // This re-renders each time we have new children in the view port - return ( -
*` 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 > div > *", gs.betweenRowsCss) - // Flatten out the Item - .addIn("& > div", Css.display("contents").$).$, - ...gs.rootCss, - ...xss, - }} - data-testid={id} - > - {children} -
- ); - }), -); +// Use memoize to create a single component type for a given set of props. I'm not +// entirely sure this is necessary, but [1] made it seem so. Also xss will probably +// not be memoized. +// +// [1]: https://codesandbox.io/s/github/bvaughn/react-window/tree/master/website/sandboxes/memoized-list-items?file=/index.js +const VirtualRoot = memoizeOne< + (gs: GridStyle, gridTemplateColumns: string, id: string, xss: any) => Components["List"] +>((gs, gridTemplateColumns, id, xss) => { + return React.forwardRef(({ style, children }, ref) => { + // This re-renders each time we have new children in the view port + return ( +
*` 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 > div > *", gs.betweenRowsCss) + // Flatten out the Item + .addIn("& > div", Css.display("contents").$).$, + ...gs.rootCss, + ...xss, + }} + data-testid={id} + > + {children} +
+ ); + }); +}); /** * Given an ADT of type T, performs a look up and returns the type of kind K. From 07dfa3c61fcd9811ed31308d39c7041832e62fcd Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 14:57:50 -0500 Subject: [PATCH 05/24] Fix story not memoizing columns. --- src/components/GridTable.stories.tsx | 22 ++++++++++------------ src/utils/objectId.ts | 0 2 files changed, 10 insertions(+), 12 deletions(-) create mode 100644 src/utils/objectId.ts diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index d9dd41dc7..72573e1f8 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -85,13 +85,18 @@ export const Hovering = newStory( ); export function Filtering() { - const nameColumn: GridColumn = { header: "Name", data: ({ name }) => name }; - const valueColumn: GridColumn = { header: "Value", data: ({ value }) => value }; - const actionColumn: GridColumn = { header: "Action", data: () =>
Actions
, sort: false }; const rows: GridDataRow[] = useMemo( () => [ { kind: "header", id: "header" }, - ...zeroTo(1_000).map((i) => ({ kind: "data" as const, id: String(i), name: `c ${i}`, value: i })), + ...zeroTo(100).map((i) => ({ kind: "data" as const, id: String(i), name: `ccc ${i}`, value: i })), + ], + [], + ); + const columns: GridColumn[] = useMemo( + () => [ + { header: "Name", data: ({ name }) => name }, + { header: "Value", data: ({ value }) => value }, + { header: "Action", data: () =>
Actions
, sort: false }, ], [], ); @@ -102,14 +107,7 @@ export function Filtering() { setFilter(e.target.value)} css={Css.ba.bGray900.$} />
- +
); diff --git a/src/utils/objectId.ts b/src/utils/objectId.ts new file mode 100644 index 000000000..e69de29bb From ce20d0931ceefb8b51dfc4a96840c476d49b1759 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 14:58:01 -0500 Subject: [PATCH 06/24] Add objectId. --- src/utils/objectId.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/utils/objectId.ts b/src/utils/objectId.ts index e69de29bb..cdbd4e536 100644 --- a/src/utils/objectId.ts +++ b/src/utils/objectId.ts @@ -0,0 +1,11 @@ +/** Utility for debugging object identity/caching/memo issues. */ +export const objectId = (() => { + let currentId = 0; + const map = new WeakMap(); + return (object: object): number => { + if (!map.has(object)) { + map.set(object, ++currentId); + } + return map.get(object)!; + }; +})(); From 59dc991f76b8bc97f989c2c057a62b13b56a5f91 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 14:58:15 -0500 Subject: [PATCH 07/24] Make renders top-level. --- src/components/GridTable.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index f76dbf9c5..6bca061e6 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -331,16 +331,16 @@ export function GridTable = { - table: renderTable, - div: renderCssGrid, - virtual: renderVirtual, - }; - const render = renders[as]; - return render(style, id, columns, headerRows, filteredRows, firstRowMessage, stickyHeader, xss); + return renders[as](style, id, columns, headerRows, filteredRows, firstRowMessage, stickyHeader, xss); } +// Determine which HTML element to use to build the GridTable +const renders: Record = { + table: renderTable, + div: renderCssGrid, + virtual: renderVirtual, +}; + function renderCssGrid( style: GridStyle, id: string, From 5aa8f4833bcfc0acaf3abc64db7210e7426f6403 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 17:26:19 -0500 Subject: [PATCH 08/24] Working with auto sizing. --- src/components/GridTable.stories.tsx | 11 +- src/components/GridTable.tsx | 185 ++++++++++++++++----------- 2 files changed, 118 insertions(+), 78 deletions(-) diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index 72573e1f8..b2841d990 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -106,8 +106,15 @@ export function Filtering() {
setFilter(e.target.value)} css={Css.ba.bGray900.$} />
-
- +
+
); diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 6bca061e6..e68b27b2e 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -4,10 +4,12 @@ import React, { MutableRefObject, ReactElement, ReactNode, + RefObject, useCallback, useContext, useEffect, useMemo, + useRef, useState, } from "react"; import { Link } from "react-router-dom"; @@ -108,7 +110,12 @@ export function setDefaultStyle(style: GridStyle): void { type RenderAs = "div" | "table" | "virtual"; -type RowTuple = [GridDataRow, string[], ReactElement, Array]; +type RowTuple = [ + GridDataRow, + string[], + (others?: object) => ReactElement, + Array, +]; export interface GridTableProps { id?: string; @@ -155,7 +162,6 @@ export interface GridTableProps { observeRows?: boolean; /** A combination of CSS settings to set the static look & feel (vs. rowStyles which is per-row styling). */ style?: GridStyle; - xss?: X; /** * If provided, collapsed rows on the table persists when the page is reloaded. * @@ -163,6 +169,7 @@ export interface GridTableProps { * be the collapsed state for project `p:1`'s precon stage specs & selections table. */ persistCollapse?: string; + xss?: X; } /** @@ -229,8 +236,8 @@ export function GridTable + const rowElement = (others?: object) => ( + ); @@ -264,7 +271,6 @@ export function GridTable[]>([]); + headerRowsRef.current = headerRows; + const filteredRowsRef = useRef[]>([]); + filteredRowsRef.current = filteredRows; + + return renders[as](style, id, columns, headerRowsRef, filteredRowsRef, firstRowMessage, stickyHeader, xss); } // Determine which HTML element to use to build the GridTable @@ -345,8 +356,8 @@ function renderCssGrid( style: GridStyle, id: string, columns: GridColumn[], - headerRows: RowTuple[], - filteredRows: RowTuple[], + headerRows: RefObject[]>, + filteredRows: RefObject[]>, firstRowMessage: string | undefined, stickyHeader: boolean, xss: any, @@ -371,14 +382,14 @@ function renderCssGrid( }} data-testid={id} > - {headerRows.map(([, , node]) => node)} + {headerRows.current!.map(([, , nodeFn]) => nodeFn())} {/* Show an all-column-span info message if it's set. */} {firstRowMessage && (
{firstRowMessage}
)} - {filteredRows.map(([, , node]) => node)} + {filteredRows.current!.map(([, , nodeFn]) => nodeFn())} ); } @@ -387,8 +398,8 @@ function renderTable( style: GridStyle, id: string, columns: GridColumn[], - headerRows: RowTuple[], - filteredRows: RowTuple[], + headerRows: RefObject[]>, + filteredRows: RefObject[]>, firstRowMessage: string | undefined, stickyHeader: boolean, xss: any, @@ -405,7 +416,7 @@ function renderTable( }} data-testid={id} > - {headerRows.map(([, , node]) => node)} + {headerRows.current!.map(([, , nodeFn]) => nodeFn())} {/* Show an all-column-span info message if it's set. */} {firstRowMessage && ( @@ -415,7 +426,7 @@ function renderTable( )} - {filteredRows.map(([, , node]) => node)} + {filteredRows.current!.map(([, , nodeFn]) => nodeFn())} ); @@ -425,20 +436,18 @@ function renderVirtual( style: GridStyle, id: string, columns: GridColumn[], - headerRows: RowTuple[], - filteredRows: RowTuple[], + headerRows: RefObject[]>, + filteredRows: RefObject[]>, firstRowMessage: string | undefined, stickyHeader: boolean, xss: any, ): ReactElement { - const gridTemplateColumns = columns - // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string - .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) - .join(" "); - return ( ( // I've tried to provide a custom Item component, but since it needs to accept // a custom ref and `data-*` props, and b/c RowTuple has already invoked // the GridRow function, we can't easily combine the two. - fixedItemHeight={56} - topItemCount={stickyHeader ? 1 : 0} - itemContent={(index) => { - let i = index; - if (i < headerRows.length) { - return headerRows[i][2]; - } - i -= headerRows.length; - if (firstRowMessage) { - if (i === 0) { - return firstRowMessage; - } - i -= 1; - } - return filteredRows[i][2]; - }} - totalCount={headerRows.length + (firstRowMessage ? 1 : 0) + filteredRows.length} + topItemCount={stickyHeader ? headerRows.current!.length : 0} + itemContent={() => null} + totalCount={(headerRows.current?.length || 0) + (firstRowMessage ? 1 : 0) + (filteredRows.current?.length || 0)} /> ); } -// Use memoize to create a single component type for a given set of props. I'm not -// entirely sure this is necessary, but [1] made it seem so. Also xss will probably -// not be memoized. -// -// [1]: https://codesandbox.io/s/github/bvaughn/react-window/tree/master/website/sandboxes/memoized-list-items?file=/index.js -const VirtualRoot = memoizeOne< - (gs: GridStyle, gridTemplateColumns: string, id: string, xss: any) => Components["List"] ->((gs, gridTemplateColumns, id, xss) => { - return React.forwardRef(({ style, children }, ref) => { - // This re-renders each time we have new children in the view port - return ( -
*` 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 > div > *", gs.betweenRowsCss) - // Flatten out the Item - .addIn("& > div", Css.display("contents").$).$, - ...gs.rootCss, - ...xss, - }} - data-testid={id} - > - {children} -
- ); - }); -}); +const VirtualItem = memoizeOne< + ( + style: GridStyle, + columns: GridColumn[], + headerRowsRef: RefObject[]>, + filteredRows: RefObject[]>, + firstRowMessage: string | undefined, + ) => Components["Item"] +>((style, columns, headerRowsRef, filteredRowsRef, firstRowMessage) => + React.memo((props) => { + // We purposefully ignore children that comes from the `itemContent` (which we don't + // even implement) because we want to purposefully "squish" our content up into + // react-virtuoso's Item location in the DOM. + const { children, ...others } = props; + + const headerRows = headerRowsRef.current || []; + const filteredRows = filteredRowsRef.current || []; + + // We keep header and filter rows separate, but react-virtuoso is a flat list, + // so we pick the right header / first row message / actual row. + let i = Number(props["data-index"]); + if (i < headerRows.length) { + return headerRows[i][2](others); + } + i -= headerRows.length; + + if (firstRowMessage) { + if (i === 0) { + return ( +
+
{firstRowMessage}
+
+ ); + } + i -= 1; + } + + // We pass in others, which has data-index, data-known-size, etc. + return filteredRows[i][2](others); + }), +); + +// Use memoize to create a single component type for a given set of props. +const VirtualRoot = memoizeOne<(gs: GridStyle, columns: GridColumn[], id: string, xss: any) => Components["List"]>( + (gs, columns, id, xss) => { + return React.forwardRef(({ style, children }, ref) => { + // This re-renders each time we have new children in the view port + const gridTemplateColumns = columns + // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string + .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) + .join(" "); + return ( +
div + div > *", gs.betweenRowsCss).$, + ...gs.rootCss, + ...xss, + }} + data-testid={id} + > + {children} +
+ ); + }); + }, +); /** * Given an ADT of type T, performs a look up and returns the type of kind K. @@ -616,7 +647,7 @@ interface GridRowProps { } // We extract GridRow to its own mini-component primarily so we can React.memo'ize it. -function GridRow(props: GridRowProps) { +function GridRow(props: GridRowProps): ReactElement { const { as, columns, @@ -630,8 +661,11 @@ function GridRow(props: GridRowProps) { setSortValue, isCollapsed, toggleCollapsedId, + ...others } = props; + console.log("GridRow rendering", row.id); + // We treat the "header" kind as special for "good defaults" styling const isHeader = row.kind === "header"; const rowStyle = rowStyles?.[row.kind]; @@ -654,7 +688,7 @@ function GridRow(props: GridRowProps) { const Row = as === "table" ? "tr" : "div"; const div = ( - + {columns.map((column, idx) => { const maybeContent = applyRowFn(column, row); @@ -722,7 +756,6 @@ const ObservedGridRow = React.memo((props: GridRowProps) => ( }} )); - /** If a column def return just string text for a given row, apply some default styling. */ function maybeAddHeaderStyling(content: ReactNode | GridCellContent, isHeader: boolean, sorting: boolean): ReactNode { if (typeof content === "string" && isHeader) { From ab920689db2d9fbb07af79de19e9dc1e48cd8afd Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 19:46:43 -0500 Subject: [PATCH 09/24] Working with auto-sizing and no Item. --- src/components/GridTable.tsx | 104 ++++++++++++----------------------- 1 file changed, 36 insertions(+), 68 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index e68b27b2e..330e668db 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -4,7 +4,6 @@ import React, { MutableRefObject, ReactElement, ReactNode, - RefObject, useCallback, useContext, useEffect, @@ -342,7 +341,7 @@ export function GridTable[]>([]); filteredRowsRef.current = filteredRows; - return renders[as](style, id, columns, headerRowsRef, filteredRowsRef, firstRowMessage, stickyHeader, xss); + return renders[as](style, id, columns, headerRows, filteredRows, firstRowMessage, stickyHeader, xss); } // Determine which HTML element to use to build the GridTable @@ -356,8 +355,8 @@ function renderCssGrid( style: GridStyle, id: string, columns: GridColumn[], - headerRows: RefObject[]>, - filteredRows: RefObject[]>, + headerRows: RowTuple[], + filteredRows: RowTuple[], firstRowMessage: string | undefined, stickyHeader: boolean, xss: any, @@ -382,14 +381,14 @@ function renderCssGrid( }} data-testid={id} > - {headerRows.current!.map(([, , nodeFn]) => nodeFn())} + {headerRows.map(([, , nodeFn]) => nodeFn())} {/* Show an all-column-span info message if it's set. */} {firstRowMessage && (
{firstRowMessage}
)} - {filteredRows.current!.map(([, , nodeFn]) => nodeFn())} + {filteredRows.map(([, , nodeFn]) => nodeFn())} ); } @@ -398,8 +397,8 @@ function renderTable( style: GridStyle, id: string, columns: GridColumn[], - headerRows: RefObject[]>, - filteredRows: RefObject[]>, + headerRows: RowTuple[], + filteredRows: RowTuple[], firstRowMessage: string | undefined, stickyHeader: boolean, xss: any, @@ -416,7 +415,7 @@ function renderTable( }} data-testid={id} > - {headerRows.current!.map(([, , nodeFn]) => nodeFn())} + {headerRows.map(([, , nodeFn]) => nodeFn())} {/* Show an all-column-span info message if it's set. */} {firstRowMessage && ( @@ -426,7 +425,7 @@ function renderTable( )} - {filteredRows.current!.map(([, , nodeFn]) => nodeFn())} + {filteredRows.map(([, , nodeFn]) => nodeFn())} ); @@ -436,74 +435,42 @@ function renderVirtual( style: GridStyle, id: string, columns: GridColumn[], - headerRows: RefObject[]>, - filteredRows: RefObject[]>, + headerRows: RowTuple[], + filteredRows: RowTuple[], firstRowMessage: string | undefined, stickyHeader: boolean, xss: any, ): ReactElement { return ( { + // We keep header and filter rows separate, but react-virtuoso is a flat list, + // so we pick the right header / first row message / actual row. + let i = index; + if (i < headerRows.length) { + return headerRows[i][2](); + } + i -= headerRows.length; + if (firstRowMessage) { + if (i === 0) { + return ( +
+
{firstRowMessage}
+
+ ); + } + i -= 1; + } + // We pass in others, which has data-index, data-known-size, etc. + return filteredRows[i][2](); }} - // We use display:contents to promote the itemContent out of virtuoso's - // Item/div wrapper (b/c our GridRow already has it), but that breaks the - // auto height detection. - // - // I've tried to provide a custom Item component, but since it needs to accept - // a custom ref and `data-*` props, and b/c RowTuple has already invoked - // the GridRow function, we can't easily combine the two. - topItemCount={stickyHeader ? headerRows.current!.length : 0} - itemContent={() => null} - totalCount={(headerRows.current?.length || 0) + (firstRowMessage ? 1 : 0) + (filteredRows.current?.length || 0)} + totalCount={(headerRows.length || 0) + (firstRowMessage ? 1 : 0) + (filteredRows.length || 0)} /> ); } -const VirtualItem = memoizeOne< - ( - style: GridStyle, - columns: GridColumn[], - headerRowsRef: RefObject[]>, - filteredRows: RefObject[]>, - firstRowMessage: string | undefined, - ) => Components["Item"] ->((style, columns, headerRowsRef, filteredRowsRef, firstRowMessage) => - React.memo((props) => { - // We purposefully ignore children that comes from the `itemContent` (which we don't - // even implement) because we want to purposefully "squish" our content up into - // react-virtuoso's Item location in the DOM. - const { children, ...others } = props; - - const headerRows = headerRowsRef.current || []; - const filteredRows = filteredRowsRef.current || []; - - // We keep header and filter rows separate, but react-virtuoso is a flat list, - // so we pick the right header / first row message / actual row. - let i = Number(props["data-index"]); - if (i < headerRows.length) { - return headerRows[i][2](others); - } - i -= headerRows.length; - - if (firstRowMessage) { - if (i === 0) { - return ( -
-
{firstRowMessage}
-
- ); - } - i -= 1; - } - - // We pass in others, which has data-index, data-known-size, etc. - return filteredRows[i][2](others); - }), -); - // Use memoize to create a single component type for a given set of props. const VirtualRoot = memoizeOne<(gs: GridStyle, columns: GridColumn[], id: string, xss: any) => Components["List"]>( (gs, columns, id, xss) => { @@ -519,7 +486,8 @@ const VirtualRoot = memoizeOne<(gs: GridStyle, columns: GridColumn[], id: s style={style} css={{ ...Css.dg.add({ gridTemplateColumns }).$, - ...Css.addIn("& > div + div > *", gs.betweenRowsCss).$, + ...Css.addIn("& > div + div > div > *", gs.betweenRowsCss).$, + ...Css.addIn("& > div", Css.display("contents").$).$, ...gs.rootCss, ...xss, }} From 1e9792ebd3f07a9fdef00cf1bde952018cce8c5b Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 20:48:48 -0500 Subject: [PATCH 10/24] Remove react-window and auto-sizer. --- package.json | 2 -- src/components/GridTable.tsx | 27 +++++++++++++++++++++++++++ yarn.lock | 29 +---------------------------- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index 27d4e28f6..9b0d4949e 100644 --- a/package.json +++ b/package.json @@ -49,9 +49,7 @@ "react-aria": "^3.6.0", "react-day-picker": "^7.4.10", "react-stately": "^3.5.0", - "react-virtualized-auto-sizer": "^1.0.5", "react-virtuoso": "^1.8.6", - "react-window": "^1.8.6", "tributejs": "^5.1.3", "trix": "^1.3.1", "react-popper": "^2.2.5", diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 330e668db..5c3c864a1 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -393,6 +393,7 @@ function renderCssGrid( ); } +/** Renders as a table, primarily/solely for good print support. */ function renderTable( style: GridStyle, id: string, @@ -431,6 +432,30 @@ function renderTable( ); } +/** + * Uses react-virtuoso to render rows virtually. + * + * It seems like react-virtuoso is the only one that can do _measured_ variable + * sizes. I.e. react-window's variable list let's you provide a size, but it's + * a size you lookup, not one that is measured from the DOM, see [1]. + * + * I also tried react-virtual, which is headless and really small, but a) the + * `measureRef` approach seems buggy [2] and b) rows were getting re-rendered + * maybe due to [3] and they have no examples showing memoization, which is + * concerning. + * + * Note that technically I had to patch react-virtuoso, see [4], to support our + * usage of `display: contents` but hopefully it'll get patched soon. + * + * react-virtuoso also seems like the most maintained (react-window is no + * longer being actively worked on) and featureful library (like sticky headers), + * so going with that for now. + * + * [1]: https://github.com/bvaughn/react-window/issues/6 + * [2]: https://github.com/tannerlinsley/react-virtual/issues/85 + * [3]: https://github.com/tannerlinsley/react-virtual/issues/108 + * [4]: https://github.com/petyosi/react-virtuoso/issues/375 + */ function renderVirtual( style: GridStyle, id: string, @@ -486,7 +511,9 @@ const VirtualRoot = memoizeOne<(gs: GridStyle, columns: GridColumn[], id: s style={style} css={{ ...Css.dg.add({ gridTemplateColumns }).$, + // Add an extra `> div` due to Item + itemContent both having divs ...Css.addIn("& > div + div > div > *", gs.betweenRowsCss).$, + // Add `display:contents` to Item to flatten it like we do GridRow ...Css.addIn("& > div", Css.display("contents").$).$, ...gs.rootCss, ...xss, diff --git a/yarn.lock b/yarn.lock index aa632dcc8..3b04f2a12 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4010,20 +4010,6 @@ dependencies: "@types/react" "*" -"@types/react-virtualized-auto-sizer@^1.0.0": - version "1.0.0" - resolved "https://registry.yarnpkg.com/@types/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.0.tgz#fc32f30a8dab527b5816f3a757e1e1d040c8f272" - integrity sha512-NMErdIdSnm2j/7IqMteRiRvRulpjoELnXWUwdbucYCz84xG9PHcoOrr7QfXwB/ku7wd6egiKFrzt/+QK4Imeeg== - dependencies: - "@types/react" "*" - -"@types/react-window@^1.8.3": - version "1.8.3" - resolved "https://registry.yarnpkg.com/@types/react-window/-/react-window-1.8.3.tgz#14f74b144b4e3df9421eb31182dc580b7ccc7617" - integrity sha512-Xf+IR2Zyiyh/6z1CM8kv1aQba3S3X/hBXt4tH+T9bDSIGwFhle0GZFZGTSU8nw2cUT3UNbCHDjhxVQVZPtE8cA== - dependencies: - "@types/react" "*" - "@types/react@*": version "17.0.3" resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.3.tgz#ba6e215368501ac3826951eef2904574c262cc79" @@ -10787,7 +10773,7 @@ memfs@^3.1.2: dependencies: fs-monkey "1.0.3" -"memoize-one@>=3.1.1 <6", memoize-one@^5.2.1: +memoize-one@^5.2.1: version "5.2.1" resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.2.1.tgz#8337aa3c4335581839ec01c3d594090cebe8f00e" integrity sha512-zYiwtZUcYyXKo/np96AGZAckk+FWWsUdJ3cHGGmld7+AhvcWmQyGCYUh1hc4Q/pkOhb65dQR/pqCyK0cOaHz4Q== @@ -13082,11 +13068,6 @@ react-textarea-autosize@^8.3.0: use-composed-ref "^1.0.0" use-latest "^1.0.0" -react-virtualized-auto-sizer@^1.0.5: - version "1.0.5" - resolved "https://registry.yarnpkg.com/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.5.tgz#9eeeb8302022de56fbd7a860b08513120ce36509" - integrity sha512-kivjYVWX15TX2IUrm8F1jaCEX8EXrpy3DD+u41WGqJ1ZqbljWpiwscV+VxOM1l7sSIM1jwi2LADjhhAJkJ9dxA== - react-virtuoso@^1.8.6: version "1.8.6" resolved "https://registry.yarnpkg.com/react-virtuoso/-/react-virtuoso-1.8.6.tgz#dc7d79ee9e309c13700296ff65b04f6ebecdcbaa" @@ -13097,14 +13078,6 @@ react-virtuoso@^1.8.6: react-app-polyfill "^1.0.6" resize-observer-polyfill "^1.5.1" -react-window@^1.8.6: - version "1.8.6" - resolved "https://registry.yarnpkg.com/react-window/-/react-window-1.8.6.tgz#d011950ac643a994118632665aad0c6382e2a112" - integrity sha512-8VwEEYyjz6DCnGBsd+MgkD0KJ2/OXFULyDtorIiTz+QzwoP94tBoA7CnbtyXMm+cCeAUER5KJcPtWl9cpKbOBg== - dependencies: - "@babel/runtime" "^7.0.0" - memoize-one ">=3.1.1 <6" - react@^16.11.0, react@^16.12.0, react@^16.14.0, react@^16.8.3: version "16.14.0" resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" From 5d1f5896d8a744957ac19fb1bfe402c01fefd086 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 20:52:19 -0500 Subject: [PATCH 11/24] Undo the nodeFn approach. --- src/components/GridTable.stories.tsx | 2 +- src/components/GridTable.tsx | 22 ++++++++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index b2841d990..507ba1ebc 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -88,7 +88,7 @@ export function Filtering() { const rows: GridDataRow[] = useMemo( () => [ { kind: "header", id: "header" }, - ...zeroTo(100).map((i) => ({ kind: "data" as const, id: String(i), name: `ccc ${i}`, value: i })), + ...zeroTo(1_000).map((i) => ({ kind: "data" as const, id: String(i), name: `ccc ${i}`, value: i })), ], [], ); diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 5c3c864a1..8451f0753 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -109,12 +109,7 @@ export function setDefaultStyle(style: GridStyle): void { type RenderAs = "div" | "table" | "virtual"; -type RowTuple = [ - GridDataRow, - string[], - (others?: object) => ReactElement, - Array, -]; +type RowTuple = [GridDataRow, string[], ReactElement, Array]; export interface GridTableProps { id?: string; @@ -235,7 +230,7 @@ export function GridTable ( + const rowElement = ( ); @@ -381,14 +375,14 @@ function renderCssGrid( }} data-testid={id} > - {headerRows.map(([, , nodeFn]) => nodeFn())} + {headerRows.map(([, , node]) => node)} {/* Show an all-column-span info message if it's set. */} {firstRowMessage && (
{firstRowMessage}
)} - {filteredRows.map(([, , nodeFn]) => nodeFn())} + {filteredRows.map(([, , node]) => node)} ); } @@ -416,7 +410,7 @@ function renderTable( }} data-testid={id} > - {headerRows.map(([, , nodeFn]) => nodeFn())} + {headerRows.map(([, , node]) => node)} {/* Show an all-column-span info message if it's set. */} {firstRowMessage && ( @@ -426,7 +420,7 @@ function renderTable( )} - {filteredRows.map(([, , nodeFn]) => nodeFn())} + {filteredRows.map(([, , node]) => node)} ); @@ -475,7 +469,7 @@ function renderVirtual( // so we pick the right header / first row message / actual row. let i = index; if (i < headerRows.length) { - return headerRows[i][2](); + return headerRows[i][2]; } i -= headerRows.length; if (firstRowMessage) { @@ -489,7 +483,7 @@ function renderVirtual( i -= 1; } // We pass in others, which has data-index, data-known-size, etc. - return filteredRows[i][2](); + return filteredRows[i][2]; }} totalCount={(headerRows.length || 0) + (firstRowMessage ? 1 : 0) + (filteredRows.length || 0)} /> From d6053efd7653d19d793c4da334b2196d57abdd94 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 20:57:50 -0500 Subject: [PATCH 12/24] Remove now unused refs. --- src/components/GridTable.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 8451f0753..8f7ab4e35 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -8,7 +8,6 @@ import React, { useContext, useEffect, useMemo, - useRef, useState, } from "react"; import { Link } from "react-router-dom"; @@ -330,11 +329,6 @@ export function GridTable[]>([]); - headerRowsRef.current = headerRows; - const filteredRowsRef = useRef[]>([]); - filteredRowsRef.current = filteredRows; - return renders[as](style, id, columns, headerRows, filteredRows, firstRowMessage, stickyHeader, xss); } From c30298e462d3643d9c3ee540daa4d04354b7ba4d Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 20:57:55 -0500 Subject: [PATCH 13/24] Add comments. --- src/components/GridTable.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 8f7ab4e35..ee0a73434 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -339,6 +339,7 @@ const renders: Record = { virtual: renderVirtual, }; +/** Renders as a CSS Grid, which is the default / most well-supported rendered. */ function renderCssGrid( style: GridStyle, id: string, @@ -484,7 +485,13 @@ function renderVirtual( ); } -// Use memoize to create a single component type for a given set of props. +/** + * Customizes the `List` element that react-virtuoso renders, to have our css grid logic. + * + * We wrap this in memoizeOne so that React.createElement sees a consistent/stable component + * identity, even though technically we have a different "component" per the given set of props + * (solely to capture as params that we can't pass through react-virtuoso's API as props). + */ const VirtualRoot = memoizeOne<(gs: GridStyle, columns: GridColumn[], id: string, xss: any) => Components["List"]>( (gs, columns, id, xss) => { return React.forwardRef(({ style, children }, ref) => { From adc0fc796a94d717bd98b1c204b779e23f24cd59 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Sun, 30 May 2021 21:01:03 -0500 Subject: [PATCH 14/24] Update sticky story. --- src/components/GridTable.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/GridTable.stories.tsx b/src/components/GridTable.stories.tsx index 507ba1ebc..034f01965 100644 --- a/src/components/GridTable.stories.tsx +++ b/src/components/GridTable.stories.tsx @@ -226,14 +226,14 @@ export function StickyHeader() { sort: false, }; return ( -
+
some other top of page content ({ kind: "data" as const, id: "1", name: "c", value: 1 })), + ...zeroTo(200).map((i) => ({ kind: "data" as const, id: `${i}`, name: `row ${i}`, value: i })), ]} />
From 4127afceefbab874240f7719810b8d7e074a6856 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Mon, 31 May 2021 10:37:59 -0500 Subject: [PATCH 15/24] Use our react-virtuoso fork until patch is upstream. --- package.json | 2 +- src/components/GridTable.tsx | 2 +- yarn.lock | 20 ++++++++++---------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 9b0d4949e..0fb181912 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "react-aria": "^3.6.0", "react-day-picker": "^7.4.10", "react-stately": "^3.5.0", - "react-virtuoso": "^1.8.6", + "@homebound/react-virtuoso": "^1.8.6", "tributejs": "^5.1.3", "trix": "^1.3.1", "react-popper": "^2.2.5", diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index ee0a73434..045201411 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -1,3 +1,4 @@ +import { Components, Virtuoso } from "@homebound/react-virtuoso"; import memoizeOne from "memoize-one"; import { Observer } from "mobx-react"; import React, { @@ -11,7 +12,6 @@ import React, { useState, } from "react"; import { Link } from "react-router-dom"; -import { Components, Virtuoso } from "react-virtuoso"; import { navLink } from "src/components/CssReset"; import { Icon } from "src/components/Icon"; import { Css, Margin, Only, Palette, Properties, px, Xss } from "src/Css"; diff --git a/yarn.lock b/yarn.lock index 3b04f2a12..3011cbbd6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1382,6 +1382,16 @@ resolved "https://registry.yarnpkg.com/@homebound/form-state/-/form-state-1.0.2.tgz#da581bf9951a7d6ce9036133246d17a367488a99" integrity sha512-1PfejczCNuENqAaM6m60k9CspmdXqXyhErakuSPPMUJi751++ss9V0ZTIhZ1faSrL/I7LUVvOSG36CMLb0lv2w== +"@homebound/react-virtuoso@^1.8.6": + version "1.8.6" + resolved "https://registry.yarnpkg.com/@homebound/react-virtuoso/-/react-virtuoso-1.8.6.tgz#6281518dd5d6496200312375e96b38b58108c949" + integrity sha512-DYFFmC+WMraFi9d6DiG90kp0eJt/57UCtxyl6FgIrO3tF665o0UUguL24/r5X9a7bFs6BJDZwlXnFjFK/utFWQ== + dependencies: + "@virtuoso.dev/react-urx" "^0.2.5" + "@virtuoso.dev/urx" "^0.2.5" + react-app-polyfill "^1.0.6" + resize-observer-polyfill "^1.5.1" + "@homebound/rtl-utils@^1.39.0": version "1.39.0" resolved "https://registry.yarnpkg.com/@homebound/rtl-utils/-/rtl-utils-1.39.0.tgz#5774d941cc2830dc474641167995e90f8c83f43c" @@ -13068,16 +13078,6 @@ react-textarea-autosize@^8.3.0: use-composed-ref "^1.0.0" use-latest "^1.0.0" -react-virtuoso@^1.8.6: - version "1.8.6" - resolved "https://registry.yarnpkg.com/react-virtuoso/-/react-virtuoso-1.8.6.tgz#dc7d79ee9e309c13700296ff65b04f6ebecdcbaa" - integrity sha512-WFSI4YzdyAFrr3CuZnQ8cmubGMJjgmgyuhx/OpWLls1uK9O43s+6DMr9oBXHAgvCDjS3Nd3vkIJAhsJBJzS1nQ== - dependencies: - "@virtuoso.dev/react-urx" "^0.2.5" - "@virtuoso.dev/urx" "^0.2.5" - react-app-polyfill "^1.0.6" - resize-observer-polyfill "^1.5.1" - react@^16.11.0, react@^16.12.0, react@^16.14.0, react@^16.8.3: version "16.14.0" resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" From 8a02e5560349e6104fe7d83fb84c5ba35e9dac21 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Mon, 31 May 2021 10:47:54 -0500 Subject: [PATCH 16/24] Remove console.log. --- src/components/GridTable.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 045201411..4edd56300 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -654,8 +654,6 @@ function GridRow(props: GridRowProps): ReactElement { ...others } = props; - console.log("GridRow rendering", row.id); - // We treat the "header" kind as special for "good defaults" styling const isHeader = row.kind === "header"; const rowStyle = rowStyles?.[row.kind]; From 208fe8bf1e05322cd0141a85a64571e38445883f Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Mon, 31 May 2021 10:49:50 -0500 Subject: [PATCH 17/24] Update snapshots. --- src/components/__snapshots__/GridTable.test.tsx.snap | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/__snapshots__/GridTable.test.tsx.snap b/src/components/__snapshots__/GridTable.test.tsx.snap index 2680ba041..8949c43da 100644 --- a/src/components/__snapshots__/GridTable.test.tsx.snap +++ b/src/components/__snapshots__/GridTable.test.tsx.snap @@ -6,15 +6,13 @@ exports[`GridTable renders 1`] = ` grid-template-columns: auto auto; } -.emotion-0>div+div>*, -.emotion-0>tbody>tr { +.emotion-0>div+div>* { border-color: rgba(201,201,201,1); border-top-style: solid; border-top-width: 1px; } -.emotion-0>div:nth-of-type(2)>*, -.emotion-0>tbody>tr:first-of-type { +.emotion-0>div:nth-of-type(2)>* { border-top-style: none; } From e2211d70494e8ae4d57c670d6b0e531f6b9893f6 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 1 Jun 2021 07:30:29 -0500 Subject: [PATCH 18/24] Bump back to react-virtuoso which was itemSize merged. --- package.json | 2 +- src/components/GridTable.tsx | 5 ++++- yarn.lock | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index 0fb181912..08b839453 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "react-aria": "^3.6.0", "react-day-picker": "^7.4.10", "react-stately": "^3.5.0", - "@homebound/react-virtuoso": "^1.8.6", + "react-virtuoso": "^1.9.0", "tributejs": "^5.1.3", "trix": "^1.3.1", "react-popper": "^2.2.5", diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 4edd56300..bdc0afaba 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -1,4 +1,3 @@ -import { Components, Virtuoso } from "@homebound/react-virtuoso"; import memoizeOne from "memoize-one"; import { Observer } from "mobx-react"; import React, { @@ -12,6 +11,7 @@ import React, { useState, } from "react"; import { Link } from "react-router-dom"; +import { Components, Virtuoso } from "react-virtuoso"; import { navLink } from "src/components/CssReset"; import { Icon } from "src/components/Icon"; import { Css, Margin, Only, Palette, Properties, px, Xss } from "src/Css"; @@ -459,6 +459,9 @@ function renderVirtual( (el.firstElementChild!.firstElementChild! as HTMLElement).offsetHeight} itemContent={(index) => { // We keep header and filter rows separate, but react-virtuoso is a flat list, // so we pick the right header / first row message / actual row. diff --git a/yarn.lock b/yarn.lock index 3011cbbd6..a7d019180 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1382,16 +1382,6 @@ resolved "https://registry.yarnpkg.com/@homebound/form-state/-/form-state-1.0.2.tgz#da581bf9951a7d6ce9036133246d17a367488a99" integrity sha512-1PfejczCNuENqAaM6m60k9CspmdXqXyhErakuSPPMUJi751++ss9V0ZTIhZ1faSrL/I7LUVvOSG36CMLb0lv2w== -"@homebound/react-virtuoso@^1.8.6": - version "1.8.6" - resolved "https://registry.yarnpkg.com/@homebound/react-virtuoso/-/react-virtuoso-1.8.6.tgz#6281518dd5d6496200312375e96b38b58108c949" - integrity sha512-DYFFmC+WMraFi9d6DiG90kp0eJt/57UCtxyl6FgIrO3tF665o0UUguL24/r5X9a7bFs6BJDZwlXnFjFK/utFWQ== - dependencies: - "@virtuoso.dev/react-urx" "^0.2.5" - "@virtuoso.dev/urx" "^0.2.5" - react-app-polyfill "^1.0.6" - resize-observer-polyfill "^1.5.1" - "@homebound/rtl-utils@^1.39.0": version "1.39.0" resolved "https://registry.yarnpkg.com/@homebound/rtl-utils/-/rtl-utils-1.39.0.tgz#5774d941cc2830dc474641167995e90f8c83f43c" @@ -13078,6 +13068,16 @@ react-textarea-autosize@^8.3.0: use-composed-ref "^1.0.0" use-latest "^1.0.0" +react-virtuoso@^1.9.0: + version "1.9.0" + resolved "https://registry.yarnpkg.com/react-virtuoso/-/react-virtuoso-1.9.0.tgz#5223d82b4b88021e73cd7cb671824e65537ce3ed" + integrity sha512-7ugCTy+zuKIplhhRLvOVHjyluLeB/BNBF2XwwJyKifsQNY/H565BV4yJYzejbmusdFGI7Dt8iJi1Zb9Nfj/pew== + dependencies: + "@virtuoso.dev/react-urx" "^0.2.5" + "@virtuoso.dev/urx" "^0.2.5" + react-app-polyfill "^1.0.6" + resize-observer-polyfill "^1.5.1" + react@^16.11.0, react@^16.12.0, react@^16.14.0, react@^16.8.3: version "16.14.0" resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" From e23d26257b045ca7766b67169e6b68766cb33143 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 1 Jun 2021 12:36:57 -0500 Subject: [PATCH 19/24] Remove stale comment. --- src/components/GridTable.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index bdc0afaba..187282570 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -480,7 +480,6 @@ function renderVirtual( } i -= 1; } - // We pass in others, which has data-index, data-known-size, etc. return filteredRows[i][2]; }} totalCount={(headerRows.length || 0) + (firstRowMessage ? 1 : 0) + (filteredRows.length || 0)} From a119c0ad0caaf346ee4c091dc7e87108b6730281 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 1 Jun 2021 12:37:07 -0500 Subject: [PATCH 20/24] Pin the firstRowMessage too b/c it's easy. --- src/components/GridTable.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 187282570..8a6cbb1bc 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -458,7 +458,8 @@ function renderVirtual( return ( (el.firstElementChild!.firstElementChild! as HTMLElement).offsetHeight} From 959cdfd81333a5a76d0ea81974504ad46ee7cf2c Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 1 Jun 2021 12:41:22 -0500 Subject: [PATCH 21/24] Remove link to resolved/patched issue. --- src/components/GridTable.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 8a6cbb1bc..16a08b380 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -433,9 +433,6 @@ function renderTable( * maybe due to [3] and they have no examples showing memoization, which is * concerning. * - * Note that technically I had to patch react-virtuoso, see [4], to support our - * usage of `display: contents` but hopefully it'll get patched soon. - * * react-virtuoso also seems like the most maintained (react-window is no * longer being actively worked on) and featureful library (like sticky headers), * so going with that for now. @@ -443,7 +440,6 @@ function renderTable( * [1]: https://github.com/bvaughn/react-window/issues/6 * [2]: https://github.com/tannerlinsley/react-virtual/issues/85 * [3]: https://github.com/tannerlinsley/react-virtual/issues/108 - * [4]: https://github.com/petyosi/react-virtuoso/issues/375 */ function renderVirtual( style: GridStyle, From 86f666eb70d340895660f7c829ac42148ac219a3 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 1 Jun 2021 12:43:27 -0500 Subject: [PATCH 22/24] Dedup copy/pasted gridTemplateColumns calc. --- src/components/GridTable.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index 16a08b380..ab638486a 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -350,10 +350,7 @@ function renderCssGrid( stickyHeader: boolean, xss: any, ): ReactElement { - const gridTemplateColumns = columns - // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string - .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) - .join(" "); + const gridTemplateColumns = calcGridColumns(columns); return (
[], id: s (gs, columns, id, xss) => { return React.forwardRef(({ style, children }, ref) => { // This re-renders each time we have new children in the view port - const gridTemplateColumns = columns - // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string - .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) - .join(" "); + const gridTemplateColumns = calcGridColumns(columns); return (
[], id: s }, ); +function calcGridColumns(columns: GridColumn[]): string { + return ( + columns + // Default to auto, but use `c.w` as a fr if numeric or else `c.w` as-if if a string + .map((c) => (typeof c.w === "string" ? c.w : c.w !== undefined ? `${c.w}fr` : "auto")) + .join(" ") + ); +} + /** * Given an ADT of type T, performs a look up and returns the type of kind K. * From dffbb147dc6263108222e761fcfd9b8aa216b69a Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 1 Jun 2021 12:45:08 -0500 Subject: [PATCH 23/24] Use Css.gtc and inline the vars. --- src/components/GridTable.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/GridTable.tsx b/src/components/GridTable.tsx index ab638486a..642a0b3dd 100644 --- a/src/components/GridTable.tsx +++ b/src/components/GridTable.tsx @@ -350,11 +350,10 @@ function renderCssGrid( stickyHeader: boolean, xss: any, ): ReactElement { - const gridTemplateColumns = calcGridColumns(columns); return (
*` 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. @@ -492,13 +491,12 @@ const VirtualRoot = memoizeOne<(gs: GridStyle, columns: GridColumn[], id: s (gs, columns, id, xss) => { return React.forwardRef(({ style, children }, ref) => { // This re-renders each time we have new children in the view port - const gridTemplateColumns = calcGridColumns(columns); return (
div` due to Item + itemContent both having divs ...Css.addIn("& > div + div > div > *", gs.betweenRowsCss).$, // Add `display:contents` to Item to flatten it like we do GridRow From 3ae470f54beac4ce4b0340097acff311314111fc Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 2 Jun 2021 19:31:20 -0500 Subject: [PATCH 24/24] Fix up after rebases. --- package.json | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/package.json b/package.json index 08b839453..4d841b925 100644 --- a/package.json +++ b/package.json @@ -40,19 +40,13 @@ "change-case": "^4.1.2", "date-fns": "^2.21.3", "framer-motion": "^4.1.11", - "@types/react-virtualized-auto-sizer": "^1.0.0", - "@types/react-window": "^1.8.3", - "change-case": "^4.1.2", - "date-fns": "^2.21.3", - "framer-motion": "^4.1.11", "memoize-one": "^5.2.1", "react-aria": "^3.6.0", "react-day-picker": "^7.4.10", + "react-popper": "^2.2.5", "react-stately": "^3.5.0", "react-virtuoso": "^1.9.0", "tributejs": "^5.1.3", - "trix": "^1.3.1", - "react-popper": "^2.2.5", "trix": "^1.3.1" }, "peerDependencies": {