From 0986200af94f425a9ba19e4d0948786525557b2e Mon Sep 17 00:00:00 2001 From: Rivka Ungar Date: Tue, 10 Dec 2024 13:38:26 +0200 Subject: [PATCH] fix(List): fix ListTitle getting focus on initialization (#2638) --- packages/core/src/components/List/List.tsx | 21 +++++++++++++++++-- .../__tests__/__snapshots__/List.test.js.snap | 2 +- .../src/components/List/utils/ListUtils.ts | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/core/src/components/List/List.tsx b/packages/core/src/components/List/List.tsx index 7c32fdacf5..1b69f867c2 100644 --- a/packages/core/src/components/List/List.tsx +++ b/packages/core/src/components/List/List.tsx @@ -5,6 +5,7 @@ import React, { forwardRef, ReactElement, useCallback, + useEffect, useMemo, useRef, useState @@ -26,6 +27,7 @@ import { getListItemIndexById, getNextListItemIndex, getPrevListItemIndex, + isListItem, useListId } from "./utils/ListUtils"; import styles from "./List.module.scss"; @@ -113,6 +115,20 @@ const List: VibeComponent & { ref: componentRef }); + useEffect(() => { + const selectedItemIndex = childrenRefs.current.findIndex( + child => isListItem(child) && child?.getAttribute("aria-selected") === "true" + ); + if (selectedItemIndex !== -1) { + updateFocusedItem(getListItemIdByIndex(childrenRefs, selectedItemIndex)); + } else { + const firstFocusableIndex = childrenRefs.current.findIndex(child => isListItem(child)); + if (firstFocusableIndex !== -1) { + updateFocusedItem(getListItemIdByIndex(childrenRefs, firstFocusableIndex)); + } + } + }, [updateFocusedItem]); + const overrideChildren = useMemo(() => { let override: ReactElement | ReactElement[] = Array.isArray(children) ? children : [children]; if (renderOnlyVisibleItems) { @@ -123,12 +139,13 @@ const List: VibeComponent & { if (!React.isValidElement(child)) { return child; } - const id = (child.props as { id: string }).id || `${overrideId}-item-${index}`; + const isFocusableItem = isListItem(childrenRefs.current[index]); + return React.cloneElement(child, { // @ts-ignore not sure how to deal with ref here ref: ref => (childrenRefs.current[index] = ref), - tabIndex: focusIndex === index ? 0 : -1, + tabIndex: focusIndex === index && isFocusableItem ? 0 : -1, id, component: getListItemComponentType(component) }); diff --git a/packages/core/src/components/List/__tests__/__snapshots__/List.test.js.snap b/packages/core/src/components/List/__tests__/__snapshots__/List.test.js.snap index a9205c8112..1b318243fd 100644 --- a/packages/core/src/components/List/__tests__/__snapshots__/List.test.js.snap +++ b/packages/core/src/components/List/__tests__/__snapshots__/List.test.js.snap @@ -29,7 +29,7 @@ exports[`List renders correctly with list items 1`] = ` onFocus={[Function]} onMouseEnter={[Function]} role="option" - tabIndex={0} + tabIndex={-1} > 1 diff --git a/packages/core/src/components/List/utils/ListUtils.ts b/packages/core/src/components/List/utils/ListUtils.ts index e81291396c..443fa0d3be 100644 --- a/packages/core/src/components/List/utils/ListUtils.ts +++ b/packages/core/src/components/List/utils/ListUtils.ts @@ -36,7 +36,7 @@ export const getListItemComponentType = (listComponent: ListElement): ListItemEl } }; -const isListItem = (element: HTMLElement) => { +export const isListItem = (element: HTMLElement) => { return element && element.getAttribute("role") === "option"; };