mirror of
https://github.com/vector-im/element-web.git
synced 2026-05-07 21:26:13 +02:00
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
This commit is contained in:
David Langley
parent
876bb7dff9
commit
654aade19a
@ -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
|
||||
|
||||
@ -29,12 +29,6 @@ export interface IListViewProps<Item, Context>
|
||||
*/
|
||||
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<Item, Context>
|
||||
*/
|
||||
export function ListView<Item, Context = any>(props: IListViewProps<Item, Context>): 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<VirtuosoHandle>(null);
|
||||
/** Reference to the DOM element containing the virtualized list */
|
||||
@ -181,10 +175,6 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
|
||||
} else if (e.code === "ArrowDown" && currentIndex !== undefined) {
|
||||
scrollToItem(currentIndex + 1, true);
|
||||
handled = true;
|
||||
} else if ((e.code === "Enter" || e.code === "Space") && currentIndex !== undefined) {
|
||||
const item = items[currentIndex];
|
||||
onSelectItem(item);
|
||||
handled = true;
|
||||
} else if (e.code === "Home") {
|
||||
scrollToIndex(0);
|
||||
handled = true;
|
||||
@ -206,7 +196,7 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
|
||||
e.preventDefault();
|
||||
}
|
||||
},
|
||||
[scrollToIndex, scrollToItem, tabIndexKey, keyToIndexMap, visibleRange, items, onSelectItem],
|
||||
[scrollToIndex, scrollToItem, tabIndexKey, keyToIndexMap, visibleRange, items],
|
||||
);
|
||||
|
||||
/**
|
||||
@ -242,8 +232,12 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
|
||||
[keyToIndexMap, visibleRange, scrollToIndex, tabIndexKey],
|
||||
);
|
||||
|
||||
const onBlur = useCallback((): void => {
|
||||
setIsFocused(false);
|
||||
const onBlur = useCallback((event: React.FocusEvent<HTMLDivElement>): 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<Context> = {
|
||||
|
||||
@ -105,7 +105,6 @@ const MemberListView: React.FC<IProps> = (props: IProps) => {
|
||||
</Form.Root>
|
||||
<ListView
|
||||
items={vm.members}
|
||||
onSelectItem={handleSelectItem}
|
||||
getItemComponent={getItemComponent}
|
||||
getItemKey={getItemKey}
|
||||
isItemFocusable={isItemFocusable}
|
||||
|
||||
@ -6,15 +6,12 @@
|
||||
*/
|
||||
|
||||
import React, { useCallback, type JSX } from "react";
|
||||
import { AutoSizer, List, type ListRowProps } from "react-virtualized";
|
||||
|
||||
import { type RoomListViewState } from "../../../viewmodels/roomlist/RoomListViewModel";
|
||||
import { _t } from "../../../../languageHandler";
|
||||
import { RoomListItemView } from "./RoomListItemView";
|
||||
import { RovingTabIndexProvider } from "../../../../accessibility/RovingTabIndex";
|
||||
import { getKeyBindingsManager } from "../../../../KeyBindingsManager";
|
||||
import { KeyBindingAction } from "../../../../accessibility/KeyboardShortcuts";
|
||||
import { Landmark, LandmarkNavigation } from "../../../../accessibility/LandmarkNavigation";
|
||||
import { ListContext, ListView } from "../../../utils/ListView";
|
||||
import { Room } from "matrix-js-sdk/src/matrix";
|
||||
|
||||
interface RoomListProps {
|
||||
/**
|
||||
@ -27,54 +24,40 @@ interface RoomListProps {
|
||||
* A virtualized list of rooms.
|
||||
*/
|
||||
export function RoomList({ vm: { rooms, activeIndex } }: RoomListProps): JSX.Element {
|
||||
const roomRendererMemoized = useCallback(
|
||||
({ key, index, style }: ListRowProps) => (
|
||||
<RoomListItemView room={rooms[index]} key={key} style={style} isSelected={activeIndex === index} />
|
||||
),
|
||||
const getItemComponent = useCallback(
|
||||
(index: number, item: Room, context: ListContext<any>): JSX.Element => {
|
||||
const itemKey = item.roomId;
|
||||
const isRovingItem = itemKey === context.tabIndexKey;
|
||||
const isFocused = isRovingItem && context.focused;
|
||||
const isSelected = activeIndex === index;
|
||||
return (
|
||||
<RoomListItemView
|
||||
room={item}
|
||||
key={itemKey}
|
||||
isSelected={isSelected}
|
||||
isFocused={isFocused}
|
||||
tabIndex={isRovingItem ? 0 : -1}
|
||||
/>
|
||||
);
|
||||
},
|
||||
[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 (
|
||||
<RovingTabIndexProvider handleHomeEnd={true} handleUpDown={true}>
|
||||
{({ onKeyDownHandler }) => (
|
||||
<div
|
||||
className="mx_RoomList"
|
||||
data-testid="room-list"
|
||||
onKeyDown={(ev) => {
|
||||
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);
|
||||
}}
|
||||
>
|
||||
<AutoSizer>
|
||||
{({ height, width }) => (
|
||||
<List
|
||||
aria-label={_t("room_list|list_title")}
|
||||
className="mx_RoomList_List"
|
||||
rowRenderer={roomRendererMemoized}
|
||||
rowCount={rooms.length}
|
||||
rowHeight={48}
|
||||
height={height}
|
||||
width={width}
|
||||
scrollToIndex={activeIndex ?? 0}
|
||||
tabIndex={-1}
|
||||
/>
|
||||
)}
|
||||
</AutoSizer>
|
||||
</div>
|
||||
)}
|
||||
</RovingTabIndexProvider>
|
||||
<ListView
|
||||
data-testid="room-list"
|
||||
role="grid"
|
||||
aria-label={_t("room_list|list_title")}
|
||||
fixedItemHeight={48}
|
||||
items={rooms}
|
||||
getItemComponent={getItemComponent}
|
||||
getItemKey={getItemKey}
|
||||
isItemFocusable={() => true}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@ -46,7 +46,13 @@ export function RoomListItemMenuView({ room, setMenuOpen }: RoomListItemMenuView
|
||||
const vm = useRoomListItemMenuViewModel(room);
|
||||
|
||||
return (
|
||||
<Flex className="mx_RoomListItemMenuView" align="center" gap="var(--cpd-space-1x)">
|
||||
<Flex
|
||||
className="mx_RoomListItemMenuView"
|
||||
align="center"
|
||||
gap="var(--cpd-space-1x)"
|
||||
// We don't want keyboard navigation event to bubble up to the ListView changing the focused item
|
||||
onKeyDown={(e) => e.stopPropagation()}
|
||||
>
|
||||
{vm.showMoreOptionsMenu && <MoreOptionsMenu setMenuOpen={setMenuOpen} vm={vm} />}
|
||||
{vm.showNotificationMenu && <NotificationMenu setMenuOpen={setMenuOpen} vm={vm} />}
|
||||
</Flex>
|
||||
|
||||
@ -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<HTMLButtonElement> {
|
||||
@ -26,6 +25,10 @@ interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement>
|
||||
* Whether the room is selected
|
||||
*/
|
||||
isSelected: boolean;
|
||||
/**
|
||||
* Whether the room is focused
|
||||
*/
|
||||
isFocused: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -34,18 +37,16 @@ interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement>
|
||||
export const RoomListItemView = memo(function RoomListItemView({
|
||||
room,
|
||||
isSelected,
|
||||
isFocused,
|
||||
...props
|
||||
}: RoomListItemViewProps): JSX.Element {
|
||||
const buttonRef = useRef<HTMLButtonElement>(null);
|
||||
const [onFocus, isActive, ref] = useRovingTabIndex(buttonRef);
|
||||
|
||||
const ref = useRef<HTMLButtonElement>(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 = (
|
||||
<button
|
||||
<Flex
|
||||
as="button"
|
||||
ref={ref}
|
||||
className={classNames("mx_RoomListItemView", {
|
||||
mx_RoomListItemView_hover: showHoverDecoration,
|
||||
@ -63,63 +71,55 @@ export const RoomListItemView = memo(function RoomListItemView({
|
||||
mx_RoomListItemView_selected: isSelected,
|
||||
mx_RoomListItemView_bold: vm.isBold,
|
||||
})}
|
||||
gap="var(--cpd-space-3x)"
|
||||
align="center"
|
||||
type="button"
|
||||
role="gridcell"
|
||||
aria-selected={isSelected}
|
||||
aria-label={vm.a11yLabel}
|
||||
onClick={() => vm.openRoom()}
|
||||
onMouseOver={() => setIsHoverWithDelay(true)}
|
||||
onMouseOut={() => setIsHoverWithDelay(false)}
|
||||
onFocus={() => {
|
||||
setIsHoverWithDelay(true);
|
||||
onFocus();
|
||||
}}
|
||||
// Adding a timeout because when tabbing to go to the more options and notification menu, the focus moves out of the button
|
||||
// The blur makes the button lose the hover state and these menu are not shown
|
||||
// We delay the blur event to give time to the focus to move to the menu
|
||||
onBlur={() => setIsHoverWithDelay(false, 10)}
|
||||
tabIndex={isActive ? 0 : -1}
|
||||
onMouseOver={() => setHover(true)}
|
||||
onMouseOut={() => setHover(false)}
|
||||
tabIndex={isFocused ? 0 : -1}
|
||||
{...props}
|
||||
>
|
||||
{/* We need this extra div between the button and the content in order to add a padding which is not messing with the virtualized list */}
|
||||
<Flex className="mx_RoomListItemView_container" gap="var(--cpd-space-3x)" align="center">
|
||||
<RoomAvatarView room={room} />
|
||||
<Flex
|
||||
className="mx_RoomListItemView_content"
|
||||
gap="var(--cpd-space-2x)"
|
||||
align="center"
|
||||
justify="space-between"
|
||||
>
|
||||
{/* We truncate the room name when too long. Title here is to show the full name on hover */}
|
||||
<div className="mx_RoomListItemView_text">
|
||||
<div className="mx_RoomListItemView_roomName" title={vm.name}>
|
||||
{vm.name}
|
||||
</div>
|
||||
{vm.messagePreview && (
|
||||
<div className="mx_RoomListItemView_messagePreview" title={vm.messagePreview}>
|
||||
{vm.messagePreview}
|
||||
</div>
|
||||
)}
|
||||
<RoomAvatarView room={room} />
|
||||
<Flex
|
||||
className="mx_RoomListItemView_content"
|
||||
gap="var(--cpd-space-2x)"
|
||||
align="center"
|
||||
justify="space-between"
|
||||
>
|
||||
{/* We truncate the room name when too long. Title here is to show the full name on hover */}
|
||||
<div className="mx_RoomListItemView_text">
|
||||
<div className="mx_RoomListItemView_roomName" title={vm.name}>
|
||||
{vm.name}
|
||||
</div>
|
||||
{showHoverMenu ? (
|
||||
<RoomListItemMenuView
|
||||
room={room}
|
||||
setMenuOpen={(isOpen) => (isOpen ? setIsMenuOpen(true) : closeMenu())}
|
||||
/>
|
||||
) : (
|
||||
<>
|
||||
{/* aria-hidden because we summarise the unread count/notification status in a11yLabel variable */}
|
||||
{vm.showNotificationDecoration && (
|
||||
<NotificationDecoration
|
||||
notificationState={vm.notificationState}
|
||||
aria-hidden={true}
|
||||
hasVideoCall={vm.hasParticipantInCall}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
{vm.messagePreview && (
|
||||
<div className="mx_RoomListItemView_messagePreview" title={vm.messagePreview}>
|
||||
{vm.messagePreview}
|
||||
</div>
|
||||
)}
|
||||
</Flex>
|
||||
</div>
|
||||
{showHoverMenu ? (
|
||||
<RoomListItemMenuView
|
||||
room={room}
|
||||
setMenuOpen={(isOpen) => (isOpen ? setIsMenuOpen(true) : closeMenu())}
|
||||
/>
|
||||
) : (
|
||||
<>
|
||||
{/* aria-hidden because we summarise the unread count/notification status in a11yLabel variable */}
|
||||
{vm.showNotificationDecoration && (
|
||||
<NotificationDecoration
|
||||
notificationState={vm.notificationState}
|
||||
aria-hidden={true}
|
||||
hasVideoCall={vm.hasParticipantInCall}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</Flex>
|
||||
</button>
|
||||
</Flex>
|
||||
);
|
||||
|
||||
if (!vm.showContextMenu) return content;
|
||||
@ -140,33 +140,3 @@ export const RoomListItemView = memo(function RoomListItemView({
|
||||
</RoomListItemContextMenuView>
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* 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<number | undefined>(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];
|
||||
}
|
||||
|
||||
@ -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<TestItemWithSeparator, any> = {
|
||||
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");
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user