mirrors/element-web
1
0
Fork 0
mirror of https://github.com/vector-im/element-web.git synced 2026-05-08 21:56:19 +02:00
Code Issues Projects Releases Wiki Activity

Always set the roving tab index regardless of whether we are actually focused.

Browse Source 
Fixes the issue of not being able to shift+t
This commit is contained in:
David Langley 2025-07-30 13:37:55 +01:00
parent 4fecf1413c
commit 2db428f667
7 changed files with 96 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
res/css/views/rooms/_MemberTileView.pcss
View File

@ -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);
}

115
src/components/utils/ListView.tsx
View File

@ -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<Context> = {
/** 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<Item, Context>
* @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<Context>, onBlur: () => void) => JSX.Element;
getItemComponent: (index: number, item: Item, context: ListContext<Context>) => JSX.Element;
/**
* Optional additional context data to pass to each rendered item.
@ -72,27 +73,34 @@ export interface IListViewProps<Item, Context>
* @template Context - The type of additional context data passed to items
*/
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;
/** Reference to the Virtuoso component for programmatic scrolling */
const virtuosoHandleRef = useRef<VirtuosoHandle>(null);
/** Reference to the DOM element containing the virtualized list */
const virtuosoDomRef = useRef<HTMLElement | Window>(null);
/** Key of the currently focused item (unknown if no item is focused) */
const [focusKey, setfocusKey] = React.useState<string | undefined>(undefined);
/** Key of the last focused item (used when regaining focus) */
const [lastFocusKey, setLastFocusKey] = React.useState<string | undefined>(undefined);
/** Key of the item that should have tabIndex == 0 */
const [tabIndexKey, setTabIndexKey] = useState<string | undefined>(
props.items[0] ? getItemKey(props.items[0]) : undefined,
);
/** Range of currently visible items in the viewport */
const [visibleRange, setVisibleRange] = React.useState<ListRange | undefined>(undefined);
const [visibleRange, setVisibleRange] = useState<ListRange | undefined>(undefined);
/** Map from item keys to their indices in the items array */
const [keyToIndexMap, setKeyToIndexMap] = React.useState<Map<string, number>>(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<Map<string, number>>(new Map());
/** Whether the list is currently scrolling to an item */
const isScrollingToItem = useRef<boolean>(false);
/** Whether the list is currently focused */
const [isFocused, setIsFocused] = useState<boolean>(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<string, number>();
items.forEach((item, index) => {
const key = getItemKey(item);
@ -101,24 +109,6 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
setKeyToIndexMap(newKeyToIndexMap);
}, [items, getItemKey]);
/**
* Wrapper function that renders each list item and provides the onBlur callback.
* This function is called by Virtuoso for each visible item.
*/
const getItemComponentInternal = useCallback(
(index: number, item: Item, context: ListContext<Context>): 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<Item, Context = any>(props: IListViewProps<Item, Contex
}
if (items[clampedIndex]) {
const key = getItemKey(items[clampedIndex]);
setfocusKey(key);
setTabIndexKey(key);
isScrollingToItem.current = true;
virtuosoHandleRef?.current?.scrollIntoView({
index: clampedIndex,
@ -182,7 +172,7 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
(e: React.KeyboardEvent) => {
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<Item, Context = any>(props: IListViewProps<Item, Contex
e.preventDefault();
}
},
[scrollToIndex, scrollToItem, focusKey, keyToIndexMap, visibleRange, items, onSelectItem],
[scrollToIndex, scrollToItem, tabIndexKey, keyToIndexMap, visibleRange, items, onSelectItem],
);
/**
* Callback ref for the Virtuoso scroller element.
* Stores the reference for use in focus management.
*/
const scrollerRef = React.useCallback((element: HTMLElement | Window | null) => {
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<Context> = {
focusKey: focusKey,
tabIndexKey: tabIndexKey,
focused: isFocused,
context: props.context || ({} as Context),
};
return (
<Virtuoso
role={props.role || "grid"}
aria-rowcount={props.items.length}
aria-colcount={1}
tabIndex={props.tabIndex || undefined} // We don't need to focus the container, so leave it undefined by default
scrollerRef={scrollerRef}
ref={virtuosoHandleRef}
onKeyDown={keyDownCallback}
@ -268,7 +266,8 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
overscan={props.overscan || 0}
data={props.items}
onFocus={onFocus}
itemContent={getItemComponentInternal}
onBlur={onBlur}
itemContent={props.getItemComponent}
{...virtuosoProps}
/>
);

9
src/components/views/rooms/MemberList/MemberListView.tsx
View File

@ -41,9 +41,10 @@ const MemberListView: React.FC<IProps> = (props: IProps) => {
}, []);
const getItemComponent = useCallback(
(index: number, item: MemberWithSeparator, context: ListContext<any>, onBlur: () => void): JSX.Element => {
(index: number, item: MemberWithSeparator, context: ListContext<any>): 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 <hr className="mx_MemberListView_separator" />;
} else if (item.member) {
@ -52,9 +53,9 @@ const MemberListView: React.FC<IProps> = (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<IProps> = (props: IProps) => {
<ThreePidInviteTileView
threePidInvite={item.threePidInvite}
focused={focused}
tabIndex={isRovingItem ? 0 : -1}
memberIndex={index - 1} // Adjust as invites are below the separator
memberCount={memberCount}
onBlur={onBlur}
/>
);
}

4
src/components/views/rooms/MemberList/tiles/RoomMemberTileView.tsx
View File

@ -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 (
<MemberTileView
onClick={vm.onClick}
onBlur={props.onBlur}
avatarJsx={av}
presenceJsx={presenceJSX}
nameJsx={nameJSX}
@ -67,6 +66,7 @@ export function RoomMemberTileView(props: IProps): JSX.Element {
ariaLabel={name}
iconJsx={iconJsx}
focused={props.focused}
tabIndex={props.tabIndex}
memberIndex={props.index - (member.isInvite ? 1 : 0)} // Adjust as invites are below the seperator
memberCount={props.memberCount}
/>

4
src/components/views/rooms/MemberList/tiles/ThreePidInviteTileView.tsx
View File

@ -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}
/>
);
}

10
src/components/views/rooms/MemberList/tiles/common/MemberTileView.tsx
View File

@ -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 {
<div>
<AccessibleButton
ref={ref}
className={classNames("mx_MemberTileView", {
mx_MemberTileView_hover: props.focused,
})}
className="mx_MemberTileView"
onClick={props.onClick}
onBlur={props.onBlur}
aria-label={props?.ariaLabel}
tabIndex={props.focused ? 0 : -1}
tabIndex={props.tabIndex}
role="option"
aria-posinset={props.memberIndex + 1}
aria-setsize={props.memberCount}

96
test/unit-tests/components/views/utils/ListView-test.tsx
View File

@ -40,36 +40,33 @@ describe("ListView", () => {
getItemKey: (item) => (typeof item === "string" ? item : item.id),
};
const getListViewComponent = (props: Partial<IListViewProps<TestItemWithSeparator, any>> = {}) => {
const mergedProps = { ...defaultProps, ...props };
return <ListView {...mergedProps} role="grid" aria-rowcount={props.items?.length} aria-colcount={1} />;
};
const renderListViewWithHeight = (props: Partial<IListViewProps<TestItemWithSeparator, any>> = {}) => {
const mergedProps = { ...defaultProps, ...props };
return render(
<div style={{ height: "400px" }}>
<ListView {...mergedProps} style={{ height: "100%" }} />
</div>,
{
wrapper: ({ children }) => (
<VirtuosoMockContext.Provider value={{ viewportHeight: 400, itemHeight: 56 }}>
{children}
</VirtuosoMockContext.Provider>
),
},
);
return render(getListViewComponent(mergedProps), {
wrapper: ({ children }) => (
<VirtuosoMockContext.Provider value={{ viewportHeight: 400, itemHeight: 56 }}>
{children}
</VirtuosoMockContext.Provider>
),
});
};
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 (
<div className="mx_item" data-testid={`row-${index}`} tabIndex={isFocused ? 0 : -1}>
{item === SEPARATOR_ITEM ? "---" : (item as TestItem).name}
<button onClick={onBlur}>Blur</button>
</div>
);
},
);
mockGetItemComponent.mockImplementation((index: number, item: TestItemWithSeparator, context: any) => {
const itemKey = typeof item === "string" ? item : item.id;
const isFocused = context.tabIndexKey === itemKey;
return (
<div className="mx_item" data-testid={`row-${index}`} tabIndex={isFocused ? 0 : -1}>
{item === SEPARATOR_ITEM ? "---" : (item as TestItem).name}
</div>
);
});
mockIsItemFocusable.mockImplementation((item: TestItemWithSeparator) => item !== SEPARATOR_ITEM);
});
@ -79,12 +76,12 @@ describe("ListView", () => {
describe("Rendering", () => {
it("should render the ListView component", () => {
render(<ListView {...defaultProps} />);
renderListViewWithHeight();
expect(screen.getByRole("grid")).toBeInTheDocument();
});
it("should render with empty items array", () => {
render(<ListView {...defaultProps} items={[]} />);
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 (
<div
className="mx_item"
data-testid={`row-${index}`}
tabIndex={isFocused ? 0 : -1}
onBlur={() => {
onBlur();
blurSpy();
}}
>
{item === SEPARATOR_ITEM ? "---" : (item as TestItem).name}
<button>Blur</button>
</div>
);
},
);
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(
<div style={{ height: "400px" }}>
<ListView {...defaultProps} items={fewerItems} style={{ height: "100%" }} />
</div>,
getListViewComponent({
...defaultProps,
items: fewerItems,
}),
);
container = screen.getByRole("grid");