Fix a11y issue on list in invite dialog (#30878)

* fix: focus decoration when tabbing on rich item

* feat: add `useListKeyDown` hook

* fix: improve keyboard navigation on `RichList` and `RichItem`

Author: florianduros

src/shared-components/hooks/useListKeyDown.test.ts

@@ -0,0 +1,140 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import { type KeyboardEvent } from "react";
+import { renderHook } from "jest-matrix-react";
+
+import { useListKeyDown } from "./useListKeyDown";
+
+describe("useListKeyDown", () => {
+    let mockList: HTMLUListElement;
+    let mockItems: HTMLElement[];
+    let mockEvent: Partial<KeyboardEvent<HTMLUListElement>>;
+
+    beforeEach(() => {
+        // Create mock DOM elements
+        mockList = document.createElement("ul");
+        mockItems = [document.createElement("li"), document.createElement("li"), document.createElement("li")];
+
+        // Set up the DOM structure
+        mockItems.forEach((item, index) => {
+            item.setAttribute("tabindex", "0");
+            item.setAttribute("data-testid", `item-${index}`);
+            mockList.appendChild(item);
+        });
+
+        document.body.appendChild(mockList);
+
+        // Mock event object
+        mockEvent = {
+            preventDefault: jest.fn(),
+            key: "",
+        };
+
+        // Mock focus methods
+        mockItems.forEach((item) => {
+            item.focus = jest.fn();
+            item.click = jest.fn();
+        });
+    });
+
+    afterEach(() => {
+        document.body.removeChild(mockList);
+        jest.clearAllMocks();
+    });
+
+    function render(): {
+        current: {
+            listRef: React.RefObject<HTMLUListElement | null>;
+            onKeyDown: React.KeyboardEventHandler<HTMLUListElement>;
+        };
+    } {
+        const { result } = renderHook(() => useListKeyDown());
+        result.current.listRef.current = mockList;
+        return result;
+    }
+
+    it.each([
+        ["Enter", "Enter"],
+        ["Space", " "],
+    ])("should handle %s key to click active element", (name, key) => {
+        const result = render();
+
+        // Mock document.activeElement
+        Object.defineProperty(document, "activeElement", {
+            value: mockItems[1],
+            configurable: true,
+        });
+
+        // Simulate key press
+        result.current.onKeyDown({
+            ...mockEvent,
+            key,
+        } as KeyboardEvent<HTMLUListElement>);
+
+        expect(mockItems[1].click).toHaveBeenCalledTimes(1);
+        expect(mockEvent.preventDefault).toHaveBeenCalledTimes(1);
+    });
+
+    it.each(
+        // key, finalPosition, startPosition
+        [
+            ["ArrowDown", 1, 0],
+            ["ArrowUp", 1, 2],
+            ["Home", 0, 1],
+            ["End", 2, 1],
+        ],
+    )("should handle %s to focus the %inth element", (key, finalPosition, startPosition) => {
+        const result = render();
+        mockList.contains = jest.fn().mockReturnValue(true);
+
+        Object.defineProperty(document, "activeElement", {
+            value: mockItems[startPosition],
+            configurable: true,
+        });
+
+        result.current.onKeyDown({
+            ...mockEvent,
+            key,
+        } as KeyboardEvent<HTMLUListElement>);
+
+        expect(mockItems[finalPosition].focus).toHaveBeenCalledTimes(1);
+        expect(mockEvent.preventDefault).toHaveBeenCalledTimes(1);
+    });
+
+    it.each([["ArrowDown"], ["ArrowUp"]])("should not handle %s when active element is not in list", (key) => {
+        const result = render();
+        mockList.contains = jest.fn().mockReturnValue(false);
+
+        const outsideElement = document.createElement("button");
+
+        Object.defineProperty(document, "activeElement", {
+            value: outsideElement,
+            configurable: true,
+        });
+
+        result.current.onKeyDown({
+            ...mockEvent,
+            key,
+        } as KeyboardEvent<HTMLUListElement>);
+
+        // No item should be focused
+        mockItems.forEach((item) => expect(item.focus).not.toHaveBeenCalled());
+        expect(mockEvent.preventDefault).toHaveBeenCalledTimes(1);
+    });
+
+    it("should not prevent default for unhandled keys", () => {
+        const result = render();
+
+        result.current.onKeyDown({
+            ...mockEvent,
+            key: "Tab",
+        } as KeyboardEvent<HTMLUListElement>);
+
+        expect(mockEvent.preventDefault).not.toHaveBeenCalled();
+    });
+});

src/shared-components/hooks/useListKeyDown.ts

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import { useCallback, useRef, type RefObject, type KeyboardEvent, type KeyboardEventHandler } from "react";
+
+/**
+ * A hook that provides keyboard navigation for a list of options.
+ */
+export function useListKeyDown(): {
+    listRef: RefObject<HTMLUListElement | null>;
+    onKeyDown: KeyboardEventHandler<HTMLUListElement>;
+} {
+    const listRef = useRef<HTMLUListElement>(null);
+
+    const onKeyDown = useCallback((evt: KeyboardEvent<HTMLUListElement>) => {
+        const { key } = evt;
+
+        let handled = false;
+
+        switch (key) {
+            case "Enter":
+            case " ": {
+                handled = true;
+                (document.activeElement as HTMLElement).click();
+                break;
+            }
+            case "ArrowDown": {
+                handled = true;
+                const currentFocus = document.activeElement;
+                if (listRef.current?.contains(currentFocus) && currentFocus) {
+                    (currentFocus.nextElementSibling as HTMLElement)?.focus();
+                }
+                break;
+            }
+            case "ArrowUp": {
+                handled = true;
+                const currentFocus = document.activeElement;
+                if (listRef.current?.contains(currentFocus) && currentFocus) {
+                    (currentFocus.previousElementSibling as HTMLElement)?.focus();
+                }
+                break;
+            }
+            case "Home": {
+                handled = true;
+                (listRef.current?.firstElementChild as HTMLElement)?.focus();
+                break;
+            }
+            case "End": {
+                handled = true;
+                (listRef.current?.lastElementChild as HTMLElement)?.focus();
+                break;
+            }
+        }
+
+        if (handled) {
+            evt.preventDefault();
+        }
+    }, []);
+    return { listRef, onKeyDown };
+}

src/shared-components/rich-list/RichItem/RichItem.module.css

@@ -6,11 +6,14 @@
  */
 
 .richItem {
-    all: unset;
+    /* Remove browser button style */
+    background: transparent;
+    border: none;
     padding: var(--cpd-space-2x) var(--cpd-space-4x) var(--cpd-space-2x) var(--cpd-space-4x);
     width: 100%;
     box-sizing: border-box;
     cursor: pointer;
+    text-align: start;
 
     display: grid;
     column-gap: var(--cpd-space-3x);
@@ -20,7 +23,8 @@
         "avatar description time";
 }
 
-.richItem:hover {
+.richItem:hover,
+.richItem:focus {
     background-color: var(--cpd-color-bg-subtle-secondary);
     border-radius: 12px;
 }

src/shared-components/rich-list/RichItem/RichItem.tsx

@@ -12,7 +12,7 @@ import styles from "./RichItem.module.css";
 import { humanizeTime } from "../../utils/humanize";
 import { Flex } from "../../utils/Flex";
 
-export interface RichItemProps extends HTMLAttributes<HTMLButtonElement> {
+export interface RichItemProps extends HTMLAttributes<HTMLLIElement> {
     /**
      * Avatar to display at the start of the item
      */
@@ -64,10 +64,10 @@ export const RichItem = memo(function RichItem({
     ...props
 }: RichItemProps): JSX.Element {
     return (
-        <button
+        <li
             className={styles.richItem}
-            type="button"
             role="option"
+            tabIndex={0}
             aria-selected={selected}
             aria-label={title}
             {...props}
@@ -80,7 +80,7 @@ export const RichItem = memo(function RichItem({
                     {humanizeTime(timestamp)}
                 </span>
             )}
-        </button>
+        </li>
     );
 });
 

src/shared-components/rich-list/RichItem/__snapshots__/RichItem.test.tsx.snap

@@ -6,11 +6,11 @@ exports[`RichItem renders the item in default state 1`] = `
     role="listbox"
     style="all: unset; list-style: none;"
   >
-    <button
+    <li
       aria-label="Rich Item Title"
       class="richItem"
       role="option"
-      type="button"
+      tabindex="0"
     >
       <div
         class="flex avatar"
@@ -36,7 +36,7 @@ exports[`RichItem renders the item in default state 1`] = `
       >
         145 days ago
       </span>
-    </button>
+    </li>
   </ul>
 </div>
 `;
@@ -47,12 +47,12 @@ exports[`RichItem renders the item in selected state 1`] = `
     role="listbox"
     style="all: unset; list-style: none;"
   >
-    <button
+    <li
       aria-label="Rich Item Title"
       aria-selected="true"
       class="richItem"
       role="option"
-      type="button"
+      tabindex="0"
     >
       <div
         aria-hidden="true"
@@ -88,7 +88,7 @@ exports[`RichItem renders the item in selected state 1`] = `
       >
         145 days ago
       </span>
-    </button>
+    </li>
   </ul>
 </div>
 `;
@@ -99,11 +99,11 @@ exports[`RichItem renders the item without timestamp 1`] = `
     role="listbox"
     style="all: unset; list-style: none;"
   >
-    <button
+    <li
       aria-label="Rich Item Title"
       class="richItem"
       role="option"
-      type="button"
+      tabindex="0"
     >
       <div
         class="flex avatar"
@@ -123,7 +123,7 @@ exports[`RichItem renders the item without timestamp 1`] = `
       >
         This is a description of the rich item.
       </span>
-    </button>
+    </li>
   </ul>
 </div>
 `;

src/shared-components/rich-list/RichList/RichList.tsx

@@ -5,11 +5,12 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
-import React, { type HTMLProps, type JSX, type PropsWithChildren } from "react";
+import React, { type HTMLProps, type JSX, type PropsWithChildren, useId } from "react";
 import classNames from "classnames";
 
 import styles from "./RichList.module.css";
 import { Flex } from "../../utils/Flex";
+import { useListKeyDown } from "../../hooks/useListKeyDown";
 
 export interface RichListProps extends HTMLProps<HTMLDivElement> {
     /**
@@ -51,15 +52,25 @@ export function RichList({
     isEmpty = false,
     ...props
 }: PropsWithChildren<RichListProps>): JSX.Element {
+    const id = useId();
+    const { listRef, onKeyDown } = useListKeyDown();
+
     return (
         <Flex className={classNames(styles.richList, className)} direction="column" {...props}>
-            <span className={styles.title} {...titleAttributes}>
+            <span id={id} className={styles.title} {...titleAttributes}>
                 {title}
             </span>
             {isEmpty ? (
                 <span className={styles.empty}>{children}</span>
             ) : (
-                <ul role="listbox" className={styles.content} aria-label={title}>
+                <ul
+                    ref={listRef}
+                    role="listbox"
+                    className={styles.content}
+                    aria-labelledby={id}
+                    tabIndex={0}
+                    onKeyDown={onKeyDown}
+                >
                     {children}
                 </ul>
             )}