Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

POC of react-window to virutalize selectfield options #87

Closed
wants to merge 1 commit into from
Closed

POC of react-window to virutalize selectfield options #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,16 @@
"dependencies": {
"@react-aria/combobox": "3.0.0-beta.1",
"@react-aria/numberfield": "3.0.0",
"@react-types/shared": "^3.6.0",
"@react-stately/combobox": "^3.0.0-beta.1",
"@react-stately/numberfield": "^3.0.0",
"@react-types/shared": "^3.6.0",
"change-case": "^4.1.2",
"date-fns": "^2.21.3",
"framer-motion": "^4.1.11",
"react-aria": "^3.6.0",
"react-day-picker": "^7.4.10",
"react-stately": "^3.5.0",
"framer-motion": "^4.1.11"
"react-window": "^1.8.6"
},
"peerDependencies": {
"@emotion/react": ">=11",
Expand Down Expand Up @@ -82,6 +83,7 @@
"@types/react": "^17.0.5",
"@types/react-dom": "^16.9.11",
"@types/react-router-dom": "^5.1.7",
"@types/react-window": "^1.8.3",
"@types/tinycolor2": "^1.4.2",
"@typescript-eslint/eslint-plugin": "^4.17.0",
"@typescript-eslint/parser": "^4.17.0",
Expand Down
9 changes: 5 additions & 4 deletions src/components/SelectField.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Meta } from "@storybook/react";
import { Key, useState } from "react";
import { HasIdAndName, Icon, Icons, Optional, SelectField, SelectFieldProps } from "src/components";
import { zeroTo } from "src/components/GridTable.stories";
import { Css } from "src/Css";

