diff --git a/src/components/Filters/SingleFilter.tsx b/src/components/Filters/SingleFilter.tsx index 76afc1bd5..f44f20049 100644 --- a/src/components/Filters/SingleFilter.tsx +++ b/src/components/Filters/SingleFilter.tsx @@ -37,7 +37,7 @@ class SingleFilter extends BaseFilter diff --git a/src/inputs/SelectField.stories.tsx b/src/inputs/SelectField.stories.tsx index 685ff7ad4..b19ffad7a 100644 --- a/src/inputs/SelectField.stories.tsx +++ b/src/inputs/SelectField.stories.tsx @@ -224,6 +224,7 @@ Contrast.args = { compact: true, contrast: true }; const loadTestOptions: TestOption[] = zeroTo(1000).map((i) => ({ id: String(i), name: `Project ${i}` })); export function PerfTest() { + const [loaded, setLoaded] = useState([]); const [selectedValue, setSelectedValue] = useState(loadTestOptions[2].id); return ( { - return new Promise((resolve) => { - // @ts-ignore - believes `options` should be of type `never[]` - setTimeout(() => resolve({ options: loadTestOptions }), 1500); - }); + await sleep(1500); + setLoaded(loadTestOptions); }, + options: loaded, }} onBlur={action("onBlur")} onFocus={action("onFocus")} @@ -248,6 +248,7 @@ export function PerfTest() { PerfTest.parameters = { chromatic: { disableSnapshot: true } }; export function LazyLoadStateFields() { + const [loaded, setLoaded] = useState([]); const [selectedValue, setSelectedValue] = useState(loadTestOptions[2].id); return ( <> @@ -257,13 +258,12 @@ export function LazyLoadStateFields() { onSelect={setSelectedValue} unsetLabel={"-"} options={{ - initial: [loadTestOptions.find((o) => o.id === selectedValue)!], + initial: loadTestOptions.find((o) => o.id === selectedValue)!, load: async () => { - return new Promise((resolve) => { - // @ts-ignore - believes `options` should be of type `never[]` - setTimeout(() => resolve({ options: loadTestOptions }), 1500); - }); + await sleep(1500); + setLoaded(loadTestOptions); }, + options: loaded, }} /> o.id === selectedValue)!], + initial: loadTestOptions.find((o) => o.id === selectedValue)!, load: async () => { - return new Promise((resolve) => { - // @ts-ignore - believes `options` should be of type `never[]` - setTimeout(() => resolve({ options: loadTestOptions }), 1500); - }); + await sleep(1500); + setLoaded(loadTestOptions); }, + options: loaded, }} /> @@ -287,21 +286,20 @@ export function LazyLoadStateFields() { LazyLoadStateFields.parameters = { chromatic: { disableSnapshot: true } }; export function LoadingState() { + const [loaded, setLoaded] = useState([]); const [selectedValue, setSelectedValue] = useState(loadTestOptions[2].id); - return ( { - return new Promise((resolve) => { - // @ts-ignore - believes `options` should be of type `never[]` - setTimeout(() => resolve({ options: loadTestOptions }), 5000); - }); + await sleep(5000); + setLoaded(loadTestOptions); }, + options: loadTestOptions, }} /> ); @@ -392,3 +390,5 @@ function TestSelectField( ); } + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); diff --git a/src/inputs/SelectField.test.tsx b/src/inputs/SelectField.test.tsx index 7abfe4354..3f6d0ce4a 100644 --- a/src/inputs/SelectField.test.tsx +++ b/src/inputs/SelectField.test.tsx @@ -181,16 +181,25 @@ describe("SelectFieldTest", () => { it("can load options via options prop callback", async () => { // Given a Select Field with options that are loaded via a callback - const r = await render( - ({ options }) }} - getOptionLabel={(o) => o.name} - getOptionValue={(o) => o.id} - data-testid="age" - />, - ); + function Test() { + const [loaded, setLoaded] = useState([]); + return ( + setLoaded(options), + options: loaded, + }} + onSelect={() => {}} + getOptionLabel={(o) => o.name} + getOptionValue={(o) => o.id} + data-testid="age" + /> + ); + } + const r = await render(); // When opening the menu click(r.age); // Then expect to see the initial option and loading state @@ -305,17 +314,25 @@ describe("SelectFieldTest", () => { it("can define and select 'unsetLabel' when options are lazily loaded", async () => { const onSelect = jest.fn(); // Given a Select Field with options that are loaded lazily - const r = await render( - ({ options: labelValueOptions }) }} - getOptionLabel={(o) => o.label} - getOptionValue={(o) => o.value} - onSelect={onSelect} - />, - ); + function Test() { + const [loaded, setLoaded] = useState([]); + return ( + setLoaded(labelValueOptions), + options: loaded, + }} + getOptionLabel={(o) => o.label} + getOptionValue={(o) => o.value} + onSelect={onSelect} + /> + ); + } + const r = await render(); // When we click the field to open the menu await clickAndWait(r.age); // The 'unset' option is in the menu and we select it @@ -442,11 +459,12 @@ describe("SelectFieldTest", () => { ); } - function TestMultipleSelectField( + function TestMultipleSelectField( props: Optional, "onSelect">, ): JSX.Element { const [selected, setSelected] = useState(props.value); const init = options.find((o) => o.id === selected) as O; + const [loaded, setLoaded] = useState([]); return ( <> @@ -455,13 +473,12 @@ describe("SelectFieldTest", () => { onSelect={setSelected} unsetLabel={"-"} options={{ - initial: [init], + initial: init, load: async () => { - return new Promise((resolve) => { - // @ts-ignore - believes `options` should be of type `never[]` - setTimeout(() => resolve({ options }), 1500); - }); + await sleep(1500); + setLoaded(props.options as O[]); }, + options: loaded, }} /> @@ -470,16 +487,17 @@ describe("SelectFieldTest", () => { onSelect={setSelected} unsetLabel={"-"} options={{ - initial: [init], + initial: init, load: async () => { - return new Promise((resolve) => { - // @ts-ignore - believes `options` should be of type `never[]` - setTimeout(() => resolve({ options }), 1500); - }); + await sleep(1500); + setLoaded(props.options as O[]); }, + options: loaded, }} /> ); } }); + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); diff --git a/src/inputs/SelectField.tsx b/src/inputs/SelectField.tsx index 3db9af6cc..066d0998f 100644 --- a/src/inputs/SelectField.tsx +++ b/src/inputs/SelectField.tsx @@ -1,3 +1,4 @@ +import { useMemo } from "react"; import { Value } from "src/inputs"; import { ComboBoxBase, ComboBoxBaseProps, unsetOption } from "src/inputs/internal/ComboBoxBase"; import { HasIdAndName, Optional } from "src/types"; @@ -35,14 +36,14 @@ export function SelectField( value, ...otherProps } = props; - + const values = useMemo(() => [value], [value]); return ( { // If the user used `unsetLabel`, then values will be `[undefined]` and options `[unsetOption]` if (values.length > 0 && options.length > 0) { diff --git a/src/inputs/internal/ComboBoxBase.tsx b/src/inputs/internal/ComboBoxBase.tsx index d5bae196d..caf819956 100644 --- a/src/inputs/internal/ComboBoxBase.tsx +++ b/src/inputs/internal/ComboBoxBase.tsx @@ -12,6 +12,7 @@ import { keyToValue, Value, valueToKey } from "src/inputs/Value"; import { BeamFocusableProps } from "src/interfaces"; import { areArraysEqual } from "src/utils"; +/** Base props for either `SelectField` or `MultiSelectField`. */ export interface ComboBoxBaseProps extends BeamFocusableProps, PresentationFieldProps { /** Renders `opt` in the dropdown menu, defaults to the `getOptionLabel` prop. `isUnsetOpt` is only defined for single SelectField */ getOptionMenuLabel?: (opt: O, isUnsetOpt?: boolean) => string | ReactNode; @@ -85,6 +86,9 @@ export function ComboBoxBase(props: ComboBoxBaseProps) disabledOptions, borderless, unsetLabel, + getOptionLabel: propOptionLabel, + getOptionValue: propOptionValue, + getOptionMenuLabel: propOptionMenuLabel, ...otherProps } = props; const labelStyle = otherProps.labelStyle ?? fieldProps?.labelStyle ?? "above"; @@ -93,25 +97,17 @@ export function ComboBoxBase(props: ComboBoxBaseProps) const maybeOptions = useMemo(() => initializeOptions(options, unsetLabel), [options, unsetLabel]); // Memoize the callback functions and handle the `unset` option if provided. const getOptionLabel = useCallback( - (o: O) => (unsetLabel && o === unsetOption ? unsetLabel : props.getOptionLabel(o)), - // TODO: validate this eslint-disable. It was automatically ignored as part of https://app.shortcut.com/homebound-team/story/40033/enable-react-hooks-exhaustive-deps-for-react-projects - // eslint-disable-next-line react-hooks/exhaustive-deps - [props.getOptionLabel, unsetLabel], + (o: O) => (unsetLabel && o === unsetOption ? unsetLabel : propOptionLabel(o)), + [propOptionLabel, unsetLabel], ); const getOptionValue = useCallback( - (o: O) => (unsetLabel && o === unsetOption ? (undefined as V) : props.getOptionValue(o)), - // TODO: validate this eslint-disable. It was automatically ignored as part of https://app.shortcut.com/homebound-team/story/40033/enable-react-hooks-exhaustive-deps-for-react-projects - // eslint-disable-next-line react-hooks/exhaustive-deps - [props.getOptionValue, unsetLabel], + (o: O) => (unsetLabel && o === unsetOption ? (undefined as V) : propOptionValue(o)), + [propOptionValue, unsetLabel], ); const getOptionMenuLabel = useCallback( (o: O) => - props.getOptionMenuLabel - ? props.getOptionMenuLabel(o, Boolean(unsetLabel) && o === unsetOption) - : getOptionLabel(o), - // TODO: validate this eslint-disable. It was automatically ignored as part of https://app.shortcut.com/homebound-team/story/40033/enable-react-hooks-exhaustive-deps-for-react-projects - // eslint-disable-next-line react-hooks/exhaustive-deps - [props.getOptionValue, unsetLabel, getOptionLabel], + propOptionMenuLabel ? propOptionMenuLabel(o, Boolean(unsetLabel) && o === unsetOption) : getOptionLabel(o), + [propOptionMenuLabel, unsetLabel, getOptionLabel], ); const { contains } = useFilter({ sensitivity: "base" }); @@ -119,7 +115,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) const isReadOnly = !!readOnly; const [fieldState, setFieldState] = useState>(() => { - const initOptions = Array.isArray(maybeOptions) ? maybeOptions : maybeOptions.initial; + const initOptions = Array.isArray(maybeOptions) ? maybeOptions : [maybeOptions.initial]; const selectedOptions = initOptions.filter((o) => values.includes(getOptionValue(o))); return { selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], @@ -218,15 +214,8 @@ export function ComboBoxBase(props: ComboBoxBaseProps) async function maybeInitLoad() { if (!Array.isArray(maybeOptions)) { setFieldState((prevState) => ({ ...prevState, optionsLoading: true })); - const loadedOptions = (await maybeOptions.load()).options; - // Ensure the `unset` option is prepended to the top of the list if `unsetLabel` was provided - const options = !unsetLabel ? loadedOptions : getOptionsWithUnset(unsetLabel, loadedOptions); - setFieldState((prevState) => ({ - ...prevState, - filteredOptions: options, - allOptions: options, - optionsLoading: false, - })); + await maybeOptions.load(); + setFieldState((prevState) => ({ ...prevState, optionsLoading: false })); } } @@ -324,20 +313,23 @@ export function ComboBoxBase(props: ComboBoxBaseProps) [values], ); + // When options are an array, then use them as-is. + // If options are an object, then use the `initial` array if the menu has not been opened + // Otherwise, use the current fieldState array options. + const maybeUpdatedOptions = Array.isArray(maybeOptions) + ? maybeOptions + : firstOpen.current === false && !fieldState.optionsLoading + ? maybeOptions.options + : maybeOptions.initial; + useEffect( () => { - // When options are an array, then use them as-is. - // If options are an object, then use the `initial` array if the menu has not been opened - // Otherwise, use the current fieldState array options. - const maybeUpdatedOptions = Array.isArray(maybeOptions) - ? maybeOptions - : firstOpen.current === false - ? fieldState.allOptions - : maybeOptions.initial; - - if (maybeUpdatedOptions !== fieldState.allOptions) { + // We leave `maybeOptions.initial` as a non-array so that it's stable, but now that we're inside the + // useEffect, array-ize it if needed. + const maybeUpdatedArray = Array.isArray(maybeUpdatedOptions) ? maybeUpdatedOptions : [maybeUpdatedOptions]; + if (maybeUpdatedArray !== fieldState.allOptions) { setFieldState((prevState) => { - const selectedOptions = maybeUpdatedOptions.filter((o) => values?.includes(getOptionValue(o))); + const selectedOptions = maybeUpdatedArray.filter((o) => values?.includes(getOptionValue(o))); return { ...prevState, selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], @@ -348,15 +340,16 @@ export function ComboBoxBase(props: ComboBoxBaseProps) ? nothingSelectedText : "", selectedOptions: selectedOptions, - filteredOptions: maybeUpdatedOptions, - allOptions: maybeUpdatedOptions, + filteredOptions: maybeUpdatedArray, + allOptions: maybeUpdatedArray, }; }); } }, - // TODO: validate this eslint-disable. It was automatically ignored as part of https://app.shortcut.com/homebound-team/story/40033/enable-react-hooks-exhaustive-deps-for-react-projects + // I started working on fixing this deps array, but seems like `getOptionLabel` & friends + // would very rarely be stable anyway, so going to hold off on further fixes for now... // eslint-disable-next-line react-hooks/exhaustive-deps - [maybeOptions], + [maybeUpdatedOptions, getOptionLabel, getOptionValue], ); // For the most part, the returned props contain `aria-*` and `id` attributes for accessibility purposes. @@ -454,7 +447,19 @@ type FieldState = { allOptions: O[]; optionsLoading: boolean; }; -export type OptionsOrLoad = O[] | { initial: O[]; load: () => Promise<{ options: O[] }> }; + +/** Allows lazy-loading select fields, which is useful for pages w/lots of fields the user may not actually use. */ +export type OptionsOrLoad = + | O[] + | { + /** The initial option to show before the user interacts with the dropdown. */ + initial: O; + /** Fired when the user interacts with the dropdown, to load the real options. */ + load: () => Promise; + /** The full list of options, after load() has been fired. */ + options: O[]; + }; + type UnsetOption = { id: undefined; name: string }; function getInputValue( @@ -474,18 +479,17 @@ export function initializeOptions(options: OptionsOrLoad, unsetLabel: stri if (!unsetLabel) { return options; } - if (Array.isArray(options)) { return getOptionsWithUnset(unsetLabel, options); } - - return { ...options, initial: getOptionsWithUnset(unsetLabel, options.initial) }; + return { ...options, options: getOptionsWithUnset(unsetLabel, options.options) }; } function getOptionsWithUnset(unsetLabel: string, options: O[]): O[] { return [unsetOption as unknown as O, ...options]; } +/** A marker option to automatically add an "Unset" option to the start of options. */ export const unsetOption = {}; export function disabledOptionToKeyedTuple(