From 31dcc7feb81628b46feffe7a1f7900582514a254 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 27 Jul 2021 17:41:40 -0500 Subject: [PATCH 1/3] feat: Add SelectField.mapOption prop. --- src/forms/BoundSelectField.test.tsx | 29 ++++++++++++++--------- src/forms/BoundSelectField.tsx | 13 +---------- src/inputs/SelectField.tsx | 36 ++++++++++------------------- 3 files changed, 31 insertions(+), 47 deletions(-) diff --git a/src/forms/BoundSelectField.test.tsx b/src/forms/BoundSelectField.test.tsx index 3d5e079b8..9ed506618 100644 --- a/src/forms/BoundSelectField.test.tsx +++ b/src/forms/BoundSelectField.test.tsx @@ -11,20 +11,26 @@ const sports = [ describe("BoundSelectField", () => { it("shows the current value", async () => { const author = createObjectState(formConfig, { favoriteSport: "s:1" }); - const { favoriteSport } = await render(); + const { favoriteSport } = await render( + , + ); expect(favoriteSport()).toHaveValue("Football"); }); it("shows the error message", async () => { const author = createObjectState(formConfig, {}); author.favoriteSport.touched = true; - const { favoriteSport_errorMsg } = await render(); + const { favoriteSport_errorMsg } = await render( + , + ); expect(favoriteSport_errorMsg()).toHaveTextContent("Required"); }); it("shows the label", async () => { const author = createObjectState(formConfig, { favoriteSport: "s:1" }); - const { favoriteSport_label } = await render(); + const { favoriteSport_label } = await render( + , + ); expect(favoriteSport_label()).toHaveTextContent("Favorite Sport"); }); @@ -35,14 +41,7 @@ describe("BoundSelectField", () => { { label: "No", value: false }, { label: "", value: undefined }, ]; - const r = await render( - o.label} - getOptionValue={(o) => o.value} - />, - ); + const r = await render(); expect(r.isAvailable()).toHaveValue(""); }); }); @@ -51,3 +50,11 @@ const formConfig: ObjectConfig = { favoriteSport: { type: "value", rules: [required] }, isAvailable: { type: "value" }, }; + +function identity(o: T): T { + return o; +} + +function idAndName({ id, name }: { id: string; name: string }) { + return { value: id, label: name }; +} diff --git a/src/forms/BoundSelectField.tsx b/src/forms/BoundSelectField.tsx index eb1f1a6ec..c09beaba1 100644 --- a/src/forms/BoundSelectField.tsx +++ b/src/forms/BoundSelectField.tsx @@ -1,7 +1,6 @@ import { FieldState } from "@homebound/form-state/dist/formState"; import { Observer } from "mobx-react"; import { SelectField, SelectFieldProps, Value } from "src/inputs"; -import { HasIdAndName, Optional } from "src/types"; import { defaultLabel } from "src/utils/defaultLabel"; import { useTestIds } from "src/utils/useTestIds"; @@ -24,19 +23,11 @@ export type BoundSelectFieldProps = Omit< * The caller has to tell us how to turn `T` into `V`, which is usually a * lambda like `t => t.id`. */ -export function BoundSelectField(props: BoundSelectFieldProps): JSX.Element; -export function BoundSelectField, V extends Value>( - props: Optional, "getOptionLabel" | "getOptionValue">, -): JSX.Element; -export function BoundSelectField( - props: Optional, "getOptionValue" | "getOptionLabel">, -): JSX.Element { +export function BoundSelectField(props: BoundSelectFieldProps): JSX.Element { const { field, options, readOnly, - getOptionValue = (opt: T) => (opt as any).id, // if unset, assume O implements HasId - getOptionLabel = (opt: T) => (opt as any).name, // if unset, assume O implements HasName onSelect = (value) => field.set(value), label = defaultLabel(field.key), ...others @@ -53,8 +44,6 @@ export function BoundSelectField( readOnly={readOnly ?? field.readOnly} errorMsg={field.touched ? field.errors.join(" ") : undefined} required={field.required} - getOptionLabel={getOptionLabel} - getOptionValue={getOptionValue} onBlur={() => field.blur()} onFocus={() => field.focus()} {...others} diff --git a/src/inputs/SelectField.tsx b/src/inputs/SelectField.tsx index ea2ffa484..8b42ad180 100644 --- a/src/inputs/SelectField.tsx +++ b/src/inputs/SelectField.tsx @@ -1,17 +1,14 @@ -import React, { ReactNode } from "react"; +import React from "react"; import { Value } from "src/inputs"; import { BeamSelectFieldBaseProps, SelectFieldBase } from "src/inputs/internal/SelectFieldBase"; -import { HasIdAndName, Optional } from "src/types"; export interface SelectFieldProps extends BeamSelectFieldBaseProps { /** Renders `opt` in the dropdown menu, defaults to the `getOptionLabel` prop. */ - getOptionMenuLabel?: (opt: O) => string | ReactNode; - getOptionValue: (opt: O) => V; - getOptionLabel: (opt: O) => string; /** The current value; it can be `undefined`, even if `V` cannot be. */ value: V | undefined; onSelect: (value: V, opt: O) => void; options: O[]; + mapOption: (opt: O) => { label: string; value: V; menuLabel?: string }; } /** @@ -20,33 +17,24 @@ export interface SelectFieldProps extends BeamSelectFieldBas * The `O` type is a list of options to show, the `V` is the primitive value of a * given `O` (i.e. it's id) that you want to use as the current/selected value. */ -export function SelectField(props: SelectFieldProps): JSX.Element; -export function SelectField, V extends Value>( - props: Optional, "getOptionValue" | "getOptionLabel">, -): JSX.Element; -export function SelectField( - props: Optional, "getOptionLabel" | "getOptionValue">, -): JSX.Element { - const { - getOptionValue = (opt: O) => (opt as any).id, // if unset, assume O implements HasId - getOptionLabel = (opt: O) => (opt as any).name, // if unset, assume O implements HasName - options, - onSelect, - value, - ...otherProps - } = props; +export function SelectField(props: SelectFieldProps): JSX.Element { + const { options, onSelect, value, mapOption, ...otherProps } = props; return ( mapOption(o).label} + getOptionMenuLabel={(o) => { + const mapped = mapOption(o); + return mapped.menuLabel ?? mapped.label; + }} + getOptionValue={(o) => mapOption(o).value} values={value ? [value] : []} onSelect={(values) => { if (values.length > 0) { - const selectedOption = options.find((o) => getOptionValue(o) === values[0]); - onSelect && selectedOption && onSelect(getOptionValue(selectedOption), selectedOption); + const selectedOption = options.find((o) => mapOption(o).value === values[0]); + onSelect && selectedOption && onSelect(mapOption(selectedOption).value, selectedOption); } }} /> From 7333d83973646e6fbc1437478cdb0e8c5c5c7b32 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Tue, 27 Jul 2021 17:56:00 -0500 Subject: [PATCH 2/3] Fix some more. --- src/forms/BoundSelectField.test.tsx | 9 +- src/inputs/SelectField.stories.tsx | 203 ++++++++++++++++------------ src/inputs/SelectField.test.tsx | 10 +- src/inputs/SelectField.tsx | 12 +- 4 files changed, 129 insertions(+), 105 deletions(-) diff --git a/src/forms/BoundSelectField.test.tsx b/src/forms/BoundSelectField.test.tsx index 9ed506618..07d9ae193 100644 --- a/src/forms/BoundSelectField.test.tsx +++ b/src/forms/BoundSelectField.test.tsx @@ -2,6 +2,7 @@ import { createObjectState, ObjectConfig, required } from "@homebound/form-state import { render } from "@homebound/rtl-utils"; import { BoundSelectField } from "src/forms/BoundSelectField"; import { AuthorInput } from "src/forms/formStateDomain"; +import { idAndName, identity } from "src/inputs/SelectField"; const sports = [ { id: "s:1", name: "Football" }, @@ -50,11 +51,3 @@ const formConfig: ObjectConfig = { favoriteSport: { type: "value", rules: [required] }, isAvailable: { type: "value" }, }; - -function identity(o: T): T { - return o; -} - -function idAndName({ id, name }: { id: string; name: string }) { - return { value: id, label: name }; -} diff --git a/src/inputs/SelectField.stories.tsx b/src/inputs/SelectField.stories.tsx index 03fc7c2a6..57a99abff 100644 --- a/src/inputs/SelectField.stories.tsx +++ b/src/inputs/SelectField.stories.tsx @@ -3,8 +3,7 @@ import { Meta } from "@storybook/react"; import { useState } from "react"; import { GridColumn, GridTable, Icon, Icons, simpleHeader, SimpleHeaderAndDataOf } from "src/components"; import { Css } from "src/Css"; -import { SelectField, SelectFieldProps, Value } from "src/inputs"; -import { HasIdAndName, Optional } from "src/types"; +import { idAndName, identity, SelectField, SelectFieldProps, Value } from "src/inputs"; import { noop } from "src/utils"; import { zeroTo } from "src/utils/sb"; @@ -51,61 +50,73 @@ export function SelectFields() { label="Favorite Icon" value={options[2].id} options={options} - getOptionMenuLabel={(o) => ( -
- {o.icon && ( - - - - )} - {o.name} -
- )} + mapOption={(o) => ({ + ...idAndName(o), + menuLabel: ( +
+ {o.icon && ( + + + + )} + {o.name} +
+ ), + })} /> o.icon && } value={options[1].id} - getOptionMenuLabel={(o) => ( -
- {o.icon && ( - - - - )} - {o.name} -
- )} + mapOption={(o) => ({ + ...idAndName(o), + menuLabel: ( +
+ {o.icon && ( + + + + )} + {o.name} +
+ ), + })} /> label="Favorite Icon - Disabled" value={undefined} options={options} disabled + mapOption={idAndName} + /> + + + label="Favorite Icon - Invalid" + value={undefined} + options={options} + mapOption={idAndName} /> - - label="Favorite Icon - Invalid" value={undefined} options={options} /> o.id} - getOptionLabel={(o) => o.name} - /> - o.value} - getOptionLabel={(o) => o.label} + mapOption={idAndName} /> +
@@ -115,16 +126,19 @@ export function SelectFields() { label="Favorite Icon" value={options[2].id} options={options} - getOptionMenuLabel={(o) => ( -
- {o.icon && ( - - - - )} - {o.name} -
- )} + mapOption={(o) => ({ + ...idAndName(o), + menuLabel: ( +
+ {o.icon && ( + + + + )} + {o.name} +
+ ), + })} /> o.icon && } value={options[1].id} - getOptionMenuLabel={(o) => ( -
- {o.icon && ( - - - - )} - {o.name} -
- )} + mapOption={(o) => ({ + ...idAndName(o), + menuLabel: ( +
+ {o.icon && ( + + + + )} + {o.name} +
+ ), + })} /> compact label="Favorite Icon - Disabled" value={undefined} options={options} + mapOption={idAndName} disabled /> - + compact label="Favorite Icon - Invalid" options={options} value={undefined} + mapOption={idAndName} />

