Skip to content
Open
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
4 changes: 3 additions & 1 deletion packages/@react-aria/combobox/src/useComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ export function useComboBox<T, M extends SelectionMode = 'single'>(props: AriaCo
break;
}
}
state.commit();
if (e.key === 'Enter' || state.isOpen) {
state.commit();
}
break;
case 'Escape':
if (
Expand Down
17 changes: 17 additions & 0 deletions packages/@react-aria/combobox/test/useComboBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,23 @@ describe('useComboBox', function () {
expect(preventDefault).toHaveBeenCalledTimes(1);
});

it('should only call commit on Tab when the menu is open', function () {
Copy link
Member

Choose a reason for hiding this comment

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

is this still true if it allows a custom value?

Copy link
Member

Choose a reason for hiding this comment

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

I think useComboBoxState will still clear the selected key on field blur via

still

let commitSpy = jest.fn();
let {result: state} = renderHook((props) => useComboBoxState(props), {initialProps: props});
let closedState = {...state.current, isOpen: false, commit: commitSpy};
let {result: closedResult} = renderHook((props) => useComboBox(props, closedState), {initialProps: props});
act(() => {
closedResult.current.inputProps.onKeyDown(event({key: 'Tab'}));
});
expect(commitSpy).toHaveBeenCalledTimes(0);
let openState = {...state.current, isOpen: true, commit: commitSpy};
let {result: openResult} = renderHook((props) => useComboBox(props, openState), {initialProps: props});
act(() => {
openResult.current.inputProps.onKeyDown(event({key: 'Tab'}));
});
expect(commitSpy).toHaveBeenCalledTimes(1);
});

it('calls open and toggle with the expected parameters when arrow down/up/trigger button is pressed', function () {
let {result: state} = renderHook((props) => useComboBoxState(props), {initialProps: props});
state.current.open = openSpy;
Expand Down
19 changes: 13 additions & 6 deletions packages/@react-stately/combobox/src/useComboBoxState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,21 @@ export function useComboBoxState<T extends object, M extends SelectionMode = 'si
closeMenu();
};

let commitSelection = () => {
// If multiple things are controlled, call onSelectionChange
let commitSelection = (shouldForceSelectionChange = false) => {
// If multiple things are controlled, call onSelectionChange only when selecting the focused item,
// or when inputValue needs to be synced back to the selected item on commit/blur.
if (value !== undefined && props.inputValue !== undefined) {
props.onSelectionChange?.(selectedKey);
props.onChange?.(displayValue);
let itemText = selectedKey != null ? collection.getItem(selectedKey)?.textValue ?? '' : '';
if (
shouldForceSelectionChange ||
selectionMode === 'multiple' ||
inputValue !== itemText
) {
props.onSelectionChange?.(selectedKey);
props.onChange?.(displayValue);
}

// Stop menu from reopening from useEffect
let itemText = selectedKey != null ? collection.getItem(selectedKey)?.textValue ?? '' : '';
setLastValue(itemText);
closeMenu();
} else {
Expand All @@ -401,7 +408,7 @@ export function useComboBoxState<T extends object, M extends SelectionMode = 'si
// Reset inputValue and close menu here if the selected key is already the focused key. Otherwise
// fire onSelectionChange to allow the application to control the closing.
if (selectionManager.isSelected(selectionManager.focusedKey) && selectionMode === 'single') {
commitSelection();
commitSelection(true);
} else {
selectionManager.select(selectionManager.focusedKey);
}
Expand Down
17 changes: 17 additions & 0 deletions packages/@react-stately/combobox/test/useComboBoxState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,23 @@ describe('useComboBoxState tests', function () {
expect(onSelectionChange).toHaveBeenCalledWith('1');
});

it('does not fire onSelectionChange on blur when fully controlled values are already synced', function () {
let initialProps = {...defaultProps, selectedKey: '1', inputValue: 'onomatopoeia'};
let {result} = renderHook((props) => useComboBoxState(props), {initialProps});

act(() => {result.current.setFocused(false);});
expect(onSelectionChange).not.toHaveBeenCalled();
});

it('fires onSelectionChange on blur when fully controlled inputValue is out of sync', function () {
let initialProps = {...defaultProps, selectedKey: '1', inputValue: 'onom'};
let {result} = renderHook((props) => useComboBoxState(props), {initialProps});

act(() => {result.current.setFocused(false);});
expect(onSelectionChange).toHaveBeenCalledTimes(1);
expect(onSelectionChange).toHaveBeenCalledWith('1');
});

it('won\'t update the returned collection if the combobox is closed (uncontrolled items)', function () {
let filter = renderHook((props) => useFilter(props), {sensitivity: 'base'});
let initialProps = {...defaultProps, items: null, defaultItems: [{id: '0', name: 'one'}, {id: '1', name: 'onomatopoeia'}], defaultInputValue: '', defaultFilter: filter.result.current.contains};
Expand Down
57 changes: 57 additions & 0 deletions packages/react-aria-components/test/ComboBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,63 @@ describe('ComboBox', () => {
expect(document.querySelector('input[type=hidden]')).toBeNull();
});

it.each(['click', 'tab'])('should not fire extra onSelectionChange calls after focus moves away in fully controlled mode via %s', async (focusMove) => {
let onSelectionChange = jest.fn();
let keyToText = {
'1': 'Cat',
'2': 'Dog',
'3': 'Kangaroo'
};

function ControlledComboBox() {
let [selectedKey, setSelectedKey] = useState(null);
let [inputValue, setInputValue] = useState('');

return (
<>
<ComboBox
value={selectedKey}
inputValue={inputValue}
onChange={(key) => {
onSelectionChange(key);
setSelectedKey(key);
setInputValue(key != null ? keyToText[key] : '');
}}
onInputChange={setInputValue}>
<Label>Favorite Animal</Label>
<Input />
<Button />
<Popover>
<ListBox>
<ListBoxItem id="1">Cat</ListBoxItem>
<ListBoxItem id="2">Dog</ListBoxItem>
<ListBoxItem id="3">Kangaroo</ListBoxItem>
</ListBox>
</Popover>
</ComboBox>
<button type="button">Next</button>
</>
);
}

let tree = render(<ControlledComboBox />);
let comboboxTester = testUtilUser.createTester('ComboBox', {root: tree.container});

await comboboxTester.selectOption({option: 'Dog'});
expect(onSelectionChange).toHaveBeenCalledTimes(1);

if (focusMove === 'click') {
await user.click(tree.getByRole('button', {name: 'Next'}));
} else {
act(() => {
comboboxTester.combobox.focus();
});
await user.tab();
}

expect(onSelectionChange).toHaveBeenCalledTimes(1);
});

it('should support form reset', async () => {
const tree = render(
<form>
Expand Down