Skip to content

Commit b7f89db

Browse files
authored
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
1 parent 98a04e1 commit b7f89db

File tree

2 files changed

+73
-22
lines changed

2 files changed

+73
-22
lines changed

src/components/utils/ListView.tsx

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -184,28 +184,32 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
184184

185185
// Guard against null/undefined events and modified keys which we don't want to handle here but do
186186
// at the settings level shortcuts(E.g. Select next room, etc )
187-
if (e || !isModifiedKeyEvent(e)) {
188-
if (e.code === Key.ARROW_UP && currentIndex !== undefined) {
189-
scrollToItem(currentIndex - 1, false);
190-
handled = true;
191-
} else if (e.code === Key.ARROW_DOWN && currentIndex !== undefined) {
192-
scrollToItem(currentIndex + 1, true);
193-
handled = true;
194-
} else if (e.code === Key.HOME) {
195-
scrollToIndex(0);
196-
handled = true;
197-
} else if (e.code === Key.END) {
198-
scrollToIndex(items.length - 1);
199-
handled = true;
200-
} else if (e.code === Key.PAGE_DOWN && visibleRange && currentIndex !== undefined) {
201-
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex;
202-
scrollToItem(Math.min(currentIndex + numberDisplayed, items.length - 1), true, `start`);
203-
handled = true;
204-
} else if (e.code === Key.PAGE_UP && visibleRange && currentIndex !== undefined) {
205-
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex;
206-
scrollToItem(Math.max(currentIndex - numberDisplayed, 0), false, `start`);
207-
handled = true;
208-
}
187+
// Guard against null/undefined events and modified keys
188+
if (!e || isModifiedKeyEvent(e)) {
189+
onKeyDown?.(e);
190+
return;
191+
}
192+
193+
if (e.code === Key.ARROW_UP && currentIndex !== undefined) {
194+
scrollToItem(currentIndex - 1, false);
195+
handled = true;
196+
} else if (e.code === Key.ARROW_DOWN && currentIndex !== undefined) {
197+
scrollToItem(currentIndex + 1, true);
198+
handled = true;
199+
} else if (e.code === Key.HOME) {
200+
scrollToIndex(0);
201+
handled = true;
202+
} else if (e.code === Key.END) {
203+
scrollToIndex(items.length - 1);
204+
handled = true;
205+
} else if (e.code === Key.PAGE_DOWN && visibleRange && currentIndex !== undefined) {
206+
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex;
207+
scrollToItem(Math.min(currentIndex + numberDisplayed, items.length - 1), true, `start`);
208+
handled = true;
209+
} else if (e.code === Key.PAGE_UP && visibleRange && currentIndex !== undefined) {
210+
const numberDisplayed = visibleRange.endIndex - visibleRange.startIndex;
211+
scrollToItem(Math.max(currentIndex - numberDisplayed, 0), false, `start`);
212+
handled = true;
209213
}
210214

211215
if (handled) {

test/unit-tests/components/views/utils/ListView-test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,53 @@ describe("ListView", () => {
195195
expect(items[lastIndex]).toHaveAttribute("tabindex", "-1");
196196
});
197197

198+
it("should not handle keyboard navigation when modifier keys are pressed", () => {
199+
renderListViewWithHeight();
200+
const container = screen.getByRole("grid");
201+
202+
fireEvent.focus(container);
203+
204+
// Store initial state - first item should be focused
205+
const initialItems = container.querySelectorAll(".mx_item");
206+
expect(initialItems[0]).toHaveAttribute("tabindex", "0");
207+
expect(initialItems[2]).toHaveAttribute("tabindex", "-1");
208+
209+
// Test ArrowDown with Ctrl modifier - should NOT navigate
210+
fireEvent.keyDown(container, { code: "ArrowDown", ctrlKey: true });
211+
212+
let items = container.querySelectorAll(".mx_item");
213+
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
214+
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
215+
216+
// Test ArrowDown with Alt modifier - should NOT navigate
217+
fireEvent.keyDown(container, { code: "ArrowDown", altKey: true });
218+
219+
items = container.querySelectorAll(".mx_item");
220+
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
221+
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
222+
223+
// Test ArrowDown with Shift modifier - should NOT navigate
224+
fireEvent.keyDown(container, { code: "ArrowDown", shiftKey: true });
225+
226+
items = container.querySelectorAll(".mx_item");
227+
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
228+
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
229+
230+
// Test ArrowDown with Meta/Cmd modifier - should NOT navigate
231+
fireEvent.keyDown(container, { code: "ArrowDown", metaKey: true });
232+
233+
items = container.querySelectorAll(".mx_item");
234+
expect(items[0]).toHaveAttribute("tabindex", "0"); // Should still be on first item
235+
expect(items[2]).toHaveAttribute("tabindex", "-1"); // Should not have moved to third item
236+
237+
// Test normal ArrowDown without modifiers - SHOULD navigate
238+
fireEvent.keyDown(container, { code: "ArrowDown" });
239+
240+
items = container.querySelectorAll(".mx_item");
241+
expect(items[0]).toHaveAttribute("tabindex", "-1"); // Should have moved from first item
242+
expect(items[2]).toHaveAttribute("tabindex", "0"); // Should have moved to third item (skipping separator)
243+
});
244+
198245
it("should skip non-focusable items when navigating down", async () => {
199246
// Create items where every other item is not focusable
200247
const mixedItems = [

0 commit comments

Comments
 (0)