From 20796756f1006655bc658eb05d8dc7f7eaf39222 Mon Sep 17 00:00:00 2001 From: Brandon Date: Tue, 10 Oct 2023 11:31:04 -0400 Subject: [PATCH 01/14] feat: Add Banner component (#960) --- src/components/Banner.test.tsx | 23 +++++++++++++ src/components/Banner.tsx | 48 +++++++++++++++++++++++++++ src/components/Toast/Toast.tsx | 44 ++---------------------- src/components/Toast/ToastContext.tsx | 7 ++-- src/components/index.ts | 2 ++ 5 files changed, 77 insertions(+), 47 deletions(-) create mode 100644 src/components/Banner.test.tsx create mode 100644 src/components/Banner.tsx diff --git a/src/components/Banner.test.tsx b/src/components/Banner.test.tsx new file mode 100644 index 000000000..e7edf7301 --- /dev/null +++ b/src/components/Banner.test.tsx @@ -0,0 +1,23 @@ +import { Banner } from "src"; +import { click, render } from "src/utils/rtl"; + +describe(Banner, () => { + it("should render", async () => { + // Given the Banner with a message and no onClose callback + const r = await render(); + // Then the banner should be visible + expect(r.banner_message).toHaveTextContent("Banner message"); + // And there should be no close button + expect(r.query.banner_close).not.toBeInTheDocument(); + }); + + it("should trigger onClose", async () => { + const onClose = jest.fn(); + // Given the Banner with a message and an onClose callback + const r = await render(); + // When clicking on the close button + click(r.banner_close); + // Then the onClose callback should be called + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/components/Banner.tsx b/src/components/Banner.tsx new file mode 100644 index 000000000..ac824b5a1 --- /dev/null +++ b/src/components/Banner.tsx @@ -0,0 +1,48 @@ +import { ReactNode } from "react"; +import { Icon, IconKey } from "src/components/Icon"; +import { IconButton } from "src/components/IconButton"; +import { Css, Palette, Properties } from "src/Css"; +import { useTestIds } from "src/utils"; + +export interface BannerProps { + type: BannerTypes; + message: ReactNode; + onClose?: VoidFunction; +} + +export function Banner(props: BannerProps) { + const { type, message, onClose = false, ...others } = props; + const tid = useTestIds(others, "banner"); + return ( +
+ + + + + {message} + + {onClose && ( + + + + )} +
+ ); +} +const typeToIcon: Record = { + success: "checkCircle", + info: "infoCircle", + warning: "error", + alert: "errorCircle", + error: "xCircle", +}; + +const variantStyles: Record = { + success: Css.bgGreen100.gray900.$, + info: Css.bgBlue100.gray900.$, + warning: Css.bgYellow200.gray900.$, + alert: Css.bgGray200.gray900.$, + error: Css.bgRed100.gray900.$, +}; + +export type BannerTypes = "error" | "warning" | "success" | "info" | "alert"; diff --git a/src/components/Toast/Toast.tsx b/src/components/Toast/Toast.tsx index a8c75c088..2d161259b 100644 --- a/src/components/Toast/Toast.tsx +++ b/src/components/Toast/Toast.tsx @@ -1,49 +1,9 @@ -import { Icon, IconKey } from "src/components/Icon"; -import { Css, Palette, Properties } from "src/Css"; +import { Banner } from "src/components"; import { useTestIds } from "src/utils"; -import { IconButton } from "../IconButton"; import { useToastContext } from "./ToastContext"; export function Toast() { const { setNotice, notice } = useToastContext(); const tid = useTestIds({}, "toast"); - return ( - <> - {notice && ( -
- - - - - {notice.message} - - - setNotice(undefined)} {...tid.close} color={Palette.Gray900} /> - -
- )} - - ); + return <>{notice && setNotice(undefined)} />}; } - -const typeToIcon: Record = { - success: "checkCircle", - info: "infoCircle", - warning: "error", - alert: "errorCircle", - error: "xCircle", -}; - -const variantStyles: Record = { - success: Css.bgGreen100.gray900.$, - info: Css.bgBlue100.gray900.$, - warning: Css.bgYellow200.gray900.$, - alert: Css.bgGray200.gray900.$, - error: Css.bgRed100.gray900.$, -}; - -export type ToastTypes = "error" | "warning" | "success" | "info" | "alert"; diff --git a/src/components/Toast/ToastContext.tsx b/src/components/Toast/ToastContext.tsx index 8d3d0343f..a9ab38ba9 100644 --- a/src/components/Toast/ToastContext.tsx +++ b/src/components/Toast/ToastContext.tsx @@ -1,10 +1,7 @@ import React, { createContext, ReactNode, useContext, useMemo, useState } from "react"; -import { ToastTypes } from "./Toast"; +import { BannerProps } from "src/components"; -export interface ToastNoticeProps { - type: ToastTypes; - message: ReactNode; -} +export interface ToastNoticeProps extends Omit {} export type ToastContextProps = { notice: ToastNoticeProps | undefined; diff --git a/src/components/index.ts b/src/components/index.ts index 35d5d9e5c..86aeb029f 100644 --- a/src/components/index.ts +++ b/src/components/index.ts @@ -8,6 +8,8 @@ export * from "./Accordion"; export * from "./AccordionList"; export * from "./AutoSaveIndicator"; export * from "./Avatar"; +export * from "./Banner"; +export type { BannerProps } from "./Banner"; export { BeamProvider } from "./BeamContext"; export * from "./Button"; export * from "./ButtonDatePicker"; From e9851ee7dd37f201263c12330d20a9f40008887b Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Tue, 10 Oct 2023 15:34:15 +0000 Subject: [PATCH 02/14] chore(release): 2.319.0 [skip ci] ## [2.319.0](https://github.com/homebound-team/beam/compare/v2.318.4...v2.319.0) (2023-10-10) ### Features * Add Banner component ([#960](https://github.com/homebound-team/beam/issues/960)) ([2079675](https://github.com/homebound-team/beam/commit/20796756f1006655bc658eb05d8dc7f7eaf39222)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f1acd38bf..4001d1164 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.318.4", + "version": "2.319.0", "author": "Homebound", "license": "MIT", "main": "dist/index.js", From a089daa22a906ea4f3b3a1c2817efe250f7c6e0c Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Tue, 10 Oct 2023 11:43:16 -0500 Subject: [PATCH 03/14] fix: add pointer events none to scroll shadows (#961) --- src/components/ScrollShadows.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ScrollShadows.tsx b/src/components/ScrollShadows.tsx index 205a43cec..4ca25ae32 100644 --- a/src/components/ScrollShadows.tsx +++ b/src/components/ScrollShadows.tsx @@ -29,7 +29,7 @@ export function ScrollShadows(props: ScrollShadowsProps) { // The shadow styles will rarely every change. Memoize them to avoid recomputing them when we don't have to. const [startShadowStyles, endShadowStyles] = useMemo(() => { const transparentBgColor = bgColor.replace(/,1\)$/, ",0)"); - const commonStyles = Css.absolute.z3.$; + const commonStyles = Css.absolute.z3.add({ pointerEvents: "none" }).$; const startShadowStyles = !horizontal ? Css.top0.left0.right0.hPx(40).$ : Css.left0.top0.bottom0.wPx(25).$; const endShadowStyles = !horizontal ? Css.bottom0.left0.right0.hPx(40).$ : Css.right0.top0.bottom0.wPx(25).$; const startGradient = `linear-gradient(${!horizontal ? 180 : 90}deg, ${bgColor} 0%, ${transparentBgColor} 92%);`; From 520794fe662a501bda776f957bb96978e708b3ec Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Tue, 10 Oct 2023 16:46:56 +0000 Subject: [PATCH 04/14] chore(release): 2.319.1 [skip ci] ## [2.319.1](https://github.com/homebound-team/beam/compare/v2.319.0...v2.319.1) (2023-10-10) ### Bug Fixes * add pointer events none to scroll shadows ([#961](https://github.com/homebound-team/beam/issues/961)) ([a089daa](https://github.com/homebound-team/beam/commit/a089daa22a906ea4f3b3a1c2817efe250f7c6e0c)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4001d1164..2c5e3c838 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.319.0", + "version": "2.319.1", "author": "Homebound", "license": "MIT", "main": "dist/index.js", From e919851c53bb957c5430d85fc616eafd9fd26a65 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 10 Oct 2023 15:42:34 -0500 Subject: [PATCH 05/14] feat: Allow lazy-loaded SelectFields to change their options. (#962) * feat: Allow lazy-loaded SelectFields to change their options. I.e. if a variable in the GQL query changed, after we've done the initial load. * Allow initial to be undefined. * Add asArray, allow options to be undefined. * Rename initial to current. * Fix eslint. --- src/components/Filters/SingleFilter.tsx | 2 +- src/inputs/SelectField.stories.tsx | 42 ++++---- src/inputs/SelectField.test.tsx | 82 ++++++++++------ src/inputs/SelectField.tsx | 5 +- .../TreeSelectField.stories.tsx | 2 +- .../TreeSelectField/TreeSelectField.test.tsx | 2 +- .../TreeSelectField/TreeSelectField.tsx | 2 +- src/inputs/TreeSelectField/utils.ts | 2 +- src/inputs/internal/ComboBoxBase.tsx | 98 ++++++++++--------- 9 files changed, 132 insertions(+), 105 deletions(-) diff --git a/src/components/Filters/SingleFilter.tsx b/src/components/Filters/SingleFilter.tsx index 76afc1bd5..9174c8e8b 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..3591acd00 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)!], + current: 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)!], + current: 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..fdec47412 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], + current: 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], + current: 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/TreeSelectField/TreeSelectField.stories.tsx b/src/inputs/TreeSelectField/TreeSelectField.stories.tsx index 341b1e988..9d6efbbf5 100644 --- a/src/inputs/TreeSelectField/TreeSelectField.stories.tsx +++ b/src/inputs/TreeSelectField/TreeSelectField.stories.tsx @@ -187,7 +187,7 @@ export function AsyncOptions() { { return new Promise((resolve) => { // @ts-ignore - believes `options` should be of type `never[]` diff --git a/src/inputs/TreeSelectField/TreeSelectField.test.tsx b/src/inputs/TreeSelectField/TreeSelectField.test.tsx index fb4bf21b3..1e6d254b3 100644 --- a/src/inputs/TreeSelectField/TreeSelectField.test.tsx +++ b/src/inputs/TreeSelectField/TreeSelectField.test.tsx @@ -132,7 +132,7 @@ describe(TreeSelectField, () => { const r = await render( ({ options }) }} + options={{ current: initialOption, load: async () => ({ options }) }} label="Favorite League" values={[]} getOptionValue={(o) => o.id} diff --git a/src/inputs/TreeSelectField/TreeSelectField.tsx b/src/inputs/TreeSelectField/TreeSelectField.tsx index 5346ac985..0139a7cbd 100644 --- a/src/inputs/TreeSelectField/TreeSelectField.tsx +++ b/src/inputs/TreeSelectField/TreeSelectField.tsx @@ -147,7 +147,7 @@ function TreeSelectFieldBase(props: TreeSelectFieldProps = { option: NestedOption; parents: NestedOption[] }; export type NestedOption = O & { children?: NestedOption[] }; export type NestedOptionsOrLoad = | NestedOption[] - | { initial: NestedOption[]; load: () => Promise<{ options: NestedOption[] }> }; + | { current: NestedOption[]; load: () => Promise<{ options: NestedOption[] }> }; export type LeveledOption = [NestedOption, number]; export type TreeFieldState = { diff --git a/src/inputs/internal/ComboBoxBase.tsx b/src/inputs/internal/ComboBoxBase.tsx index d5bae196d..b64d9d4ee 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 : asArray(maybeOptions.current); 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.current; + 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 = asArray(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. */ + current: O | undefined; + /** 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[] | undefined; + }; + 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]; +function getOptionsWithUnset(unsetLabel: string, options: O[] | undefined): O[] { + return [unsetOption as unknown as O, ...(options ? options : [])]; } +/** A marker option to automatically add an "Unset" option to the start of options. */ export const unsetOption = {}; export function disabledOptionToKeyedTuple( @@ -497,3 +501,7 @@ export function disabledOptionToKeyedTuple( return [valueToKey(disabledOption), undefined]; } } + +function asArray(arrayOrElement: E[] | E | undefined): E[] { + return Array.isArray(arrayOrElement) ? arrayOrElement : arrayOrElement ? [arrayOrElement] : []; +} From 7d730982bd8557a357c47420ab89c1fdf72a39f5 Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Tue, 10 Oct 2023 20:45:29 +0000 Subject: [PATCH 06/14] chore(release): 2.320.0 [skip ci] ## [2.320.0](https://github.com/homebound-team/beam/compare/v2.319.1...v2.320.0) (2023-10-10) ### Features * Allow lazy-loaded SelectFields to change their options. ([#962](https://github.com/homebound-team/beam/issues/962)) ([e919851](https://github.com/homebound-team/beam/commit/e919851c53bb957c5430d85fc616eafd9fd26a65)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2c5e3c838..a13fdc627 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.319.1", + "version": "2.320.0", "author": "Homebound", "license": "MIT", "main": "dist/index.js", From 9c138a2b7ff228fae2913444b856458b9f79616c Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 10 Oct 2023 18:53:13 -0500 Subject: [PATCH 07/14] fix: Fix the unset label getting dropped. (#963) * fix: Fix the unset label getting dropped. * Refactor. --- src/inputs/SelectField.test.tsx | 27 +++++-- src/inputs/internal/ComboBoxBase.tsx | 117 ++++++++++++++------------- 2 files changed, 85 insertions(+), 59 deletions(-) diff --git a/src/inputs/SelectField.test.tsx b/src/inputs/SelectField.test.tsx index fdec47412..aa23677af 100644 --- a/src/inputs/SelectField.test.tsx +++ b/src/inputs/SelectField.test.tsx @@ -189,7 +189,10 @@ describe("SelectFieldTest", () => { value="1" options={{ current: options[0], - load: async () => setLoaded(options), + load: async () => { + await sleep(0); + setLoaded(options); + }, options: loaded, }} onSelect={() => {}} @@ -344,13 +347,27 @@ describe("SelectFieldTest", () => { it("can initially be set to the 'unsetLabel' option", async () => { // Given a Select Field with the value set to `undefined` const r = await render( - {}} + />, + ); + // The input value will be set to the `unsetLabel` + expect(r.age).toHaveValue("None"); + }); + + it("can initially be set to the 'unsetLabel' option when lazy loading options", async () => { + // Given a Select Field with the value set to `undefined` + const r = await render( + label="Age" value={undefined} unsetLabel="None" - options={labelValueOptions} - getOptionLabel={(o) => o.label} - getOptionValue={(o) => o.value} + options={{ current: undefined, load: async () => {}, options: undefined }} + onSelect={() => {}} />, ); // The input value will be set to the `unsetLabel` diff --git a/src/inputs/internal/ComboBoxBase.tsx b/src/inputs/internal/ComboBoxBase.tsx index b64d9d4ee..0b0b84015 100644 --- a/src/inputs/internal/ComboBoxBase.tsx +++ b/src/inputs/internal/ComboBoxBase.tsx @@ -78,7 +78,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) disabled, readOnly, onSelect, - options, + options: propOptions, multiselect = false, values = [], nothingSelectedText = "", @@ -93,8 +93,6 @@ export function ComboBoxBase(props: ComboBoxBaseProps) } = props; const labelStyle = otherProps.labelStyle ?? fieldProps?.labelStyle ?? "above"; - // Call `initializeOptions` to prepend the `unset` option if the `unsetLabel` was provided. - 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 : propOptionLabel(o)), @@ -110,24 +108,33 @@ export function ComboBoxBase(props: ComboBoxBaseProps) [propOptionMenuLabel, unsetLabel, getOptionLabel], ); + // Call `initializeOptions` to prepend the `unset` option if the `unsetLabel` was provided. + const options = useMemo( + () => initializeOptions(propOptions, getOptionValue, unsetLabel), + // If the caller is using { current, load, options }, memoize on only `current` and `options` values. + // ...and don't bother on memoizing on getOptionValue b/c it's basically always a lambda + // eslint-disable-next-line react-hooks/exhaustive-deps + Array.isArray(propOptions) ? [propOptions, unsetLabel] : [propOptions.current, propOptions.options, unsetLabel], + ); + const { contains } = useFilter({ sensitivity: "base" }); const isDisabled = !!disabled; const isReadOnly = !!readOnly; + // Do a one-time initialize of fieldState const [fieldState, setFieldState] = useState>(() => { - const initOptions = Array.isArray(maybeOptions) ? maybeOptions : asArray(maybeOptions.current); - const selectedOptions = initOptions.filter((o) => values.includes(getOptionValue(o))); + const selectedOptions = options.filter((o) => values.includes(getOptionValue(o))); return { selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], inputValue: getInputValue( - initOptions.filter((o) => values?.includes(getOptionValue(o))), + options.filter((o) => values?.includes(getOptionValue(o))), getOptionLabel, multiselect, nothingSelectedText, ), - filteredOptions: initOptions, - allOptions: initOptions, - selectedOptions: selectedOptions, + filteredOptions: options, + allOptions: options, + selectedOptions, optionsLoading: false, }; }); @@ -212,9 +219,9 @@ export function ComboBoxBase(props: ComboBoxBaseProps) } async function maybeInitLoad() { - if (!Array.isArray(maybeOptions)) { + if (!Array.isArray(propOptions)) { setFieldState((prevState) => ({ ...prevState, optionsLoading: true })); - await maybeOptions.load(); + await propOptions.load(); setFieldState((prevState) => ({ ...prevState, optionsLoading: false })); } } @@ -313,43 +320,30 @@ 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.current; - + // Re-sync fieldState.allOptions useEffect( () => { - // 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 = asArray(maybeUpdatedOptions); - if (maybeUpdatedArray !== fieldState.allOptions) { - setFieldState((prevState) => { - const selectedOptions = maybeUpdatedArray.filter((o) => values?.includes(getOptionValue(o))); - return { - ...prevState, - selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], - inputValue: - selectedOptions.length === 1 - ? getOptionLabel(selectedOptions[0]) - : multiselect && selectedOptions.length === 0 - ? nothingSelectedText - : "", - selectedOptions: selectedOptions, - filteredOptions: maybeUpdatedArray, - allOptions: maybeUpdatedArray, - }; - }); - } + setFieldState((prevState) => { + const selectedOptions = options.filter((o) => values?.includes(getOptionValue(o))); + return { + ...prevState, + selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], + inputValue: + selectedOptions.length === 1 + ? getOptionLabel(selectedOptions[0]) + : multiselect && selectedOptions.length === 0 + ? nothingSelectedText + : "", + selectedOptions: selectedOptions, + filteredOptions: options, + allOptions: options, + }; + }); }, - // 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... + // We're primarily only re-setting `allOptions`, and so recalc selected as well, but we don't + // want to depend on values/etc., b/c we'll defer to their useEffects to update their state // eslint-disable-next-line react-hooks/exhaustive-deps - [maybeUpdatedOptions, getOptionLabel, getOptionValue], + [options], ); // For the most part, the returned props contain `aria-*` and `id` attributes for accessibility purposes. @@ -475,18 +469,33 @@ function getInputValue( : ""; } -export function initializeOptions(options: OptionsOrLoad, unsetLabel: string | undefined): OptionsOrLoad { - if (!unsetLabel) { - return options; +/** Transforms/simplifies `optionsOrLoad` into just options, with unsetLabel maybe added. */ +export function initializeOptions( + optionsOrLoad: OptionsOrLoad, + getOptionValue: (opt: O) => V, + unsetLabel: string | undefined, +): O[] { + const opts: O[] = []; + if (unsetLabel) { + opts.push(unsetOption as unknown as O); } - if (Array.isArray(options)) { - return getOptionsWithUnset(unsetLabel, options); + if (Array.isArray(optionsOrLoad)) { + opts.push(...optionsOrLoad); + } else { + const { options, current } = optionsOrLoad; + if (options) { + opts.push(...options); + } + // Even if the SelectField has lazy-loaded options, make sure the current value is really in there + if (current) { + const value = getOptionValue(current); + const found = options && options.find((o) => getOptionValue(o) === value); + if (!found) { + opts.push(current); + } + } } - return { ...options, options: getOptionsWithUnset(unsetLabel, options.options) }; -} - -function getOptionsWithUnset(unsetLabel: string, options: O[] | undefined): O[] { - return [unsetOption as unknown as O, ...(options ? options : [])]; + return opts; } /** A marker option to automatically add an "Unset" option to the start of options. */ From 326a364dce067b12baae2d27ebe3d468cafe2561 Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Tue, 10 Oct 2023 23:56:20 +0000 Subject: [PATCH 08/14] chore(release): 2.320.1 [skip ci] ## [2.320.1](https://github.com/homebound-team/beam/compare/v2.320.0...v2.320.1) (2023-10-10) ### Bug Fixes * Fix the unset label getting dropped. ([#963](https://github.com/homebound-team/beam/issues/963)) ([9c138a2](https://github.com/homebound-team/beam/commit/9c138a2b7ff228fae2913444b856458b9f79616c)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a13fdc627..295a2734b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.320.0", + "version": "2.320.1", "author": "Homebound", "license": "MIT", "main": "dist/index.js", From a26a4b16fd181c2105cb460c25972e456fc42cfb Mon Sep 17 00:00:00 2001 From: Jonnathan Charpentier <92122357+JonnCharpentier@users.noreply.github.com> Date: Tue, 10 Oct 2023 21:30:19 -0600 Subject: [PATCH 09/14] feat: new icon (#959) Co-authored-by: JonnCh --- src/components/Icon.stories.tsx | 1 + src/components/Icon.tsx | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/components/Icon.stories.tsx b/src/components/Icon.stories.tsx index 6fb938020..92758a847 100644 --- a/src/components/Icon.stories.tsx +++ b/src/components/Icon.stories.tsx @@ -103,6 +103,7 @@ export const Icon = (props: IconProps) => { "openBook", ]; const miscIcons: IconProps["icon"][] = [ + "inbox", "dollar", "userCircle", "calendar", diff --git a/src/components/Icon.tsx b/src/components/Icon.tsx index e61adbfed..0b235c7eb 100644 --- a/src/components/Icon.tsx +++ b/src/components/Icon.tsx @@ -368,6 +368,9 @@ export const Icons = { ), // Misc + inbox: ( + + ), criticalPath: ( ), From aaf52aff6f95cf189d40cc197b8b35711b0dde20 Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Wed, 11 Oct 2023 03:33:23 +0000 Subject: [PATCH 10/14] chore(release): 2.321.0 [skip ci] ## [2.321.0](https://github.com/homebound-team/beam/compare/v2.320.1...v2.321.0) (2023-10-11) ### Features * new icon ([#959](https://github.com/homebound-team/beam/issues/959)) ([a26a4b1](https://github.com/homebound-team/beam/commit/a26a4b16fd181c2105cb460c25972e456fc42cfb)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 295a2734b..f0b83edb7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.320.1", + "version": "2.321.0", "author": "Homebound", "license": "MIT", "main": "dist/index.js", From f1329f6334b8a472c5edad195df17aa7b199a208 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Thu, 12 Oct 2023 12:58:52 -0500 Subject: [PATCH 11/14] fix: Refactor ComboBoxBase to memo more (#964) --- src/inputs/SelectField.test.tsx | 36 +++--- src/inputs/internal/ComboBoxBase.tsx | 177 ++++++++------------------- src/utils/rtl.test.tsx | 176 +++++++++++++++----------- 3 files changed, 181 insertions(+), 208 deletions(-) diff --git a/src/inputs/SelectField.test.tsx b/src/inputs/SelectField.test.tsx index aa23677af..b2bcf8c4c 100644 --- a/src/inputs/SelectField.test.tsx +++ b/src/inputs/SelectField.test.tsx @@ -403,20 +403,28 @@ describe("SelectFieldTest", () => { it("supports boolean values properly", async () => { // Given a select field with boolean and an undefined values const onSelect = jest.fn(); - const r = await render( - o.name} - getOptionValue={(o) => o.id} - />, - ); + type Option = { id: undefined | boolean; name: string }; + function Test() { + const [value, setValue] = useState(true); + return ( + + label="label" + value={value} + onSelect={(value) => { + onSelect(value); + setValue(value); + }} + options={[ + { id: undefined, name: "Undefined" }, + { id: false, name: "False" }, + { id: true, name: "True" }, + ]} + getOptionLabel={(o) => o.name} + getOptionValue={(o) => o.id} + /> + ); + } + const r = await render(); // When selecting the `false` option click(r.label); diff --git a/src/inputs/internal/ComboBoxBase.tsx b/src/inputs/internal/ComboBoxBase.tsx index 0b0b84015..3a3031703 100644 --- a/src/inputs/internal/ComboBoxBase.tsx +++ b/src/inputs/internal/ComboBoxBase.tsx @@ -10,7 +10,6 @@ import { ComboBoxInput } from "src/inputs/internal/ComboBoxInput"; import { ListBox } from "src/inputs/internal/ListBox"; 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 { @@ -80,7 +79,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) onSelect, options: propOptions, multiselect = false, - values = [], + values: propValues, nothingSelectedText = "", contrast, disabledOptions, @@ -96,16 +95,22 @@ export function ComboBoxBase(props: ComboBoxBaseProps) // Memoize the callback functions and handle the `unset` option if provided. const getOptionLabel = useCallback( (o: O) => (unsetLabel && o === unsetOption ? unsetLabel : propOptionLabel(o)), - [propOptionLabel, unsetLabel], + // propOptionLabel is basically always a lambda, so don't dep on it + // eslint-disable-next-line react-hooks/exhaustive-deps + [unsetLabel], ); const getOptionValue = useCallback( (o: O) => (unsetLabel && o === unsetOption ? (undefined as V) : propOptionValue(o)), - [propOptionValue, unsetLabel], + // propOptionValue is basically always a lambda, so don't dep on it + // eslint-disable-next-line react-hooks/exhaustive-deps + [unsetLabel], ); const getOptionMenuLabel = useCallback( (o: O) => propOptionMenuLabel ? propOptionMenuLabel(o, Boolean(unsetLabel) && o === unsetOption) : getOptionLabel(o), - [propOptionMenuLabel, unsetLabel, getOptionLabel], + // propOptionMenuLabel is basically always a lambda, so don't dep on it + // eslint-disable-next-line react-hooks/exhaustive-deps + [unsetLabel, getOptionLabel], ); // Call `initializeOptions` to prepend the `unset` option if the `unsetLabel` was provided. @@ -117,44 +122,33 @@ export function ComboBoxBase(props: ComboBoxBaseProps) Array.isArray(propOptions) ? [propOptions, unsetLabel] : [propOptions.current, propOptions.options, unsetLabel], ); + const values = useMemo(() => propValues ?? [], [propValues]); + + const selectedOptions = useMemo(() => { + return options.filter((o) => values.includes(getOptionValue(o))); + }, [options, values, getOptionValue]); + const { contains } = useFilter({ sensitivity: "base" }); const isDisabled = !!disabled; const isReadOnly = !!readOnly; // Do a one-time initialize of fieldState - const [fieldState, setFieldState] = useState>(() => { - const selectedOptions = options.filter((o) => values.includes(getOptionValue(o))); + const [fieldState, setFieldState] = useState(() => { return { - selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], - inputValue: getInputValue( - options.filter((o) => values?.includes(getOptionValue(o))), - getOptionLabel, - multiselect, - nothingSelectedText, - ), - filteredOptions: options, - allOptions: options, - selectedOptions, + inputValue: getInputValue(selectedOptions, getOptionLabel, multiselect, nothingSelectedText), + searchValue: undefined, optionsLoading: false, }; }); + const { searchValue } = fieldState; + const filteredOptions = useMemo(() => { + return !searchValue ? options : options.filter((o) => contains(getOptionLabel(o), searchValue)); + }, [options, searchValue, getOptionLabel, contains]); + /** Resets field's input value and filtered options list for cases where the user exits the field without making changes (on Escape, or onBlur) */ function resetField() { - const inputValue = getInputValue( - fieldState.allOptions.filter((o) => values?.includes(getOptionValue(o))), - getOptionLabel, - multiselect, - nothingSelectedText, - ); - // Conditionally reset the value if the current inputValue doesn't match that of the passed in value, or we filtered the list - if (inputValue !== fieldState.inputValue || fieldState.filteredOptions.length !== fieldState.allOptions.length) { - setFieldState((prevState) => ({ - ...prevState, - inputValue, - filteredOptions: prevState.allOptions, - })); - } + setFieldState((prevState) => ({ ...prevState, searchValue: undefined })); } function onSelectionChange(keys: Selection): void { @@ -172,34 +166,12 @@ export function ComboBoxBase(props: ComboBoxBaseProps) ); if (multiselect && keys.size === 0) { - setFieldState({ - ...fieldState, - inputValue: state.isOpen ? "" : nothingSelectedText, - selectedKeys: [], - selectedOptions: [], - }); selectionChanged && onSelect([], []); return; } const selectedKeys = [...keys.values()]; - const selectedOptions = fieldState.allOptions.filter((o) => selectedKeys.includes(valueToKey(getOptionValue(o)))); - const firstSelectedOption = selectedOptions[0]; - - setFieldState((prevState) => ({ - ...prevState, - // If menu is open then reset inputValue to "". Otherwise set inputValue depending on number of options selected. - inputValue: - multiselect && (state.isOpen || selectedKeys.length > 1) - ? "" - : firstSelectedOption - ? getOptionLabel(firstSelectedOption!) - : "", - selectedKeys, - selectedOptions, - filteredOptions: fieldState.allOptions, - })); - + const selectedOptions = options.filter((o) => selectedKeys.includes(valueToKey(getOptionValue(o)))); selectionChanged && onSelect(selectedKeys.map(keyToValue) as V[], selectedOptions); if (!multiselect) { @@ -210,11 +182,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) function onInputChange(value: string) { if (value !== fieldState.inputValue) { - setFieldState((prevState) => ({ - ...prevState, - inputValue: value, - filteredOptions: fieldState.allOptions.filter((o) => contains(getOptionLabel(o), value)), - })); + setFieldState((prevState) => ({ ...prevState, inputValue: value, searchValue: value })); } } @@ -232,7 +200,6 @@ export function ComboBoxBase(props: ComboBoxBaseProps) maybeInitLoad(); firstOpen.current = false; } - // When using the multiselect field, always empty the input upon open. if (multiselect && isOpen) { setFieldState((prevState) => ({ ...prevState, inputValue: "" })); @@ -255,7 +222,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) ...otherProps, disabledKeys: Object.keys(disabledOptionsWithReasons), inputValue: fieldState.inputValue, - items: fieldState.filteredOptions, + items: filteredOptions, isDisabled, isReadOnly, onInputChange, @@ -286,65 +253,34 @@ export function ComboBoxBase(props: ComboBoxBaseProps) }, }); + const selectedKeys = useMemo(() => { + return selectedOptions.map((o) => valueToKey(getOptionValue(o))); + }, [selectedOptions, getOptionValue]); // @ts-ignore - `selectionManager.state` exists, but not according to the types state.selectionManager.state = useMultipleSelectionState({ selectionMode: multiselect ? "multiple" : "single", // Do not allow an empty selection if single select mode disallowEmptySelection: !multiselect, - selectedKeys: fieldState.selectedKeys, + selectedKeys, onSelectionChange, }); - // Ensure we reset if the field's values change and the user is not actively selecting options. - useEffect( - () => { - if (!state.isOpen && !areArraysEqual(values, fieldState.selectedKeys)) { - setFieldState((prevState) => { - const selectedOptions = prevState.allOptions.filter((o) => values?.includes(getOptionValue(o))); - return { - ...prevState, - selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], - inputValue: - selectedOptions.length === 1 - ? getOptionLabel(selectedOptions[0]) - : multiselect && selectedOptions.length === 0 - ? nothingSelectedText - : "", - selectedOptions: selectedOptions, - }; - }); - } - }, - // 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 - [values], - ); - - // Re-sync fieldState.allOptions - useEffect( - () => { - setFieldState((prevState) => { - const selectedOptions = options.filter((o) => values?.includes(getOptionValue(o))); - return { - ...prevState, - selectedKeys: selectedOptions?.map((o) => valueToKey(getOptionValue(o))) ?? [], - inputValue: - selectedOptions.length === 1 - ? getOptionLabel(selectedOptions[0]) - : multiselect && selectedOptions.length === 0 - ? nothingSelectedText - : "", - selectedOptions: selectedOptions, - filteredOptions: options, - allOptions: options, - }; - }); - }, - // We're primarily only re-setting `allOptions`, and so recalc selected as well, but we don't - // want to depend on values/etc., b/c we'll defer to their useEffects to update their state - // eslint-disable-next-line react-hooks/exhaustive-deps - [options], - ); + // Reset inputValue when closed or selected changes + useEffect(() => { + if (state.isOpen && multiselect) { + // While the multiselect is open, let the user keep typing + setFieldState((prevState) => ({ + ...prevState, + inputValue: "", + searchValue: "", + })); + } else { + setFieldState((prevState) => ({ + ...prevState, + inputValue: getInputValue(selectedOptions, getOptionLabel, multiselect, nothingSelectedText), + })); + } + }, [state.isOpen, selectedOptions, getOptionLabel, multiselect, nothingSelectedText]); // For the most part, the returned props contain `aria-*` and `id` attributes for accessibility purposes. const { @@ -396,7 +332,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) listBoxRef={listBoxRef} state={state} labelProps={labelProps} - selectedOptions={fieldState.selectedOptions} + selectedOptions={selectedOptions} getOptionValue={getOptionValue} getOptionLabel={getOptionLabel} contrast={contrast} @@ -419,7 +355,7 @@ export function ComboBoxBase(props: ComboBoxBaseProps) positionProps={positionProps} state={state} listBoxRef={listBoxRef} - selectedOptions={fieldState.selectedOptions} + selectedOptions={selectedOptions} getOptionLabel={getOptionLabel} getOptionValue={(o) => valueToKey(getOptionValue(o))} contrast={contrast} @@ -433,12 +369,11 @@ export function ComboBoxBase(props: ComboBoxBaseProps) ); } -type FieldState = { - selectedKeys: Key[]; +type FieldState = { inputValue: string; - filteredOptions: O[]; - selectedOptions: O[]; - allOptions: O[]; + // We need separate `searchValue` vs. `inputValue` b/c we might be showing the + // currently-loaded option in the input, without the user having typed a filter yet. + searchValue: string | undefined; optionsLoading: boolean; }; @@ -510,7 +445,3 @@ export function disabledOptionToKeyedTuple( return [valueToKey(disabledOption), undefined]; } } - -function asArray(arrayOrElement: E[] | E | undefined): E[] { - return Array.isArray(arrayOrElement) ? arrayOrElement : arrayOrElement ? [arrayOrElement] : []; -} diff --git a/src/utils/rtl.test.tsx b/src/utils/rtl.test.tsx index d4c450f54..eca686488 100644 --- a/src/utils/rtl.test.tsx +++ b/src/utils/rtl.test.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { MultiSelectField, NestedOption, SelectField, TreeSelectField } from "src/inputs"; import { HasIdAndName } from "src/types"; import { getOptions, getSelected, render, select, selectAndWait } from "src/utils/rtl"; @@ -6,18 +7,25 @@ describe("rtl", () => { it("can use select helpers and select an option via value on SelectField", async () => { const onSelect = jest.fn(); // Given the SelectField - const r = await render( - , - ); + function Test() { + const [value, setValue] = useState(); + return ( + { + setValue(value); + onSelect(value, option); + }} + options={[ + { id: "1", name: "One" }, + { id: "2", name: "Two" }, + { id: "3", name: "Three" }, + ]} + /> + ); + } + const r = await render(); // Then the getOptions helper returns the correct options expect(getOptions(r.number)).toEqual(["One", "Two", "Three"]); @@ -89,18 +97,25 @@ describe("rtl", () => { it("can use select helpers and select an option via value on MultiSelectField", async () => { const onSelect = jest.fn(); // Given the MultiSelectField - const r = await render( - , - ); + function Test() { + const [values, setValues] = useState([]); + return ( + { + setValues(values); + onSelect(values, options); + }} + options={[ + { id: "1", name: "One" }, + { id: "2", name: "Two" }, + { id: "3", name: "Three" }, + ]} + /> + ); + } + const r = await render(); // Then the getOptions helper returns the correct options expect(getOptions(r.number)).toEqual(["One", "Two", "Three"]); @@ -127,18 +142,25 @@ describe("rtl", () => { it("can select options via label on MultiSelectField", async () => { const onSelect = jest.fn(); // Given the MultiSelectField - const r = await render( - , - ); + function Test() { + const [values, setValues] = useState([]); + return ( + { + setValues(values); + onSelect(values, options); + }} + options={[ + { id: "1", name: "One" }, + { id: "2", name: "Two" }, + { id: "3", name: "Three" }, + ]} + /> + ); + } + const r = await render(); // When selecting options by label select(r.number, ["One", "Three"]); @@ -215,30 +237,35 @@ describe("rtl", () => { expect(getSelected(r.number)).toEqual("One One"); }); - // TODO: validate this eslint-disable with https://app.shortcut.com/homebound-team/story/40045 - // eslint-disable-next-line jest/no-identical-title - it("can select options via label on MultiSelectField", async () => { + it("can select options via label on TreeSelectField", async () => { const onSelect = jest.fn(); // Given the TreeSelectField - const r = await render( - onSelect(all)} - options={ - [ - { - id: "1", - name: "One", - children: [ - { id: "1.1", name: "One One" }, - { id: "1.2", name: "One Two" }, - ], - }, - ] as NestedOption[] - } - />, - ); + function Test() { + const [values, setValues] = useState([]); + return ( + { + setValues(all.values); + onSelect(all); + }} + options={ + [ + { + id: "1", + name: "One", + children: [ + { id: "1.1", name: "One One" }, + { id: "1.2", name: "One Two" }, + ], + }, + ] as NestedOption[] + } + /> + ); + } + const r = await render(); // When selecting an option by its label select(r.number, ["One One"]); // Then the onSelect handler is called with the correct values @@ -285,19 +312,26 @@ describe("rtl", () => { it("can use select helpers on multiline SelectField", async () => { const onSelect = jest.fn(); // Given the SelectField is multline - const r = await render( - , - ); + function Test() { + const [value, setValue] = useState(); + return ( + { + setValue(value); + onSelect(value, option); + }} + options={[ + { id: "1", name: "One" }, + { id: "2", name: "Two" }, + { id: "3", name: "Three" }, + ]} + multiline + /> + ); + } + const r = await render(); // Then the getOptions helper returns the correct options expect(getOptions(r.number)).toEqual(["One", "Two", "Three"]); // When selecting an option From 98363586aeac276298b5749764c349652c18c4c8 Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Thu, 12 Oct 2023 18:02:01 +0000 Subject: [PATCH 12/14] chore(release): 2.321.1 [skip ci] ## [2.321.1](https://github.com/homebound-team/beam/compare/v2.321.0...v2.321.1) (2023-10-12) ### Bug Fixes * Refactor ComboBoxBase to memo more ([#964](https://github.com/homebound-team/beam/issues/964)) ([f1329f6](https://github.com/homebound-team/beam/commit/f1329f6334b8a472c5edad195df17aa7b199a208)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f0b83edb7..5799afa09 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.321.0", + "version": "2.321.1", "author": "Homebound", "license": "MIT", "main": "dist/index.js", From 5aac321676368892f32381986d0dfa848240aad1 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Thu, 12 Oct 2023 13:06:28 -0500 Subject: [PATCH 13/14] feat: Add column names to tableSnapshot. (#965) --- src/components/Table/GridTable.test.tsx | 27 +++++++++++++++++++++++++ src/utils/rtl.tsx | 27 ++++++++++++++++--------- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/components/Table/GridTable.test.tsx b/src/components/Table/GridTable.test.tsx index c615c5c2f..f4e1cc85e 100644 --- a/src/components/Table/GridTable.test.tsx +++ b/src/components/Table/GridTable.test.tsx @@ -32,6 +32,7 @@ import { type Data = { name: string; value: number | undefined | null }; type Row = SimpleHeaderAndData; +const idColumn: GridColumn = { id: "id", header: () => "Id", data: (data, { row }) => row.id }; const nameColumn: GridColumn = { id: "name", header: () => "Name", data: ({ name }) => name }; const valueColumn: GridColumn = { id: "value", header: () => "Value", data: ({ value }) => value }; const columns = [nameColumn, valueColumn]; @@ -3265,6 +3266,32 @@ describe("GridTable", () => { `); }); + it("tableSnapshot can use a subset of columns", async () => { + // Given a table with simple data + const r = await render( + , + ); + + // Then a text snapshot should be generated when using `tableSnapshot` + expect(tableSnapshot(r, ["Id", "Value"])).toMatchInlineSnapshot(` + " + | Id | Value | + | -- | ----- | + | 1 | 200 | + | 2 | 300 | + | 3 | 1000 | + " + `); + }); + it("renders totals row in the correct order", async () => { type Row = SimpleHeaderAndData | TotalsRow; // Given a table with simple header, totals, and data row diff --git a/src/utils/rtl.tsx b/src/utils/rtl.tsx index 752a1fd8a..ad10738c8 100644 --- a/src/utils/rtl.tsx +++ b/src/utils/rtl.tsx @@ -109,25 +109,32 @@ export function rowAnd(r: RenderResult, rowNum: number, testId: string): HTMLEle " `); * */ -export function tableSnapshot(r: RenderResult): string { +export function tableSnapshot(r: RenderResult, columnNames: string[] = []): string { const tableEl = r.getByTestId("gridTable"); const dataRows = Array.from(tableEl.querySelectorAll("[data-gridrow]")); const hasExpandableHeader = !!tableEl.querySelector(`[data-testid="expandableColumn"]`); - const tableDataAsStrings = dataRows.map((row) => { + let tableDataAsStrings = dataRows.map((row) => { return Array.from(row.childNodes).map(getTextFromTableCellNode); }); - return toMarkupTableString({ tableRows: tableDataAsStrings, hasExpandableHeader }); + // If the user wants a subset of columns, look for column names + if (columnNames.length > 0) { + const headerCells = tableDataAsStrings[0]; + if (headerCells) { + const columnIndices = columnNames.map((name) => { + const i = headerCells.indexOf(name); + if (i === -1) throw new Error(`Could not find header '${name}' in ${headerCells.join(", ")}`); + return i; + }); + tableDataAsStrings = tableDataAsStrings.map((row) => columnIndices.map((index) => row[index])); + } + } + + return toMarkupTableString(tableDataAsStrings, hasExpandableHeader); } -function toMarkupTableString({ - tableRows, - hasExpandableHeader, -}: { - tableRows: (string | null)[][]; - hasExpandableHeader: boolean; -}) { +function toMarkupTableString(tableRows: (string | null)[][], hasExpandableHeader: boolean): string { // Find the largest width of each column to set a consistent width for each row const columnWidths = tableRows.reduce((acc, row) => { row.forEach((cell, columnIndex) => { From dd24eb5ce4c9739c1611bcdb64fcca79fbb64d12 Mon Sep 17 00:00:00 2001 From: Homebound Bot Date: Thu, 12 Oct 2023 18:09:50 +0000 Subject: [PATCH 14/14] chore(release): 2.322.0 [skip ci] ## [2.322.0](https://github.com/homebound-team/beam/compare/v2.321.1...v2.322.0) (2023-10-12) ### Features * Add column names to tableSnapshot. ([#965](https://github.com/homebound-team/beam/issues/965)) ([5aac321](https://github.com/homebound-team/beam/commit/5aac321676368892f32381986d0dfa848240aad1)) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5799afa09..f1f0abaa8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@homebound/beam", - "version": "2.321.1", + "version": "2.322.0", "author": "Homebound", "license": "MIT", "main": "dist/index.js",