Skip to content

Commit 7fec7ff

Browse files
authored
Fixing typeselect for falsy key values (#1123)
1 parent 9ea19f2 commit 7fec7ff

File tree

3 files changed

+24
-5
lines changed

3 files changed

+24
-5
lines changed

packages/@react-aria/selection/src/useTypeSelect.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,11 @@ export function useTypeSelect(options: TypeSelectOptions): TypeSelectAria {
6868
let key = keyboardDelegate.getKeyForSearch(state.search, selectionManager.focusedKey);
6969

7070
// If no key found, search from the top.
71-
key = key || keyboardDelegate.getKeyForSearch(state.search);
71+
if (key == null) {
72+
key = keyboardDelegate.getKeyForSearch(state.search);
73+
}
7274

73-
if (key) {
75+
if (key != null) {
7476
selectionManager.setFocusedKey(key);
7577
if (onTypeSelect) {
7678
onTypeSelect(key);
@@ -86,7 +88,7 @@ export function useTypeSelect(options: TypeSelectOptions): TypeSelectAria {
8688
return {
8789
typeSelectProps: {
8890
// Using a capturing listener to catch the keydown event before
89-
// other hooks in order to handle the Spacebar event.
91+
// other hooks in order to handle the Spacebar event.
9092
onKeyDownCapture: keyboardDelegate.getKeyForSearch ? onKeyDown : null
9193
}
9294
};

packages/@react-aria/table/src/TableKeyboardDelegate.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ export class TableKeyboardDelegate<T> implements KeyboardDelegate {
405405
}
406406

407407
let hasWrapped = false;
408-
while (key) {
408+
while (key != null) {
409409
let item = collection.getItem(key);
410410

411411
// Check each of the row header cells in this row for a match

packages/@react-spectrum/picker/test/Picker.test.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1481,6 +1481,7 @@ describe('Picker', function () {
14811481
<Item key="one">One</Item>
14821482
<Item key="two">Two</Item>
14831483
<Item key="three">Three</Item>
1484+
<Item key="">None</Item>
14841485
</Picker>
14851486
</Provider>
14861487
);
@@ -1493,7 +1494,7 @@ describe('Picker', function () {
14931494

14941495
let listbox = getByRole('listbox');
14951496
let items = within(listbox).getAllByRole('option');
1496-
expect(items.length).toBe(3);
1497+
expect(items.length).toBe(4);
14971498
expect(items[0]).toHaveTextContent('One');
14981499
expect(items[1]).toHaveTextContent('Two');
14991500
expect(items[2]).toHaveTextContent('Three');
@@ -1517,6 +1518,22 @@ describe('Picker', function () {
15171518

15181519
expect(document.activeElement).toBe(picker);
15191520
expect(picker).toHaveTextContent('Three');
1521+
1522+
act(() => jest.advanceTimersByTime(500));
1523+
act(() => picker.focus());
1524+
act(() => {fireEvent.keyDown(picker, {key: 'ArrowDown'});});
1525+
act(() => jest.runAllTimers());
1526+
listbox = getByRole('listbox');
1527+
items = within(listbox).getAllByRole('option');
1528+
expect(document.activeElement).toBe(items[2]);
1529+
act(() => {fireEvent.keyDown(listbox, {key: 'n'});});
1530+
act(() => {fireEvent.keyDown(document.activeElement, {key: 'Enter'});});
1531+
act(() => {fireEvent.keyUp(document.activeElement, {key: 'Enter'});});
1532+
act(() => jest.runAllTimers());
1533+
expect(listbox).not.toBeInTheDocument();
1534+
expect(picker).toHaveTextContent('None');
1535+
expect(onSelectionChange).toHaveBeenCalledTimes(2);
1536+
expect(onSelectionChange).toHaveBeenLastCalledWith('');
15201537
});
15211538

15221539
it('does not deselect when pressing an already selected item', function () {

0 commit comments

Comments
 (0)