Inline Label

- - + + o.icon && } value={options[4].id} - getOptionMenuLabel={(o) => ( -
- {o.icon && ( - - - - )} - {o.name} -
- )} + mapOption={(o) => ({ + ...idAndName(o), + menuLabel: ( +
+ {o.icon && ( + + + + )} + {o.name} +
+ ), + })} />

Load test, 1000 Options

- +
); @@ -210,15 +257,7 @@ const columns: GridColumn[] = [ { header: "Address", data: (data) => data.address }, { header: "Contact", - data: (data) => ( - iu.id} - getOptionLabel={(iu) => iu.name} - value={data.user.id} - onSelect={noop} - options={people} - /> - ), + data: (data) => , }, { header: "Market", data: (data) => data.market }, ]; @@ -226,25 +265,15 @@ type Row = SimpleHeaderAndDataOf; type InternalUser = { name: string; id: string }; type Request = { id: string; user: InternalUser; address: string; homeowner: string; market: string }; -// Kind of annoying but to get type inference for HasIdAndName working, we -// have to re-copy/paste the overload here. function TestSelectField( props: Omit, "onSelect">, -): JSX.Element; -function TestSelectField, V extends Value>( - props: Optional, "onSelect">, "getOptionValue" | "getOptionLabel">, -): JSX.Element; -function TestSelectField( - props: Optional, "onSelect">, "getOptionValue" | "getOptionLabel">, ): JSX.Element { const [selectedOption, setSelectedOption] = useState(props.value); return (
- // The `as any` is due to something related to https://github.com/emotion-js/emotion/issues/2169 - // We may have to redo the conditional getOptionValue/getOptionLabel - {...(props as any)} + {...props} value={selectedOption} onSelect={setSelectedOption} errorMsg={ diff --git a/src/inputs/SelectField.test.tsx b/src/inputs/SelectField.test.tsx index bf7ab2ee8..45c47e085 100644 --- a/src/inputs/SelectField.test.tsx +++ b/src/inputs/SelectField.test.tsx @@ -1,6 +1,6 @@ import { click, input, render } from "@homebound/rtl-utils"; import { useState } from "react"; -import { SelectField, SelectFieldProps, Value } from "src/inputs"; +import { idAndName, SelectField, SelectFieldProps, Value } from "src/inputs"; const options = [ { id: "1", name: "One" }, @@ -14,13 +14,7 @@ describe("SelectFieldTest", () => { it("can set a value", async () => { // Given a MultiSelectField const { getByRole } = await render( - o.name} - getOptionValue={(o) => o.id} - />, + , ); // That initially has "One" selected const text = getByRole("combobox"); diff --git a/src/inputs/SelectField.tsx b/src/inputs/SelectField.tsx index 8b42ad180..b00f90780 100644 --- a/src/inputs/SelectField.tsx +++ b/src/inputs/SelectField.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import React, { ReactNode } from "react"; import { Value } from "src/inputs"; import { BeamSelectFieldBaseProps, SelectFieldBase } from "src/inputs/internal/SelectFieldBase"; @@ -8,7 +8,7 @@ export interface SelectFieldProps extends BeamSelectFieldBas value: V | undefined; onSelect: (value: V, opt: O) => void; options: O[]; - mapOption: (opt: O) => { label: string; value: V; menuLabel?: string }; + mapOption: (opt: O) => { label: string; value: V; menuLabel?: ReactNode }; } /** @@ -40,3 +40,11 @@ export function SelectField(props: SelectFieldProps): /> ); } + +export function identity(o: T): T { + return o; +} + +export function idAndName({ id, name }: { id: V; name: string }) { + return { value: id, label: name }; +} From 0f6e9c65fc61a05ff5f9de35c68b17cafbea2fda Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 28 Jul 2021 09:30:08 -0500 Subject: [PATCH 3/3] More futzing. --- src/inputs/SelectField.stories.tsx | 116 +++++++++-------------------- src/inputs/SelectField.test.tsx | 14 ++-- src/inputs/SelectField.tsx | 59 ++++++++++----- 3 files changed, 87 insertions(+), 102 deletions(-) diff --git a/src/inputs/SelectField.stories.tsx b/src/inputs/SelectField.stories.tsx index 57a99abff..10ce09350 100644 --- a/src/inputs/SelectField.stories.tsx +++ b/src/inputs/SelectField.stories.tsx @@ -3,7 +3,7 @@ import { Meta } from "@storybook/react"; import { useState } from "react"; import { GridColumn, GridTable, Icon, Icons, simpleHeader, SimpleHeaderAndDataOf } from "src/components"; import { Css } from "src/Css"; -import { idAndName, identity, SelectField, SelectFieldProps, Value } from "src/inputs"; +import { idAndName2, identity, SelectField, SelectFieldProps, Value } from "src/inputs"; import { noop } from "src/utils"; import { zeroTo } from "src/utils/sb"; @@ -50,9 +50,8 @@ export function SelectFields() { label="Favorite Icon" value={options[2].id} options={options} - mapOption={(o) => ({ - ...idAndName(o), - menuLabel: ( + mapOption={{ + menuLabel: (o: TestOption) => (
{o.icon && ( @@ -62,16 +61,15 @@ export function SelectFields() { {o.name}
), - })} + }} /> o.icon && } value={options[1].id} - mapOption={(o) => ({ - ...idAndName(o), - menuLabel: ( + mapOption={{ + menuLabel: (o: TestOption) => (
{o.icon && ( @@ -81,42 +79,30 @@ export function SelectFields() { {o.name}
), - })} + }} /> - + - - - label="Favorite Icon - Invalid" - value={undefined} - options={options} - mapOption={idAndName} + mapOption={idAndName2} /> + + + -
@@ -126,9 +112,8 @@ export function SelectFields() { label="Favorite Icon" value={options[2].id} options={options} - mapOption={(o) => ({ - ...idAndName(o), - menuLabel: ( + mapOption={{ + menuLabel: (o: TestOption) => (
{o.icon && ( @@ -138,7 +123,7 @@ export function SelectFields() { {o.name}
), - })} + }} /> o.icon && } value={options[1].id} - mapOption={(o) => ({ - ...idAndName(o), - menuLabel: ( + mapOption={{ + menuLabel: (o: TestOption) => (
{o.icon && ( @@ -158,58 +142,37 @@ export function SelectFields() { {o.name}
), - })} + }} /> - + + - compact label="Favorite Icon - Invalid" options={options} value={undefined} - mapOption={idAndName} + mapOption={idAndName2} />

Inline Label

- - + + o.icon && } value={options[4].id} - mapOption={(o) => ({ - ...idAndName(o), - menuLabel: ( + mapOption={{ + menuLabel: (o: TestOption) => (
{o.icon && ( @@ -219,17 +182,12 @@ export function SelectFields() { {o.name}
), - })} + }} />

Load test, 1000 Options

- +
); @@ -257,7 +215,7 @@ const columns: GridColumn[] = [ { header: "Address", data: (data) => data.address }, { header: "Contact", - data: (data) => , + data: (data) => , }, { header: "Market", data: (data) => data.market }, ]; @@ -265,15 +223,15 @@ type Row = SimpleHeaderAndDataOf; type InternalUser = { name: string; id: string }; type Request = { id: string; user: InternalUser; address: string; homeowner: string; market: string }; -function TestSelectField( - props: Omit, "onSelect">, +function TestSelectField( + props: Omit, "onSelect">, ): JSX.Element { const [selectedOption, setSelectedOption] = useState(props.value); return (
- - {...props} + + {...(props as any)} value={selectedOption} onSelect={setSelectedOption} errorMsg={ diff --git a/src/inputs/SelectField.test.tsx b/src/inputs/SelectField.test.tsx index 45c47e085..0567e891b 100644 --- a/src/inputs/SelectField.test.tsx +++ b/src/inputs/SelectField.test.tsx @@ -1,6 +1,6 @@ import { click, input, render } from "@homebound/rtl-utils"; import { useState } from "react"; -import { idAndName, SelectField, SelectFieldProps, Value } from "src/inputs"; +import { idAndName2, SelectField, SelectFieldProps, Value } from "src/inputs"; const options = [ { id: "1", name: "One" }, @@ -14,7 +14,7 @@ describe("SelectFieldTest", () => { it("can set a value", async () => { // Given a MultiSelectField const { getByRole } = await render( - , + , ); // That initially has "One" selected const text = getByRole("combobox"); @@ -27,11 +27,13 @@ describe("SelectFieldTest", () => { expect(onSelect).toHaveBeenCalledWith("3"); }); - function TestSelectField(props: Omit, "onSelect">): JSX.Element { - const [selected, setSelected] = useState(props.value); + function TestSelectField( + props: Omit, "onSelect">, + ): JSX.Element { + const [selected, setSelected] = useState(props.value); return ( - - {...props} + + {...(props as any)} value={selected} onSelect={(value) => { onSelect(value); diff --git a/src/inputs/SelectField.tsx b/src/inputs/SelectField.tsx index b00f90780..a70276cca 100644 --- a/src/inputs/SelectField.tsx +++ b/src/inputs/SelectField.tsx @@ -1,15 +1,16 @@ import React, { ReactNode } from "react"; import { Value } from "src/inputs"; import { BeamSelectFieldBaseProps, SelectFieldBase } from "src/inputs/internal/SelectFieldBase"; +import { HasIdAndName } from "src/types"; -export interface SelectFieldProps extends BeamSelectFieldBaseProps { - /** Renders `opt` in the dropdown menu, defaults to the `getOptionLabel` prop. */ +export type SelectFieldProps = BeamSelectFieldBaseProps & { /** The current value; it can be `undefined`, even if `V` cannot be. */ value: V | undefined; - onSelect: (value: V, opt: O) => void; + onSelect: (value: V2, opt: O) => void; options: O[]; - mapOption: (opt: O) => { label: string; value: V; menuLabel?: ReactNode }; -} +} & (O extends HasIdAndName + ? { mapOption?: { label?: (opt: O) => string; value?: (opt: O) => V2; menuLabel?: (opt: O) => ReactNode } } + : { mapOption: { label: (opt: O) => string; value: (opt: O) => V2; menuLabel?: (opt: O) => ReactNode } }); /** * Provides a non-native select/dropdown widget. @@ -17,34 +18,58 @@ export interface SelectFieldProps extends BeamSelectFieldBas * The `O` type is a list of options to show, the `V` is the primitive value of a * given `O` (i.e. it's id) that you want to use as the current/selected value. */ -export function SelectField(props: SelectFieldProps): JSX.Element { - const { options, onSelect, value, mapOption, ...otherProps } = props; +export function SelectField(props: SelectFieldProps): JSX.Element { + const { options, onSelect, value, mapOption: maybeMapOption, ...otherProps } = props; + + const mapOption = { + value: (o: any) => o.id, + label: (o: any) => o.name, + menuLabel: (o: any) => o.name, + ...maybeMapOption, + }; return ( mapOption(o).label} + getOptionLabel={(o) => mapOption.label(o)} getOptionMenuLabel={(o) => { - const mapped = mapOption(o); + const mapped = mapOption.label!(o); return mapped.menuLabel ?? mapped.label; }} - getOptionValue={(o) => mapOption(o).value} + getOptionValue={(o) => mapOption.value(o)} values={value ? [value] : []} onSelect={(values) => { if (values.length > 0) { - const selectedOption = options.find((o) => mapOption(o).value === values[0]); - onSelect && selectedOption && onSelect(mapOption(selectedOption).value, selectedOption); + const selectedOption = options.find((o) => mapOption.value(o) === values[0]); + onSelect && selectedOption && onSelect(mapOption.value(selectedOption), selectedOption); } }} /> ); } -export function identity(o: T): T { - return o; -} +export const identity = { + value(o: { value: V }): V { + return o.value; + }, + label(o: { label: string }): string { + return o.label; + }, +}; -export function idAndName({ id, name }: { id: V; name: string }) { - return { value: id, label: name }; +export function idAndName() { + return { + value: (o: { id: V; name: string }): V => o.id, + label: (o: { id: V; name: string }): String => o.name, + }; } + +export const idAndName2 = { + value(o: { id: V; name: string }): V { + return o.id; + }, + label(o: { id: V; name: string }): string { + return o.name; + }, +};