Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Combobox): ⚡ make controlled input adhere to inputValue and… #2267

Merged
merged 4 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 }));
};
Loading