diff --git a/res/css/views/rooms/_MemberTileView.pcss b/res/css/views/rooms/_MemberTileView.pcss index 1855c50593..307625d042 100644 --- a/res/css/views/rooms/_MemberTileView.pcss +++ b/res/css/views/rooms/_MemberTileView.pcss @@ -42,7 +42,3 @@ Please see LICENSE files in the repository root for full details. width: 32px; } } - -.mx_MemberTileView_hover { - background-color: var(--cpd-color-bg-action-secondary-hovered); -} diff --git a/src/components/utils/ListView.tsx b/src/components/utils/ListView.tsx index bea53eb594..3727e8f119 100644 --- a/src/components/utils/ListView.tsx +++ b/src/components/utils/ListView.tsx @@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import React, { useRef, type JSX, useCallback } from "react"; +import React, { useRef, type JSX, useCallback, useEffect, useState } from "react"; import { type VirtuosoHandle, type ListRange, Virtuoso, type VirtuosoProps } from "react-virtuoso"; /** @@ -13,8 +13,10 @@ import { type VirtuosoHandle, type ListRange, Virtuoso, type VirtuosoProps } fro * and any additional context data from the parent component. */ export type ListContext = { - /** The key of the currently focused item in the list (undefined if no item is focused) */ - focusKey?: string; + /** The key of item that should have tabIndex == 0 */ + tabIndexKey?: string; + /** Whether an item in the list is currently focused */ + focused: boolean; /** Additional context data passed from the parent component */ context: Context; }; @@ -38,10 +40,9 @@ export interface IListViewProps * @param index - The index of the item in the list * @param item - The data item to render * @param context - The context object containing the focused key and any additional data - * @param onBlur - Callback to call when the item loses focus * @returns JSX element representing the rendered item */ - getItemComponent: (index: number, item: Item, context: ListContext, onBlur: () => void) => JSX.Element; + getItemComponent: (index: number, item: Item, context: ListContext) => JSX.Element; /** * Optional additional context data to pass to each rendered item. @@ -72,27 +73,34 @@ export interface IListViewProps * @template Context - The type of additional context data passed to items */ export function ListView(props: IListViewProps): React.ReactElement { + // Extract our custom props to avoid conflicts with Virtuoso props + const { items, onSelectItem, getItemComponent, isItemFocusable, getItemKey, context, ...virtuosoProps } = props; /** Reference to the Virtuoso component for programmatic scrolling */ const virtuosoHandleRef = useRef(null); /** Reference to the DOM element containing the virtualized list */ const virtuosoDomRef = useRef(null); - - /** Key of the currently focused item (unknown if no item is focused) */ - const [focusKey, setfocusKey] = React.useState(undefined); - /** Key of the last focused item (used when regaining focus) */ - const [lastFocusKey, setLastFocusKey] = React.useState(undefined); + /** Key of the item that should have tabIndex == 0 */ + const [tabIndexKey, setTabIndexKey] = useState( + props.items[0] ? getItemKey(props.items[0]) : undefined, + ); /** Range of currently visible items in the viewport */ - const [visibleRange, setVisibleRange] = React.useState(undefined); + const [visibleRange, setVisibleRange] = useState(undefined); /** Map from item keys to their indices in the items array */ - const [keyToIndexMap, setKeyToIndexMap] = React.useState>(new Map()); - - // Extract our custom props to avoid conflicts with Virtuoso props - const { items, onSelectItem, getItemComponent, isItemFocusable, getItemKey, context, ...virtuosoProps } = props; - + const [keyToIndexMap, setKeyToIndexMap] = useState>(new Map()); + /** Whether the list is currently scrolling to an item */ const isScrollingToItem = useRef(false); + /** Whether the list is currently focused */ + const [isFocused, setIsFocused] = useState(false); // Update the key-to-index mapping whenever items change - React.useEffect(() => { + useEffect(() => { + if (items.length && !tabIndexKey) { + setTabIndexKey(getItemKey(items[0])); + } + }, [items, getItemKey, tabIndexKey]); + + // Update the key-to-index mapping whenever items change + useEffect(() => { const newKeyToIndexMap = new Map(); items.forEach((item, index) => { const key = getItemKey(item); @@ -101,24 +109,6 @@ export function ListView(props: IListViewProps): JSX.Element => { - const onBlur = (): void => { - const key = getItemKey(item); - if (focusKey === key) { - setfocusKey(undefined); - setLastFocusKey(key); - } - }; - return getItemComponent(index, item, context, onBlur); - }, - [focusKey, getItemKey, getItemComponent], - ); - /** * Scrolls to a specific item index and sets it as focused. * Uses Virtuoso's scrollIntoView method for smooth scrolling. @@ -134,7 +124,7 @@ export function ListView(props: IListViewProps(props: IListViewProps { if (!e) return; // Guard against null/undefined events - const currentIndex = focusKey ? keyToIndexMap.get(focusKey) : undefined; + const currentIndex = tabIndexKey ? keyToIndexMap.get(tabIndexKey) : undefined; let handled = false; if (e.code === "ArrowUp" && currentIndex !== undefined) { @@ -216,49 +206,57 @@ export function ListView(props: IListViewProps { + const scrollerRef = useCallback((element: HTMLElement | Window | null) => { virtuosoDomRef.current = element; }, []); /** * Handles focus events on the list. - * Sets initial focus to the last focused item or the first item if none was previously focused. + * Sets the focused state and scrolls to the focused item if it is not currently visible. */ - const onFocus = (e?: React.FocusEvent): void => { - if (e?.currentTarget !== virtuosoDomRef.current || focusKey !== undefined) { - return; - } - - let nextIndex = 0; - if (lastFocusKey) { - const lastIndex = keyToIndexMap.get(lastFocusKey); - if (lastIndex !== undefined) { - nextIndex = lastIndex; + const onFocus = useCallback( + (e?: React.FocusEvent): void => { + if (e?.currentTarget !== virtuosoDomRef.current || typeof tabIndexKey !== "string") { + return; } - } - scrollToIndex(nextIndex); - e?.stopPropagation(); - e?.preventDefault(); - }; + setIsFocused(true); + const index = keyToIndexMap.get(tabIndexKey); + if ( + index !== undefined && + visibleRange && + (index < visibleRange.startIndex || index > visibleRange.endIndex) + ) { + scrollToIndex(index); + } + e?.stopPropagation(); + e?.preventDefault(); + }, + [keyToIndexMap, visibleRange, scrollToIndex, tabIndexKey], + ); + + const onBlur = useCallback((): void => { + setIsFocused(false); + }, []); const listContext: ListContext = { - focusKey: focusKey, + tabIndexKey: tabIndexKey, + focused: isFocused, context: props.context || ({} as Context), }; return ( (props: IListViewProps ); diff --git a/src/components/views/rooms/MemberList/MemberListView.tsx b/src/components/views/rooms/MemberList/MemberListView.tsx index 9c12414e21..8afdeaf990 100644 --- a/src/components/views/rooms/MemberList/MemberListView.tsx +++ b/src/components/views/rooms/MemberList/MemberListView.tsx @@ -41,9 +41,10 @@ const MemberListView: React.FC = (props: IProps) => { }, []); const getItemComponent = useCallback( - (index: number, item: MemberWithSeparator, context: ListContext, onBlur: () => void): JSX.Element => { + (index: number, item: MemberWithSeparator, context: ListContext): JSX.Element => { const itemKey = getItemKey(item); - const focused = itemKey === context.focusKey; + const isRovingItem = itemKey === context.tabIndexKey; + const focused = isRovingItem && context.focused; if (item === SEPARATOR) { return
; } else if (item.member) { @@ -52,9 +53,9 @@ const MemberListView: React.FC = (props: IProps) => { member={item.member} showPresence={isPresenceEnabled} focused={focused} + tabIndex={isRovingItem ? 0 : -1} index={index} memberCount={memberCount} - onBlur={onBlur} /> ); } else { @@ -62,9 +63,9 @@ const MemberListView: React.FC = (props: IProps) => { ); } diff --git a/src/components/views/rooms/MemberList/tiles/RoomMemberTileView.tsx b/src/components/views/rooms/MemberList/tiles/RoomMemberTileView.tsx index 47b4bf074d..4837972da3 100644 --- a/src/components/views/rooms/MemberList/tiles/RoomMemberTileView.tsx +++ b/src/components/views/rooms/MemberList/tiles/RoomMemberTileView.tsx @@ -21,9 +21,9 @@ interface IProps { member: RoomMember; index: number; memberCount: number; - onBlur?: () => void; showPresence?: boolean; focused?: boolean; + tabIndex?: number; } export function RoomMemberTileView(props: IProps): JSX.Element { @@ -59,7 +59,6 @@ export function RoomMemberTileView(props: IProps): JSX.Element { return ( diff --git a/src/components/views/rooms/MemberList/tiles/ThreePidInviteTileView.tsx b/src/components/views/rooms/MemberList/tiles/ThreePidInviteTileView.tsx index 57d06baf67..0a93727f5f 100644 --- a/src/components/views/rooms/MemberList/tiles/ThreePidInviteTileView.tsx +++ b/src/components/views/rooms/MemberList/tiles/ThreePidInviteTileView.tsx @@ -18,7 +18,7 @@ interface Props { memberIndex: number; memberCount: number; focused?: boolean; - onBlur?: () => void; + tabIndex?: number; } export function ThreePidInviteTileView(props: Props): JSX.Element { @@ -32,13 +32,13 @@ export function ThreePidInviteTileView(props: Props): JSX.Element { nameJsx={name} avatarJsx={av} onClick={vm.onClick} - onBlur={props.onBlur} memberIndex={props.memberIndex} memberCount={props.memberCount} ariaLabel={name} userLabel={vm.userLabel} iconJsx={iconJsx} focused={props.focused} + tabIndex={props.tabIndex} /> ); } diff --git a/src/components/views/rooms/MemberList/tiles/common/MemberTileView.tsx b/src/components/views/rooms/MemberList/tiles/common/MemberTileView.tsx index 276163cb28..7254208b2a 100644 --- a/src/components/views/rooms/MemberList/tiles/common/MemberTileView.tsx +++ b/src/components/views/rooms/MemberList/tiles/common/MemberTileView.tsx @@ -5,7 +5,6 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import classNames from "classnames"; import React, { useEffect, useRef, type JSX } from "react"; import AccessibleButton from "../../../../elements/AccessibleButton"; @@ -16,11 +15,11 @@ interface Props { onClick: () => void; memberIndex: number; memberCount: number; - onBlur?: () => void; ariaLabel?: string; presenceJsx?: JSX.Element; userLabel?: React.ReactNode; iconJsx?: JSX.Element; + tabIndex?: number; focused?: boolean; } @@ -40,13 +39,10 @@ export function MemberTileView(props: Props): JSX.Element {
{ getItemKey: (item) => (typeof item === "string" ? item : item.id), }; + const getListViewComponent = (props: Partial> = {}) => { + const mergedProps = { ...defaultProps, ...props }; + return ; + }; + const renderListViewWithHeight = (props: Partial> = {}) => { const mergedProps = { ...defaultProps, ...props }; - return render( -
- -
, - { - wrapper: ({ children }) => ( - - {children} - - ), - }, - ); + return render(getListViewComponent(mergedProps), { + wrapper: ({ children }) => ( + + {children} + + ), + }); }; beforeEach(() => { jest.clearAllMocks(); - mockGetItemComponent.mockImplementation( - (index: number, item: TestItemWithSeparator, context: any, onBlur: () => void) => { - const itemKey = typeof item === "string" ? item : item.id; - const isFocused = context.focusKey === itemKey; - return ( -
- {item === SEPARATOR_ITEM ? "---" : (item as TestItem).name} - -
- ); - }, - ); + mockGetItemComponent.mockImplementation((index: number, item: TestItemWithSeparator, context: any) => { + const itemKey = typeof item === "string" ? item : item.id; + const isFocused = context.tabIndexKey === itemKey; + return ( +
+ {item === SEPARATOR_ITEM ? "---" : (item as TestItem).name} +
+ ); + }); mockIsItemFocusable.mockImplementation((item: TestItemWithSeparator) => item !== SEPARATOR_ITEM); }); @@ -79,12 +76,12 @@ describe("ListView", () => { describe("Rendering", () => { it("should render the ListView component", () => { - render(); + renderListViewWithHeight(); expect(screen.getByRole("grid")).toBeInTheDocument(); }); it("should render with empty items array", () => { - render(); + renderListViewWithHeight({ items: [] }); expect(screen.getByRole("grid")).toBeInTheDocument(); }); }); @@ -325,46 +322,6 @@ describe("ListView", () => { expect(items[2]).toHaveAttribute("tabindex", "0"); // Should still be item 2 }); - it("should handle onBlur callback from item components", () => { - const blurSpy = jest.fn(); - mockGetItemComponent.mockImplementation( - (index: number, item: TestItemWithSeparator, context: any, onBlur: () => void) => { - const itemKey = typeof item === "string" ? item : item.id; - const isFocused = context.focusKey === itemKey; - return ( -
{ - onBlur(); - blurSpy(); - }} - > - {item === SEPARATOR_ITEM ? "---" : (item as TestItem).name} - -
- ); - }, - ); - - renderListViewWithHeight(); - const container = screen.getByRole("grid"); - - fireEvent.focus(container); - - // Verify focus is established - const items = container.querySelectorAll(".mx_item"); - expect(items[0]).toHaveAttribute("tabindex", "0"); - - // Simulate blur on the focused item - const firstItem = container.querySelector("[data-testid='row-0']"); - fireEvent.blur(firstItem!); - - // Verify the blur callback was called - expect(blurSpy).toHaveBeenCalled(); - }); - it("should not interfere with focus if item is already focused", () => { renderListViewWithHeight(); const container = screen.getByRole("grid"); @@ -400,9 +357,10 @@ describe("ListView", () => { { id: "2", name: "Item 2" }, ]; rerender( -
- -
, + getListViewComponent({ + ...defaultProps, + items: fewerItems, + }), ); container = screen.getByRole("grid");