Improve keyboard navigation on invite dialog (#30930)

* fix: improve keyboard navigation on `RichList`

* test: list focus handling

* test: update snapshot

* refactor: rename `useListKeydown` to `useListKeyboardNavigation`
This commit is contained in:
Florian Duros
2025-10-01 17:26:34 +02:00
committed by GitHub
parent 2d5f1b3fb7
commit f9e718644a
6 changed files with 60 additions and 16 deletions
@@ -8,7 +8,7 @@
import { type KeyboardEvent } from "react"; import { type KeyboardEvent } from "react";
import { renderHook } from "jest-matrix-react"; import { renderHook } from "jest-matrix-react";
import { useListKeyDown } from "./useListKeyDown"; import { useListKeyboardNavigation } from "./useListKeyboardNavigation";
describe("useListKeyDown", () => { describe("useListKeyDown", () => {
let mockList: HTMLUListElement; let mockList: HTMLUListElement;
@@ -51,9 +51,10 @@ describe("useListKeyDown", () => {
current: { current: {
listRef: React.RefObject<HTMLUListElement | null>; listRef: React.RefObject<HTMLUListElement | null>;
onKeyDown: React.KeyboardEventHandler<HTMLUListElement>; onKeyDown: React.KeyboardEventHandler<HTMLUListElement>;
onFocus: React.FocusEventHandler<HTMLUListElement>;
}; };
} { } {
const { result } = renderHook(() => useListKeyDown()); const { result } = renderHook(() => useListKeyboardNavigation());
result.current.listRef.current = mockList; result.current.listRef.current = mockList;
return result; return result;
} }
@@ -137,4 +138,18 @@ describe("useListKeyDown", () => {
expect(mockEvent.preventDefault).not.toHaveBeenCalled(); expect(mockEvent.preventDefault).not.toHaveBeenCalled();
}); });
it("should focus the first item if list itself is focused", () => {
const result = render();
result.current.onFocus({ target: mockList } as React.FocusEvent<HTMLUListElement>);
expect(mockItems[0].focus).toHaveBeenCalledTimes(1);
});
it("should focus the selected item if list itself is focused", () => {
mockItems[1].setAttribute("aria-selected", "true");
const result = render();
result.current.onFocus({ target: mockList } as React.FocusEvent<HTMLUListElement>);
expect(mockItems[1].focus).toHaveBeenCalledTimes(1);
});
}); });
@@ -5,17 +5,45 @@
* Please see LICENSE files in the repository root for full details. * Please see LICENSE files in the repository root for full details.
*/ */
import { useCallback, useRef, type RefObject, type KeyboardEvent, type KeyboardEventHandler } from "react"; import {
useCallback,
useRef,
type RefObject,
type KeyboardEvent,
type KeyboardEventHandler,
type FocusEventHandler,
type FocusEvent,
} from "react";
/** /**
* A hook that provides keyboard navigation for a list of options. * A hook that provides keyboard navigation for a list of options.
*/ */
export function useListKeyDown(): { export function useListKeyboardNavigation(): {
listRef: RefObject<HTMLUListElement | null>; listRef: RefObject<HTMLUListElement | null>;
onKeyDown: KeyboardEventHandler<HTMLUListElement>; onKeyDown: KeyboardEventHandler<HTMLUListElement>;
onFocus: FocusEventHandler<HTMLUListElement>;
} { } {
const listRef = useRef<HTMLUListElement>(null); const listRef = useRef<HTMLUListElement>(null);
const onFocus = useCallback((evt: FocusEvent<HTMLUListElement>) => {
if (!listRef.current) return;
if (evt.target === listRef.current) {
// By default, focus the selected item
let selectedChild = listRef.current?.firstElementChild;
// If there is a selected item, focus that instead
for (const child of listRef.current.children) {
if (child.getAttribute("aria-selected") === "true") {
selectedChild = child;
break;
}
}
(selectedChild as HTMLElement)?.focus();
}
}, []);
const onKeyDown = useCallback((evt: KeyboardEvent<HTMLUListElement>) => { const onKeyDown = useCallback((evt: KeyboardEvent<HTMLUListElement>) => {
const { key } = evt; const { key } = evt;
@@ -60,5 +88,5 @@ export function useListKeyDown(): {
evt.preventDefault(); evt.preventDefault();
} }
}, []); }, []);
return { listRef, onKeyDown }; return { listRef, onKeyDown, onFocus };
} }
@@ -67,7 +67,7 @@ export const RichItem = memo(function RichItem({
<li <li
className={styles.richItem} className={styles.richItem}
role="option" role="option"
tabIndex={0} tabIndex={-1}
aria-selected={selected} aria-selected={selected}
aria-label={title} aria-label={title}
{...props} {...props}
@@ -10,7 +10,7 @@ exports[`RichItem renders the item in default state 1`] = `
aria-label="Rich Item Title" aria-label="Rich Item Title"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
class="flex avatar" class="flex avatar"
@@ -52,7 +52,7 @@ exports[`RichItem renders the item in selected state 1`] = `
aria-selected="true" aria-selected="true"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
aria-hidden="true" aria-hidden="true"
@@ -103,7 +103,7 @@ exports[`RichItem renders the item without timestamp 1`] = `
aria-label="Rich Item Title" aria-label="Rich Item Title"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
class="flex avatar" class="flex avatar"
@@ -10,7 +10,7 @@ import classNames from "classnames";
import styles from "./RichList.module.css"; import styles from "./RichList.module.css";
import { Flex } from "../../utils/Flex"; import { Flex } from "../../utils/Flex";
import { useListKeyDown } from "../../hooks/useListKeyDown"; import { useListKeyboardNavigation } from "../../hooks/useListKeyboardNavigation";
export interface RichListProps extends HTMLProps<HTMLDivElement> { export interface RichListProps extends HTMLProps<HTMLDivElement> {
/** /**
@@ -53,7 +53,7 @@ export function RichList({
...props ...props
}: PropsWithChildren<RichListProps>): JSX.Element { }: PropsWithChildren<RichListProps>): JSX.Element {
const id = useId(); const id = useId();
const { listRef, onKeyDown } = useListKeyDown(); const { listRef, onKeyDown, onFocus } = useListKeyboardNavigation();
return ( return (
<Flex className={classNames(styles.richList, className)} direction="column" {...props}> <Flex className={classNames(styles.richList, className)} direction="column" {...props}>
@@ -70,6 +70,7 @@ export function RichList({
aria-labelledby={id} aria-labelledby={id}
tabIndex={0} tabIndex={0}
onKeyDown={onKeyDown} onKeyDown={onKeyDown}
onFocus={onFocus}
> >
{children} {children}
</ul> </ul>
@@ -25,7 +25,7 @@ exports[`RichItem renders the list 1`] = `
aria-label="First Item" aria-label="First Item"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
class="flex avatar" class="flex avatar"
@@ -51,7 +51,7 @@ exports[`RichItem renders the list 1`] = `
aria-selected="true" aria-selected="true"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
aria-hidden="true" aria-hidden="true"
@@ -86,7 +86,7 @@ exports[`RichItem renders the list 1`] = `
aria-label="Third Item" aria-label="Third Item"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
class="flex avatar" class="flex avatar"
@@ -111,7 +111,7 @@ exports[`RichItem renders the list 1`] = `
aria-label="Fourth Item" aria-label="Fourth Item"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
class="flex avatar" class="flex avatar"
@@ -136,7 +136,7 @@ exports[`RichItem renders the list 1`] = `
aria-label="Fifth Item" aria-label="Fifth Item"
class="richItem" class="richItem"
role="option" role="option"
tabindex="0" tabindex="-1"
> >
<div <div
class="flex avatar" class="flex avatar"