Skip to content

Commit

Permalink
fix(react): correctly validate selected option on select/combobox (#1850
Browse files Browse the repository at this point in the history
)
  • Loading branch information
josokinas authored Jul 25, 2022
1 parent 70f6308 commit 7b37a20
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 23 deletions.
54 changes: 35 additions & 19 deletions packages/react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import {
c,
classy,
ComboboxProps as BaseProps,
InputProps,
m,
PopoverProps,
SelectProps,
} from '@onfido/castor';
import {
Input,
Expand All @@ -15,6 +15,7 @@ import {
import React, {
ForwardedRef,
SyntheticEvent,
useCallback,
useMemo,
useRef,
useState,
Expand All @@ -25,7 +26,7 @@ import { OptionListInit } from '../option-list/options-list-init';

export interface ComboboxProps
extends BaseProps,
InputProps,
SelectProps,
PopoverProps,
Omit<JSX.IntrinsicElements['input'], 'type'> {
icon?: JSX.Element;
Expand Down Expand Up @@ -59,14 +60,15 @@ export const Combobox = withRef(function Combobox(
onClick,
onKeyUp,
position = 'bottom',
required,
selectedIcon,
value,
...restProps
}: ComboboxProps,
ref: ForwardedRef<HTMLDivElement>
) {
const inputRef = useRef<HTMLInputElement>(null);
const valueRef = useRef<HTMLInputElement>(null);
const selectRef = useRef<HTMLSelectElement>(null);
const optionsRef = useRef<HTMLDivElement>(null);
const [input, setInput] = useState('');
const [search, setSearch] = useState<string>();
Expand All @@ -77,18 +79,32 @@ export const Combobox = withRef(function Combobox(

const id = useMemo(() => `castor_combobox_${++idCount}`, [initialId]);

const propagateOnChange = useCallback(() => {
// propagate `onChange` manually because <select> won't naturally when its
// value is changed programatically by React, and on next tick, because
// React needs to update its value first
setTimeout(() =>
selectRef.current?.dispatchEvent(new Event('change', { bubbles: true }))
);
}, []);

const close = () => setOpen(false);

return (
<div ref={ref} className={classy(c('combobox'), m({ open }), className)}>
<input
ref={valueRef}
<select
ref={selectRef}
name={name}
disabled={disabled}
required={required}
hidden
name={name}
onChange={() => void 0}
value={selected.value || ''}
/>
onChange={
(onChange as JSX.IntrinsicElements['select']['onChange']) ??
(() => void 0)
}
>
{!selected.value || <option hidden value={selected.value} />}
</select>
<Input
{...restProps}
ref={inputRef}
Expand All @@ -98,6 +114,7 @@ export const Combobox = withRef(function Combobox(
placeholder={placeholder}
autoComplete="off"
disabled={disabled}
required={required}
onBlur={(event) => {
if (preventBlur.current) return (preventBlur.current = false);

Expand All @@ -109,7 +126,6 @@ export const Combobox = withRef(function Combobox(
onChange={(event) => {
setInput(event.target.value);
setSearch(event.target.value);
onChange?.(event);
}}
onClick={(event) => {
if (!open) {
Expand Down Expand Up @@ -182,16 +198,9 @@ export const Combobox = withRef(function Combobox(
setSelected(selected);
setInput(textContent(selected.option));
setSearch(undefined);
propagateOnChange();
focus(inputRef.current);
close();
// Propagate `onChange` manually because <input> won't naturally
// when its value is changed programatically by React, and on next
// tick because React needs to update its value first.
setTimeout(() =>
valueRef.current?.dispatchEvent(
new Event('change', { bubbles: true })
)
);
}}
onKeyDown={(event) => {
// ignore confirmation keys
Expand Down Expand Up @@ -232,7 +241,14 @@ export const Combobox = withRef(function Combobox(
<OptionListInit
defaultValue={defaultValue}
value={value}
onInit={(first) => setPlaceholder(textContent(first.option))}
onInit={(selected) => {
setSelected(selected);
if (!selected.value) setPlaceholder(textContent(selected.option));
else {
setInput(textContent(selected.option));
propagateOnChange();
}
}}
>
{children}
</OptionListInit>
Expand Down
13 changes: 9 additions & 4 deletions packages/react/src/components/select/custom/custom-select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ export const CustomSelect = withRef(function CustomSelect(
ref: ForwardedRef<HTMLSelectElement>
) {
const selectRef = useForwardedRef(ref);
const [selected, _setSelected] = useState<OptionListEvent>({});
const [selected, setSelected] = useState<OptionListEvent>({});

const setSelected = useCallback((selected) => {
_setSelected(selected);
const propagateOnChange = useCallback(() => {
// propagate `onChange` manually because <select> won't naturally when its
// value is changed programatically by React, and on next tick, because
// React needs to update its value first
Expand Down Expand Up @@ -132,6 +131,7 @@ export const CustomSelect = withRef(function CustomSelect(
value={selected.value ?? value ?? defaultValue}
onChange={(selected) => {
setSelected(selected);
propagateOnChange();
close();
}}
>
Expand All @@ -144,7 +144,12 @@ export const CustomSelect = withRef(function CustomSelect(
<OptionListInit
defaultValue={defaultValue}
value={value}
onInit={setSelected}
onInit={(selected) => {
setSelected(selected);
if (selected.value) {
propagateOnChange();
}
}}
>
{children}
</OptionListInit>
Expand Down

1 comment on commit 7b37a20

@vercel
Copy link

@vercel vercel bot commented on 7b37a20 Jul 25, 2022

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

castor – ./

castor-onfido-oss.vercel.app
castor.vercel.app
castor-git-main-onfido-oss.vercel.app

Please sign in to comment.