From 654aade19aba79015efa83550c33e7f82018dee9 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 31 Jul 2025 19:27:50 +0100 Subject: [PATCH] Move Room List to ListView - Also remove Space/Enter handing from keyboard navigation we can just leave the default behaviour of those keys and handle via onClick --- .../RoomListPanel/_RoomListItemView.pcss | 60 ++++---- src/components/utils/ListView.tsx | 22 +-- .../views/rooms/MemberList/MemberListView.tsx | 1 - .../views/rooms/RoomListPanel/RoomList.tsx | 81 ++++------ .../RoomListPanel/RoomListItemMenuView.tsx | 8 +- .../rooms/RoomListPanel/RoomListItemView.tsx | 144 +++++++----------- .../components/views/utils/ListView-test.tsx | 29 ---- 7 files changed, 136 insertions(+), 209 deletions(-) diff --git a/res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss b/res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss index ac58a69bef..06ffe532d7 100644 --- a/res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss +++ b/res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss @@ -15,40 +15,44 @@ * |-------------------------------------------------------| */ .mx_RoomListItemView { - all: unset; + /* Remove button default style */ + background: unset; + border: none; + padding: 0; + text-align: unset; + cursor: pointer; + height: 48px; + width: 100%; - .mx_RoomListItemView_container { - padding-left: var(--cpd-space-3x); - font: var(--cpd-font-body-md-regular); + padding-left: var(--cpd-space-3x); + font: var(--cpd-font-body-md-regular); + + .mx_RoomListItemView_content { height: 100%; + flex: 1; + /* The border is only under the room name and the future hover menu */ + border-bottom: var(--cpd-border-width-0-5) solid var(--cpd-color-bg-subtle-secondary); + box-sizing: border-box; + min-width: 0; + padding-right: var(--cpd-space-5x); - .mx_RoomListItemView_content { - height: 100%; - flex: 1; - /* The border is only under the room name and the future hover menu */ - border-bottom: var(--cpd-border-width-0-5) solid var(--cpd-color-bg-subtle-secondary); - box-sizing: border-box; + .mx_RoomListItemView_text { min-width: 0; - padding-right: var(--cpd-space-5x); + } - .mx_RoomListItemView_text { - min-width: 0; - } + .mx_RoomListItemView_roomName { + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } - .mx_RoomListItemView_roomName { - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - } - - .mx_RoomListItemView_messagePreview { - font: var(--cpd-font-body-sm-regular); - color: var(--cpd-color-text-secondary); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - } + .mx_RoomListItemView_messagePreview { + font: var(--cpd-font-body-sm-regular); + color: var(--cpd-color-text-secondary); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } } } @@ -57,7 +61,7 @@ background-color: var(--cpd-color-bg-action-secondary-hovered); } -.mx_RoomListItemView_menu_open .mx_RoomListItemView_container .mx_RoomListItemView_content { +.mx_RoomListItemView_menu_open .mx_RoomListItemView_content { /** * The figma uses 16px padding (--cpd-space-4x) but due to https://github.com/element-hq/compound-web/issues/331 * the icon size of the menu is 18px instead of 20px with a different internal padding diff --git a/src/components/utils/ListView.tsx b/src/components/utils/ListView.tsx index 377ffb141b..ee46327fd9 100644 --- a/src/components/utils/ListView.tsx +++ b/src/components/utils/ListView.tsx @@ -29,12 +29,6 @@ export interface IListViewProps */ items: Item[]; - /** - * Callback function called when an item is selected (via Enter/Space key). - * @param item - The selected item from the items array - */ - onSelectItem: (item: Item) => void; - /** * Function that renders each list item as a JSX element. * @param index - The index of the item in the list @@ -74,7 +68,7 @@ export interface IListViewProps */ 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; + const { items, 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 */ @@ -181,10 +175,6 @@ export function ListView(props: IListViewProps(props: IListViewProps(props: IListViewProps { - setIsFocused(false); + const onBlur = useCallback((event: React.FocusEvent): void => { + // Only set isFocused to false if the focus is moving outside the list + // This prevents the list from losing focus when interacting with menus inside it + if (!event.currentTarget.contains(event.relatedTarget)) { + setIsFocused(false); + } }, []); const listContext: ListContext = { diff --git a/src/components/views/rooms/MemberList/MemberListView.tsx b/src/components/views/rooms/MemberList/MemberListView.tsx index 8afdeaf990..656ca9e2f7 100644 --- a/src/components/views/rooms/MemberList/MemberListView.tsx +++ b/src/components/views/rooms/MemberList/MemberListView.tsx @@ -105,7 +105,6 @@ const MemberListView: React.FC = (props: IProps) => { ( - - ), + const getItemComponent = useCallback( + (index: number, item: Room, context: ListContext): JSX.Element => { + const itemKey = item.roomId; + const isRovingItem = itemKey === context.tabIndexKey; + const isFocused = isRovingItem && context.focused; + const isSelected = activeIndex === index; + return ( + + ); + }, [rooms, activeIndex], ); + const getItemKey = useCallback((item: Room): string => { + return item.roomId; + }, []); + // The first div is needed to make the virtualized list take all the remaining space and scroll correctly return ( - - {({ onKeyDownHandler }) => ( -
{ - const navAction = getKeyBindingsManager().getNavigationAction(ev); - if ( - navAction === KeyBindingAction.NextLandmark || - navAction === KeyBindingAction.PreviousLandmark - ) { - LandmarkNavigation.findAndFocusNextLandmark( - Landmark.ROOM_LIST, - navAction === KeyBindingAction.PreviousLandmark, - ); - ev.stopPropagation(); - ev.preventDefault(); - return; - } - onKeyDownHandler(ev); - }} - > - - {({ height, width }) => ( - - )} - -
- )} - + true} + /> ); } diff --git a/src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx b/src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx index 66107074b7..15d53ea010 100644 --- a/src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx +++ b/src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx @@ -46,7 +46,13 @@ export function RoomListItemMenuView({ room, setMenuOpen }: RoomListItemMenuView const vm = useRoomListItemMenuViewModel(room); return ( - + e.stopPropagation()} + > {vm.showMoreOptionsMenu && } {vm.showNotificationMenu && } diff --git a/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx b/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx index a124c33851..0bffb2ebb7 100644 --- a/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx +++ b/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import React, { type JSX, memo, useCallback, useRef, useState } from "react"; +import React, { type JSX, memo, useCallback, useEffect, useRef, useState } from "react"; import { type Room } from "matrix-js-sdk/src/matrix"; import classNames from "classnames"; @@ -14,7 +14,6 @@ import { Flex } from "../../../../shared-components/utils/Flex"; import { RoomListItemMenuView } from "./RoomListItemMenuView"; import { NotificationDecoration } from "../NotificationDecoration"; import { RoomAvatarView } from "../../avatars/RoomAvatarView"; -import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex"; import { RoomListItemContextMenuView } from "./RoomListItemContextMenuView"; interface RoomListItemViewProps extends React.HTMLAttributes { @@ -26,6 +25,10 @@ interface RoomListItemViewProps extends React.HTMLAttributes * Whether the room is selected */ isSelected: boolean; + /** + * Whether the room is focused + */ + isFocused: boolean; } /** @@ -34,18 +37,16 @@ interface RoomListItemViewProps extends React.HTMLAttributes export const RoomListItemView = memo(function RoomListItemView({ room, isSelected, + isFocused, ...props }: RoomListItemViewProps): JSX.Element { - const buttonRef = useRef(null); - const [onFocus, isActive, ref] = useRovingTabIndex(buttonRef); - + const ref = useRef(null); const vm = useRoomListItemViewModel(room); - - const [isHover, setIsHoverWithDelay] = useIsHover(); + const [isHover, setHover] = useState(false); const [isMenuOpen, setIsMenuOpen] = useState(false); // The compound menu in RoomListItemMenuView needs to be rendered when the hover menu is shown // Using display: none; and then display:flex when hovered in CSS causes the menu to be misaligned - const showHoverDecoration = isMenuOpen || isHover; + const showHoverDecoration = isMenuOpen || isFocused || isHover; const showHoverMenu = showHoverDecoration && vm.showHoverMenu; const closeMenu = useCallback(() => { @@ -54,8 +55,15 @@ export const RoomListItemView = memo(function RoomListItemView({ setTimeout(() => setIsMenuOpen(false), 10); }, []); + useEffect(() => { + if (isFocused) { + ref.current?.focus({ preventScroll: true, focusVisible: true }); + } + }, [isFocused]); + const content = ( - + ); if (!vm.showContextMenu) return content; @@ -140,33 +140,3 @@ export const RoomListItemView = memo(function RoomListItemView({ ); }); - -/** - * Custom hook to manage the hover state of the room list item - * If the timeout is set, it will set the hover state after the timeout - * If the timeout is not set, it will set the hover state immediately - * When the set method is called, it will clear any existing timeout - * - * @returns {boolean} isHover - The hover state - */ -function useIsHover(): [boolean, (value: boolean, timeout?: number) => void] { - const [isHover, setIsHover] = useState(false); - // Store the timeout ID - const timeoutRef = useRef(undefined); - - const setIsHoverWithDelay = useCallback((value: boolean, timeout?: number): void => { - // Clear the timeout if it exists - clearTimeout(timeoutRef.current); - - // No delay, set the value immediately - if (timeout === undefined) { - setIsHover(value); - return; - } - - // Set a timeout to set the value after the delay - timeoutRef.current = setTimeout(() => setIsHover(value), timeout); - }, []); - - return [isHover, setIsHoverWithDelay]; -} diff --git a/test/unit-tests/components/views/utils/ListView-test.tsx b/test/unit-tests/components/views/utils/ListView-test.tsx index 8964fd2655..ced86ed118 100644 --- a/test/unit-tests/components/views/utils/ListView-test.tsx +++ b/test/unit-tests/components/views/utils/ListView-test.tsx @@ -21,7 +21,6 @@ const SEPARATOR_ITEM = "SEPARATOR" as const; type TestItemWithSeparator = TestItem | typeof SEPARATOR_ITEM; describe("ListView", () => { - const mockOnSelectItem = jest.fn(); const mockGetItemComponent = jest.fn(); const mockIsItemFocusable = jest.fn(); @@ -34,7 +33,6 @@ describe("ListView", () => { const defaultProps: IListViewProps = { items: defaultItems, - onSelectItem: mockOnSelectItem, getItemComponent: mockGetItemComponent, isItemFocusable: mockIsItemFocusable, getItemKey: (item) => (typeof item === "string" ? item : item.id), @@ -87,33 +85,6 @@ describe("ListView", () => { }); describe("Keyboard Navigation", () => { - it("should handle Enter key and call onSelectItem when focused", async () => { - renderListViewWithHeight(); - const container = screen.getByRole("grid"); - - expect(container.querySelectorAll(".mx_item")).toHaveLength(4); - - // Focus to activate the list and navigate to first focusable item - fireEvent.focus(container); - - fireEvent.keyDown(container, { code: "Enter" }); - - expect(mockOnSelectItem).toHaveBeenCalledWith(defaultItems[0]); - }); - - it("should handle Space key and call onSelectItem when focused", () => { - renderListViewWithHeight(); - const container = screen.getByRole("grid"); - - expect(container.querySelectorAll(".mx_item")).toHaveLength(4); - // Focus to activate the list and navigate to first focusable item - fireEvent.focus(container); - - fireEvent.keyDown(container, { code: "Space" }); - - expect(mockOnSelectItem).toHaveBeenCalledWith(defaultItems[0]); - }); - it("should handle ArrowDown key navigation", () => { renderListViewWithHeight(); const container = screen.getByRole("grid");