Skip to content

fix: Open button dropdown when pressing up/down arrow keys #3821

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+126 −83
Draft

fix: Open button dropdown when pressing up/down arrow keys #3821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
159 changes: 90 additions & 69 deletions src/button-dropdown/__tests__/button-dropdown-keyboard.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,93 +29,114 @@ const items: ButtonDropdownProps.Items = [
onClickSpy = jest.fn();
onFollowSpy = jest.fn();
wrapper = renderButtonDropdown({ ...props, items, onItemClick: onClickSpy, onItemFollow: onFollowSpy });
act(() => wrapper.findNativeButton().keydown(KeyCode.enter));
});

test('should close the dropdown when escape is pressed', () => {
expect(wrapper.findOpenDropdown()).not.toBe(null);
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.escape));
test('should open the dropdown and focus first item when pressing down arrow key', () => {
expect(wrapper.findOpenDropdown()).toBe(null);
});
test('should select the next item if "down" is pressed', () => {
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item1');
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item2');
});

test('should select the previous item if "up" is pressed', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.up));
act(() => wrapper.findTriggerButton()!.keydown(KeyCode.down));
expect(wrapper.findOpenDropdown()).toBeTruthy();
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item1');
});

test('should include disabled items', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item3');
});

test('should call onClick on enter', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.enter));
expect(onClickSpy).toHaveBeenCalledTimes(1);
});

test('should not call onClick on enter a disabled item', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.enter));
expect(onClickSpy).not.toHaveBeenCalled();
});

test('should call onClick on space', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.space));
act(() => wrapper.findOpenDropdown()!.keyup(KeyCode.space));
expect(onClickSpy).toHaveBeenCalledTimes(1);
test('should open the dropdown and focus last item when pressing up arrow key', () => {
expect(wrapper.findOpenDropdown()).toBe(null);
act(() => wrapper.findTriggerButton()!.keydown(KeyCode.up));
expect(wrapper.findOpenDropdown()).toBeTruthy();
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item5');
});

test('should not call onClick on space a disabled item', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.space));
expect(onClickSpy).not.toHaveBeenCalled();
});
describe('when dropdown is open', () => {
beforeEach(() => {
act(() => wrapper.findNativeButton().keydown(KeyCode.enter));
});

test('should close the dropdown when escape is pressed', () => {
expect(wrapper.findOpenDropdown()).not.toBe(null);
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.escape));
expect(wrapper.findOpenDropdown()).toBe(null);
});
test('should select the next item if "down" is pressed', () => {
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item1');
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item2');
});

test('should call onFollow on space if item has href', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.space));
act(() => wrapper.findOpenDropdown()!.keyup(KeyCode.space));
expect(onFollowSpy).toHaveBeenCalledTimes(1);
});
test('should select the previous item if "up" is pressed', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.up));
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item1');
});

test.each([KeyCode.enter, KeyCode.space])(
'should fire event correctly when items with checkbox pressed using key=%s',
keyCode => {
test('should include disabled items', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
expect(wrapper.findHighlightedItem()!.getElement()).toHaveTextContent('item3');
});

test('should call onClick on enter', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.enter));
expect(onClickSpy).toHaveBeenCalledTimes(1);
});

test('should not call onClick on enter a disabled item', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.enter));
expect(onClickSpy).not.toHaveBeenCalled();
});

// Fire keydown on the 5th element, checkbox should be false after click
act(() => wrapper.findItems()[4]!.keydown(keyCode));
// Space handling is triggered on keyup
if (keyCode === KeyCode.space) {
act(() => wrapper.findItems()[4]!.keyup(keyCode));
}
test('should call onClick on space', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.space));
act(() => wrapper.findOpenDropdown()!.keyup(KeyCode.space));
expect(onClickSpy).toHaveBeenCalledTimes(1);
expect(onClickSpy).toHaveBeenCalledWith(expect.objectContaining({ detail: { id: 'i5', checked: false } }));
});

// Open button dropdown again
act(() => wrapper.findNativeButton().keydown(KeyCode.enter));
test('should not call onClick on space a disabled item', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.space));
expect(onClickSpy).not.toHaveBeenCalled();
});

// Fire keydown on the 1st element, checked should be undefined
act(() => wrapper.findItems()[0]!.keydown(keyCode));
// Space handling is triggered on keyup
if (keyCode === KeyCode.space) {
act(() => wrapper.findItems()[0]!.keyup(keyCode));
test('should call onFollow on space if item has href', () => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.space));
act(() => wrapper.findOpenDropdown()!.keyup(KeyCode.space));
expect(onFollowSpy).toHaveBeenCalledTimes(1);
});

test.each([KeyCode.enter, KeyCode.space])(
'should fire event correctly when items with checkbox pressed using key=%s',
keyCode => {
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));
act(() => wrapper.findOpenDropdown()!.keydown(KeyCode.down));

// Fire keydown on the 5th element, checkbox should be false after click
act(() => wrapper.findItems()[4]!.keydown(keyCode));
// Space handling is triggered on keyup
if (keyCode === KeyCode.space) {
act(() => wrapper.findItems()[4]!.keyup(keyCode));
}
expect(onClickSpy).toHaveBeenCalledTimes(1);
expect(onClickSpy).toHaveBeenCalledWith(expect.objectContaining({ detail: { id: 'i5', checked: false } }));

