ListView should not handle the arrow keys if there is a modifier applied (#30633)

* ListView should not handle the arrow keys if there is a modifier applied.

* lint

* Reduce nesting
This commit is contained in:
David Langley 2025-08-27 16:48:33 +01:00 committed by GitHub
parent 98a04e1812
commit b7f89db43c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 73 additions and 22 deletions

View File

@ -184,28 +184,32 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
// Guard against null/undefined events and modified keys which we don't want to handle here but do // Guard against null/undefined events and modified keys which we don't want to handle here but do
// at the settings level shortcuts(E.g. Select next room, etc ) // at the settings level shortcuts(E.g. Select next room, etc )
if (e || !isModifiedKeyEvent(e)) { // Guard against null/undefined events and modified keys
if (e.code === Key.ARROW_UP && currentIndex !== undefined) { if (!e || isModifiedKeyEvent(e)) {
scrollToItem(currentIndex - 1, false); onKeyDown?.(e);
handled = true; return;
} else if (e.code === Key.ARROW_DOWN && currentIndex !== undefined) { }
scrollToItem(currentIndex + 1, true);
handled = true; if (e.code === Key.ARROW_UP && currentIndex !== undefined) {
} else if (e.code === Key.HOME) { scrollToItem(currentIndex - 1, false);
scrollToIndex(0); handled = true;
handled = true; } else if (e.code === Key.ARROW_DOWN && currentIndex !== undefined) {
} else if (e.code === Key.END) { scrollToItem(currentIndex + 1, true);
scrollToIndex(items.length - 1); handled = true;
handled = true; } else if (e.code === Key.HOME) {
} else if (e.code === Key.PAGE_DOWN && visibleRange && currentIndex !== undefined) { scrollToIndex(0);
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex; handled = true;
scrollToItem(Math.min(currentIndex + numberDisplayed, items.length - 1), true, `start`); } else if (e.code === Key.END) {
handled = true; scrollToIndex(items.length - 1);
} else if (e.code === Key.PAGE_UP && visibleRange && currentIndex !== undefined) { handled = true;
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex; } else if (e.code === Key.PAGE_DOWN && visibleRange && currentIndex !== undefined) {
scrollToItem(Math.max(currentIndex - numberDisplayed, 0), false, `start`); const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex;
handled = true; scrollToItem(Math.min(currentIndex + numberDisplayed, items.length - 1), true, `start`);
} handled = true;
} else if (e.code === Key.PAGE_UP && visibleRange && currentIndex !== undefined) {
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex;
scrollToItem(Math.max(currentIndex - numberDisplayed, 0), false, `start`);
handled = true;
} }
if (handled) { if (handled) {

View File

@ -195,6 +195,53 @@ describe("ListView", () => {
expect(items[lastIndex]).toHaveAttribute("tabindex", "-1"); expect(items[lastIndex]).toHaveAttribute("tabindex", "-1");
}); });
it("should not handle keyboard navigation when modifier keys are pressed", () => {
renderListViewWithHeight();
const container = screen.getByRole("grid");
fireEvent.focus(container);
// Store initial state - first item should be focused
const initialItems = container.querySelectorAll(".mx_item");
expect(initialItems[0]).toHaveAttribute("tabindex", "0");
expect(initialItems[2]).toHaveAttribute("tabindex", "-1");
// Test ArrowDown with Ctrl modifier - should NOT navigate
fireEvent.keyDown(container, { code: "ArrowDown", ctrlKey: true });
let items = container.querySelectorAll(".mx_item");
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
// Test ArrowDown with Alt modifier - should NOT navigate
fireEvent.keyDown(container, { code: "ArrowDown", altKey: true });
items = container.querySelectorAll(".mx_item");
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
// Test ArrowDown with Shift modifier - should NOT navigate
fireEvent.keyDown(container, { code: "ArrowDown", shiftKey: true });
items = container.querySelectorAll(".mx_item");
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
// Test ArrowDown with Meta/Cmd modifier - should NOT navigate
fireEvent.keyDown(container, { code: "ArrowDown", metaKey: true });
items = container.querySelectorAll(".mx_item");
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
// Test normal ArrowDown without modifiers - SHOULD navigate
fireEvent.keyDown(container, { code: "ArrowDown" });
items = container.querySelectorAll(".mx_item");
expect(items[0]).toHaveAttribute("tabindex", "-1"); // Should have moved from first item
expect(items[2]).toHaveAttribute("tabindex", "0"); // Should have moved to third item (skipping separator)
});
it("should skip non-focusable items when navigating down", async () => { it("should skip non-focusable items when navigating down", async () => {
// Create items where every other item is not focusable // Create items where every other item is not focusable
const mixedItems = [ const mixedItems = [