Skip to content

Commit

Permalink
fix(Combobox): ⚡ make controlled input adhere to inputValue and… (#…
Browse files Browse the repository at this point in the history
…2267)

… send all change events

resolves #2264
  • Loading branch information
Barsnes authored Aug 16, 2024
1 parent 1d79992 commit 2444bfe
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/long-boxes-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@digdir/designsystemet-react": patch
---

fix(Combobox): :zap: make controlled input adhere to `inputValue` and send all change events
20 changes: 20 additions & 0 deletions packages/react/src/components/form/Combobox/Combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,26 @@ describe('Combobox', () => {
});
});

it('should send `onChange` event when value changes', async () => {
const onChange = vi.fn();
const { user } = await render({ onChange });
const combobox = screen.getByRole('combobox');

await act(async () => await user.click(combobox));
await act(async () => await user.click(screen.getByText('Oslo')));

await vi.waitFor(() => {
/* we expect a change event with (e) => e.target.value === 'Oslo' */
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
target: expect.objectContaining({
value: 'Oslo',
}),
}),
);
});
});

it('should show a chip of a selected option in multiple mode', async () => {
await render({ multiple: true, value: ['leikanger'] });
expect(screen.getByText('Leikanger')).toBeInTheDocument();
Expand Down
18 changes: 13 additions & 5 deletions packages/react/src/components/form/Combobox/Combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type { Option } from './useCombobox';
import { useCombobox } from './useCombobox';
import { useComboboxKeyboard } from './useComboboxKeyboard';
import { useFloatingCombobox } from './useFloatingCombobox';
import { prefix, removePrefix } from './utilities';
import { prefix, removePrefix, setReactInputValue } from './utilities';

export type ComboboxProps = {
/**
Expand Down Expand Up @@ -161,6 +161,12 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(

const [inputValue, setInputValue] = useState<string>(rest.inputValue || '');

useEffect(() => {
if (typeof rest.inputValue === 'string') {
setInputValue(rest.inputValue);
}
}, [rest.inputValue]);

const {
selectedOptions,
options,
Expand Down Expand Up @@ -208,7 +214,8 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
useEffect(() => {
if (value && value.length > 0 && !multiple) {
const option = options[prefix(value[0])];
setInputValue(option?.label || '');
inputRef.current &&
setReactInputValue(inputRef.current, option?.label || '');
}
}, [multiple, value, options]);

Expand Down Expand Up @@ -239,7 +246,7 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
const { option, clear, remove } = args;
if (clear) {
setSelectedOptions({});
setInputValue('');
inputRef.current && setReactInputValue(inputRef.current, '');
onValueChange?.([]);
return;
}
Expand All @@ -264,15 +271,16 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
} else {
newSelectedOptions[prefix(option.value)] = option;
}
setInputValue('');
inputRef.current && setReactInputValue(inputRef.current, '');
inputRef.current?.focus();
} else {
/* clear newSelectedOptions */
for (const key of Object.keys(newSelectedOptions)) {
delete newSelectedOptions[key];
}
newSelectedOptions[prefix(option.value)] = option;
setInputValue(option?.label || '');
inputRef.current &&
setReactInputValue(inputRef.current, option?.label || '');
// move cursor to the end of the input
setTimeout(() => {
inputRef.current?.setSelectionRange(
Expand Down
22 changes: 22 additions & 0 deletions packages/react/src/components/form/Combobox/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,25 @@ export const prefix = (value?: string): string => {
export const removePrefix = (value: string): string => {
return value.slice(INTERNAL_OPTION_PREFIX.length);
};

// Workaround from https://github.com/equinor/design-system/blob/develop/packages/eds-utils/src/utils/setReactInputValue.ts
// React ignores 'dispathEvent' on input/textarea, see https://github.com/facebook/react/issues/10135
type ReactInternalHack = { _valueTracker?: { setValue: (a: string) => void } };

export const setReactInputValue = (
input: HTMLInputElement & ReactInternalHack,
value: string,
): void => {
const previousValue = input.value;

input.value = value;

const tracker = input._valueTracker;

if (typeof tracker !== 'undefined') {
tracker.setValue(previousValue);
}

//'change' instead of 'input', see https://github.com/facebook/react/issues/11488#issuecomment-381590324
input.dispatchEvent(new Event('change', { bubbles: true }));
};

0 comments on commit 2444bfe

Please sign in to comment.