export default {
Expand Down Expand Up @@ -28,10 +29,10 @@ const optionsWithNumericIds: TestOption[] = [
{ id: 4, name: "Four" },
];

// export function LotsOfOptions() {
// const options: TestOption[] = zeroTo(1000).map((i) => ({ id: i, name: `Project ${i}` }));
// return <TestSelectField label="Project" value={options[2].id} options={options} />;
// }
export function LotsOfOptions() {
const options: TestOption[] = zeroTo(1000).map((i) => ({ id: i, name: `Project ${i}` }));
return <TestSelectField label="Project" value={options[2].id} options={options} />;
}

export function SelectFields() {
return (
Expand Down
44 changes: 37 additions & 7 deletions src/components/SelectField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@ import { mergeProps } from "@react-aria/utils";
import { Item } from "@react-stately/collections";
import { ComboBoxState, useComboBoxState } from "@react-stately/combobox";
import { CollectionChildren, Node } from "@react-types/shared";
import React, { Fragment, InputHTMLAttributes, Key, MutableRefObject, ReactNode, useRef, useState } from "react";
import React, {
CSSProperties,
Fragment,
InputHTMLAttributes,
Key,
MutableRefObject,
ReactNode,
useRef,
useState,
} from "react";
import {
OverlayContainer,
useButton,
Expand All @@ -15,6 +24,7 @@ import {
useOverlay,
useOverlayPosition,
} from "react-aria";
import { VariableSizeList } from "react-window";
import { ErrorMessage } from "src/components/ErrorMessage";
import { HelperText } from "src/components/HelperText";
import { Icon } from "src/components/Icon";
Expand Down Expand Up @@ -350,27 +360,46 @@ function ListBoxPopup<T extends object>(props: ListBoxPopupProps<T>) {
width: getFieldWidth(compact),
};

const Row = ({ data, index, style }: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be a top-level function, b/c as is the "Row function" will get re-created on each re-render of ListBoxPopup and so look like a brand new component to react. ...I think. Basing this assertion on this ticket that I wandered by:

bvaughn/react-window#302

"Inline item renderers incur a high cost. Because their "type" (the function definition) gets recreated each time the parent component renders, React deeply unmounts and remounts their rendered tree. This means that docs need to teach people not to use them even though they're often more convenient."

const item = data.items[index];
return <Option item={item} state={state} style={style} key={item.key} />;
};

return (
<OverlayContainer>
<div {...{ ...overlayProps, ...positionProps }} ref={popoverRef}>
<ul
css={{
...Css.mtPx(4).bgWhite.br4.w100.bshBasic.$,
...Css.mtPx(4).bgWhite.br4.w100.bshBasic.overflowAuto.maxh("400px").$,
"&:hover": Css.bshHover.$,
}}
ref={listBoxRef}
{...listBoxProps}
>
{[...state.collection].map((item) => (
<Option key={item.key} item={item} state={state} />
))}
<VariableSizeList
itemData={{ items: [...state.collection], state }}
itemSize={() => 42}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have this figure out the size for us dynamically based on option itself... Whether that is based on the MenuItem type (if we fix those heights), or doing a render of the item to figure out its height.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When it figures it out for us, does it just look at the 1st item and assume that is the height of all items in the list?

...I assume that is what it's doing, but AFAICT a core assumption of the virtualized approach is known/consistent height across all items?

height={Math.min(42 * state.collection.size, 400)}
itemCount={state.collection.size}
Copy link
Contributor

@stephenh stephenh May 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, it seems like with itemCount and itemSize, it could figure out height ... ah sure but you're adding in a min of 400...

width={getFieldWidth(compact)}
>
{Row}
</VariableSizeList>
</ul>
</div>
</OverlayContainer>
);
}

function Option<T extends object>({ item, state }: { item: Node<T>; state: ComboBoxState<T> }) {
function Option<T extends object>({
item,
state,
style,
}: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw once the destructure-in-params starts to wrap like this, I think the props: ... and const { ... } = props approach is cleaner.

item: Node<T>;
state: ComboBoxState<T>;
style: CSSProperties;
}) {
const ref = useRef<HTMLLIElement>(null);
const isDisabled = state.disabledKeys.has(item.key);
const isSelected = state.selectionManager.isSelected(item.key);
Expand Down Expand Up @@ -399,10 +428,11 @@ function Option<T extends object>({ item, state }: { item: Node<T>; state: Combo
{...optionProps}
{...hoverProps}
ref={ref as any}
style={style}
css={{
...Css.df.itemsCenter.justifyBetween.py1.px2.mh("42px").cursorPointer.gray900.sm.$,
...(isHovered ? Css.bgGray100.$ : {}),
...(isFocused ? Css.add("boxShadow", `0 0 0 1px ${Palette.LightBlue700}`).$ : {}),
...(isFocused ? Css.add("boxShadow", `inset 0 0 0 1px ${Palette.LightBlue700}`).$ : {}),
}}
>
{item.rendered}
Expand Down
30 changes: 25 additions & 5 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4010,6 +4010,13 @@
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"
Expand Down Expand Up @@ -8515,11 +8522,6 @@ he@^1.2.0:
resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f"
integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==

hey-listen@^1.0.8:
version "1.0.8"
resolved "https://registry.yarnpkg.com/hey-listen/-/hey-listen-1.0.8.tgz#8e59561ff724908de1aa924ed6ecc84a56a9aa68"
integrity sha512-COpmrF2NOg4TBWUJ5UVyaCU2A88wEMkUPK4hNqyCkqHbxT92BbvfjoSozkAIIm6XhicGlJHhFdullInrdhwU8Q==

header-case@^2.0.4:
version "2.0.4"
resolved "https://registry.yarnpkg.com/header-case/-/header-case-2.0.4.tgz#5a42e63b55177349cf405beb8d775acabb92c063"
Expand All @@ -8528,6 +8530,11 @@ header-case@^2.0.4:
capital-case "^1.0.4"
tslib "^2.0.3"

hey-listen@^1.0.8:
version "1.0.8"
resolved "https://registry.yarnpkg.com/hey-listen/-/hey-listen-1.0.8.tgz#8e59561ff724908de1aa924ed6ecc84a56a9aa68"
integrity sha512-COpmrF2NOg4TBWUJ5UVyaCU2A88wEMkUPK4hNqyCkqHbxT92BbvfjoSozkAIIm6XhicGlJHhFdullInrdhwU8Q==

highlight.js@^10.1.1, highlight.js@~10.7.0:
version "10.7.2"
resolved "https://registry.yarnpkg.com/highlight.js/-/highlight.js-10.7.2.tgz#89319b861edc66c48854ed1e6da21ea89f847360"
Expand Down Expand Up @@ -10756,6 +10763,11 @@ memfs@^3.1.2:
dependencies:
fs-monkey "1.0.3"

"memoize-one@>=3.1.1 <6":
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"
Expand Down Expand Up @@ -13020,6 +13032,14 @@ react-textarea-autosize@^8.3.0:
use-composed-ref "^1.0.0"
use-latest "^1.0.0"

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"
Expand Down