From b8a22ae8b20c2525d8e190c430504100e4d4b9dc Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 2 Mar 2022 09:48:42 +0000 Subject: [PATCH] fix(multi-select): fix labels on multi select component --- .../{ComboMultiple.jsx => ComboMultiple.tsx} | 163 ++++++++++-------- package.json | 1 + yarn.lock | 5 + 3 files changed, 96 insertions(+), 73 deletions(-) rename app/javascript/components/{ComboMultiple.jsx => ComboMultiple.tsx} (73%) diff --git a/app/javascript/components/ComboMultiple.jsx b/app/javascript/components/ComboMultiple.tsx similarity index 73% rename from app/javascript/components/ComboMultiple.jsx rename to app/javascript/components/ComboMultiple.tsx index 7938518a0..a0741ad09 100644 --- a/app/javascript/components/ComboMultiple.jsx +++ b/app/javascript/components/ComboMultiple.tsx @@ -5,9 +5,12 @@ import React, { useContext, createContext, useEffect, - useLayoutEffect + useLayoutEffect, + MutableRefObject, + ReactNode, + ChangeEventHandler, + KeyboardEventHandler } from 'react'; -import PropTypes from 'prop-types'; import { Combobox, ComboboxInput, @@ -24,22 +27,51 @@ import invariant from 'tiny-invariant'; import { useDeferredSubmit, useHiddenField } from './shared/hooks'; -const Context = createContext(); +const Context = createContext<{ + selectionsRef: MutableRefObject; + onRemove: (value: string) => void; +} | null>(null); -const optionValueByLabel = (values, options, label) => { - const maybeOption = values.includes(label) +type Option = [label: string, value: string]; + +function isOptions(options: string[] | Option[]): options is Option[] { + return Array.isArray(options[0]); +} + +const optionValueByLabel = ( + values: string[], + options: Option[], + label: string +): string => { + const maybeOption: Option | undefined = values.includes(label) ? [label, label] : options.find(([optionLabel]) => optionLabel == label); - return maybeOption ? maybeOption[1] : undefined; + return maybeOption ? maybeOption[1] : ''; }; -const optionLabelByValue = (values, options, value) => { - const maybeOption = values.includes(value) +const optionLabelByValue = ( + values: string[], + options: Option[], + value: string +): string => { + const maybeOption: Option | undefined = values.includes(value) ? [value, value] : options.find(([, optionValue]) => optionValue == value); - return maybeOption ? maybeOption[0] : undefined; + return maybeOption ? maybeOption[0] : ''; }; -function ComboMultiple({ +export type ComboMultipleProps = { + options: string[] | Option[]; + id: string; + labelledby: string; + describedby: string; + label: string; + group: string; + name?: string; + selected: string[]; + acceptNewValues?: boolean; +}; + +export default function ComboMultiple({ options, id, labelledby, @@ -49,21 +81,21 @@ function ComboMultiple({ name = 'value', selected, acceptNewValues = false -}) { +}: ComboMultipleProps) { invariant(id || label, 'ComboMultiple: `id` or a `label` are required'); invariant(group, 'ComboMultiple: `group` is required'); - const inputRef = useRef(); + const inputRef = useRef(null); const [term, setTerm] = useState(''); const [selections, setSelections] = useState(selected); - const [newValues, setNewValues] = useState([]); + const [newValues, setNewValues] = useState([]); const inputId = useId(id); const removedLabelledby = `${inputId}-remove`; const selectedLabelledby = `${inputId}-selected`; - const optionsWithLabels = useMemo( + const optionsWithLabels = useMemo( () => - Array.isArray(options[0]) + isOptions(options) ? options : options.filter((o) => o).map((o) => [o, o]), [options] @@ -94,11 +126,11 @@ function ComboMultiple({ const [, setHiddenFieldValue, hiddenField] = useHiddenField(group, name); const awaitFormSubmit = useDeferredSubmit(hiddenField); - const handleChange = (event) => { + const handleChange: ChangeEventHandler = (event) => { setTerm(event.target.value); }; - const saveSelection = (fn) => { + const saveSelection = (fn: (selections: string[]) => string[]) => { setSelections((selections) => { selections = fn(selections); setHiddenFieldValue(JSON.stringify(selections)); @@ -106,7 +138,7 @@ function ComboMultiple({ }); }; - const onSelect = (value) => { + const onSelect = (value: string) => { const maybeValue = [...extraOptions, ...optionsWithLabels].find( ([val]) => val == value ); @@ -134,8 +166,8 @@ function ComboMultiple({ hidePopover(); }; - const onRemove = (label) => { - const optionValue = optionValueByLabel(newValues, options, label); + const onRemove = (label: string) => { + const optionValue = optionValueByLabel(newValues, optionsWithLabels, label); if (optionValue) { saveSelection((selections) => selections.filter((value) => value != optionValue) @@ -144,10 +176,10 @@ function ComboMultiple({ newValues.filter((value) => value != optionValue) ); } - inputRef.current.focus(); + inputRef.current?.focus(); }; - const onKeyDown = (event) => { + const onKeyDown: KeyboardEventHandler = (event) => { if ( isHotkey('enter', event) || isHotkey(' ', event) || @@ -210,7 +242,11 @@ function ComboMultiple({ ))} @@ -263,31 +299,35 @@ function ComboMultiple({ ); } -function ComboboxTokenLabel({ onRemove, ...props }) { - const selectionsRef = useRef([]); +function ComboboxTokenLabel({ + onRemove, + children +}: { + onRemove: (value: string) => void; + children: ReactNode; +}) { + const selectionsRef = useRef([]); useLayoutEffect(() => { selectionsRef.current = []; - return () => (selectionsRef.current = []); - }); - - const context = { - onRemove, - selectionsRef - }; + return () => { + selectionsRef.current = []; + }; + }, []); return ( - -
+ +
{children}
 ); } -ComboboxTokenLabel.propTypes = { - onRemove: PropTypes.func -}; - -function ComboboxSeparator({ value }) { +function ComboboxSeparator({ value }: { value: string }) { return (
  • {value.slice(2, -2)} @@ -295,12 +335,17 @@ function ComboboxSeparator({ value }) { ); } -ComboboxSeparator.propTypes = { - value: PropTypes.string -}; - -function ComboboxToken({ value, describedby, ...props }) { - const { selectionsRef, onRemove } = useContext(Context); +function ComboboxToken({ + value, + describedby, + ...props +}: { + value: string; + describedby: string; +}) { + const context = useContext(Context); + invariant(context, 'invalid context'); + const { selectionsRef, onRemove } = context; useEffect(() => { selectionsRef.current.push(value); }); @@ -325,31 +370,3 @@ function ComboboxToken({ value, describedby, ...props }) {
    • ); } - -ComboboxToken.propTypes = { - value: PropTypes.string, - describedby: PropTypes.string -}; - -ComboMultiple.propTypes = { - options: PropTypes.oneOfType([ - PropTypes.arrayOf(PropTypes.string), - PropTypes.arrayOf( - PropTypes.arrayOf( - PropTypes.oneOfType([PropTypes.string, PropTypes.number]) - ) - ) - ]), - selected: PropTypes.arrayOf(PropTypes.string), - arraySelected: PropTypes.arrayOf(PropTypes.array), - acceptNewValues: PropTypes.bool, - mandatory: PropTypes.bool, - id: PropTypes.string, - group: PropTypes.string, - name: PropTypes.string, - labelledby: PropTypes.string, - describedby: PropTypes.string, - label: PropTypes.string -}; - -export default ComboMultiple; diff --git a/package.json b/package.json index 293825ee3..85524e4e0 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "@2fd/graphdoc": "^2.4.0", "@types/debounce": "^1.2.1", "@types/geojson": "^7946.0.8", + "@types/is-hotkey": "^0.1.7", "@types/mapbox__mapbox-gl-draw": "^1.2.3", "@types/rails__ujs": "^6.0.1", "@types/react": "^17.0.38", diff --git a/yarn.lock b/yarn.lock index cf3335137..92b14afda 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2383,6 +2383,11 @@ dependencies: "@types/node" "*" +"@types/is-hotkey@^0.1.7": + version "0.1.7" + resolved "https://registry.yarnpkg.com/@types/is-hotkey/-/is-hotkey-0.1.7.tgz#30ec6d4234895230b576728ef77e70a52962f3b3" + integrity sha512-yB5C7zcOM7idwYZZ1wKQ3pTfjA9BbvFqRWvKB46GFddxnJtHwi/b9y84ykQtxQPg5qhdpg4Q/kWU3EGoCTmLzQ== + "@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0": version "2.0.3" resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.3.tgz#4ba8ddb720221f432e443bd5f9117fd22cfd4762"