// Open button dropdown again
act(() => wrapper.findNativeButton().keydown(KeyCode.enter));

// Fire keydown on the 1st element, checked should be undefined
act(() => wrapper.findItems()[0]!.keydown(keyCode));
// Space handling is triggered on keyup
if (keyCode === KeyCode.space) {
act(() => wrapper.findItems()[0]!.keyup(keyCode));
}
expect(onClickSpy).toHaveBeenCalledTimes(2);
expect(onClickSpy).toHaveBeenCalledWith(
expect.objectContaining({ detail: { id: 'i1', checked: undefined } })
);
}
expect(onClickSpy).toHaveBeenCalledTimes(2);
expect(onClickSpy).toHaveBeenCalledWith(expect.objectContaining({ detail: { id: 'i1', checked: undefined } }));
}
);
);
});
});
});
10 changes: 10 additions & 0 deletions src/button-dropdown/__tests__/create-items-tree.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,14 @@ describe('create-items-tree util', () => {
expect(tree.getSequentialIndex([2, 0], -1)).toEqual([2]);
expect(tree.getSequentialIndex([0, 0], -1)).toEqual(null);
});

test('increment with looping', () => {
const tree = createItemsTree(items);
expect(tree.getSequentialIndex([4, 1], 1, true)).toEqual([0]);
});

test('decrement with looping', () => {
const tree = createItemsTree(items);
expect(tree.getSequentialIndex([0, 0], -1, true)).toEqual([4, 1]);
});
});
14 changes: 11 additions & 3 deletions src/button-dropdown/utils/create-items-tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface ItemsTreeApi {
// in the tree (referential comparison), or an error will be thrown
getItemIndex: (item: ButtonDropdownProps.ItemOrGroup) => TreeIndex;
// Returns the index of next or previous sequential node or null if out of bounds
getSequentialIndex: (index: TreeIndex, direction: -1 | 1) => TreeIndex | null;
getSequentialIndex: (index: TreeIndex, direction: -1 | 1, loop?: boolean) => TreeIndex | null;
// Returns parent tree index of a given item or null if no parent is present
getParentIndex: (item: ButtonDropdownProps.ItemOrGroup) => TreeIndex | null;
}
Expand Down Expand Up @@ -48,11 +48,19 @@ export default function createItemsTree(items: ButtonDropdownProps.Items): Items

return parseIndex(indexKey);
},
getSequentialIndex: (index: TreeIndex, direction: -1 | 1): TreeIndex | null => {
getSequentialIndex: (index: TreeIndex, direction: -1 | 1, loop = false): TreeIndex | null => {
const indexKey = stringifyIndex(index);
const position = flatIndices.indexOf(indexKey);

const nextIndexKey = flatIndices[position + direction];
let nextIndex = position + direction;
if (loop) {
if (nextIndex < 0) {
nextIndex = flatIndices.length - 1;
} else if (nextIndex >= flatIndices.length) {
nextIndex = 0;
}
}
const nextIndexKey = flatIndices[nextIndex];

if (!nextIndexKey) {
return null;
Expand Down
20 changes: 12 additions & 8 deletions src/button-dropdown/utils/use-button-dropdown.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,6 @@ export function useButtonDropdown({
closeDropdown();
};

const doVerticalNavigation = (direction: -1 | 1) => {
if (isOpen) {
moveHighlight(direction);
}
};

const openAndSelectFirst = (event: React.KeyboardEvent) => {
toggleDropdown();
event.preventDefault();
Expand Down Expand Up @@ -129,12 +123,22 @@ export function useButtonDropdown({
setIsUsingMouse(false);
switch (event.keyCode) {
case KeyCode.down: {
doVerticalNavigation(1);
if (!isOpen) {
toggleDropdown();
moveHighlight(1, true);
} else {
moveHighlight(1);
}
event.preventDefault();
break;
}
case KeyCode.up: {
doVerticalNavigation(-1);
if (!isOpen) {
toggleDropdown();
moveHighlight(-1, true);
} else {
moveHighlight(-1);
}
event.preventDefault();
break;
}
Expand Down
6 changes: 3 additions & 3 deletions src/button-dropdown/utils/use-highlighted-menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface UseHighlightedMenuOptions {
}

interface UseHighlightedMenuApi extends HighlightProps {
moveHighlight: (direction: -1 | 1) => void;
moveHighlight: (direction: -1 | 1, loop?: boolean) => void;
expandGroup: (group?: ButtonDropdownProps.ItemGroup) => void;
collapseGroup: () => void;
reset: () => void;
Expand Down Expand Up @@ -61,9 +61,9 @@ export default function useHighlightedMenu({
);

const moveHighlight = useCallback(
(direction: -1 | 1) => {
(direction: -1 | 1, loop?: boolean) => {
const getNext = (index: TreeIndex) => {
const nextIndex = getSequentialIndex(index, direction);
const nextIndex = getSequentialIndex(index, direction, loop);
const item = getItem(nextIndex || [-1]);

if (!nextIndex || !item) {
Expand Down
Loading