Remove onSelectItem and space/enter handing from ListView (#30601)

* Remove onSelectItem and space/enter handing from ListView(And therefore memberlist).)

* remove unused imports

* fix unit test
This commit is contained in:
David Langley
2025-08-20 17:09:44 +01:00
committed by GitHub
parent 4b6e5d380e
commit 4735412c91
4 changed files with 3 additions and 68 deletions

View File

@@ -29,12 +29,6 @@ export interface IListViewProps<Item, Context>
*/ */
items: Item[]; 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. * Function that renders each list item as a JSX element.
* @param index - The index of the item in the list * @param index - The index of the item in the list
@@ -79,7 +73,7 @@ export interface IListViewProps<Item, Context>
*/ */
export function ListView<Item, Context = any>(props: IListViewProps<Item, Context>): React.ReactElement { export function ListView<Item, Context = any>(props: IListViewProps<Item, Context>): React.ReactElement {
// Extract our custom props to avoid conflicts with Virtuoso props // 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 */ /** Reference to the Virtuoso component for programmatic scrolling */
const virtuosoHandleRef = useRef<VirtuosoHandle>(null); const virtuosoHandleRef = useRef<VirtuosoHandle>(null);
/** Reference to the DOM element containing the virtualized list */ /** Reference to the DOM element containing the virtualized list */
@@ -186,10 +180,6 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
} else if (e.code === "ArrowDown" && currentIndex !== undefined) { } else if (e.code === "ArrowDown" && currentIndex !== undefined) {
scrollToItem(currentIndex + 1, true); scrollToItem(currentIndex + 1, true);
handled = 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") { } else if (e.code === "Home") {
scrollToIndex(0); scrollToIndex(0);
handled = true; handled = true;
@@ -211,7 +201,7 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
e.preventDefault(); e.preventDefault();
} }
}, },
[scrollToIndex, scrollToItem, tabIndexKey, keyToIndexMap, visibleRange, items, onSelectItem], [scrollToIndex, scrollToItem, tabIndexKey, keyToIndexMap, visibleRange, items],
); );
/** /**

View File

@@ -38,8 +38,6 @@ import { isValid3pidInvite } from "../../../RoomInvite";
import { type ThreePIDInvite } from "../../../models/rooms/ThreePIDInvite"; import { type ThreePIDInvite } from "../../../models/rooms/ThreePIDInvite";
import { type XOR } from "../../../@types/common"; import { type XOR } from "../../../@types/common";
import { useTypedEventEmitter } from "../../../hooks/useEventEmitter"; import { useTypedEventEmitter } from "../../../hooks/useEventEmitter";
import { Action } from "../../../dispatcher/actions";
import dis from "../../../dispatcher/dispatcher";
type Member = XOR<{ member: RoomMember }, { threePidInvite: ThreePIDInvite }>; type Member = XOR<{ member: RoomMember }, { threePidInvite: ThreePIDInvite }>;
@@ -113,7 +111,6 @@ export interface MemberListViewState {
shouldShowSearch: boolean; shouldShowSearch: boolean;
isLoading: boolean; isLoading: boolean;
canInvite: boolean; canInvite: boolean;
onClickMember: (member: RoomMember | ThreePIDInvite) => void;
onInviteButtonClick: (ev: ButtonEvent) => void; onInviteButtonClick: (ev: ButtonEvent) => void;
} }
export function useMemberListViewModel(roomId: string): MemberListViewState { export function useMemberListViewModel(roomId: string): MemberListViewState {
@@ -136,14 +133,6 @@ export function useMemberListViewModel(roomId: string): MemberListViewState {
*/ */
const [memberCount, setMemberCount] = useState(0); const [memberCount, setMemberCount] = useState(0);
const onClickMember = (member: RoomMember | ThreePIDInvite): void => {
dis.dispatch({
action: Action.ViewUser,
member: member,
push: true,
});
};
const loadMembers = useMemo( const loadMembers = useMemo(
() => () =>
throttle( throttle(
@@ -278,7 +267,6 @@ export function useMemberListViewModel(roomId: string): MemberListViewState {
isPresenceEnabled, isPresenceEnabled,
isLoading, isLoading,
onInviteButtonClick, onInviteButtonClick,
onClickMember,
shouldShowSearch: totalMemberCount >= 20, shouldShowSearch: totalMemberCount >= 20,
canInvite, canInvite,
}; };

View File

@@ -28,7 +28,7 @@ interface IProps {
const MemberListView: React.FC<IProps> = (props: IProps) => { const MemberListView: React.FC<IProps> = (props: IProps) => {
const vm = useMemberListViewModel(props.roomId); const vm = useMemberListViewModel(props.roomId);
const { isPresenceEnabled, onClickMember, memberCount } = vm; const { isPresenceEnabled, memberCount } = vm;
const getItemKey = useCallback((item: MemberWithSeparator): string => { const getItemKey = useCallback((item: MemberWithSeparator): string => {
if (item === SEPARATOR) { if (item === SEPARATOR) {
@@ -80,19 +80,6 @@ const MemberListView: React.FC<IProps> = (props: IProps) => {
[isPresenceEnabled, getItemKey, memberCount], [isPresenceEnabled, getItemKey, memberCount],
); );
const handleSelectItem = useCallback(
(item: MemberWithSeparator): void => {
if (item !== SEPARATOR) {
if (item.member) {
onClickMember(item.member);
} else {
onClickMember(item.threePidInvite);
}
}
},
[onClickMember],
);
const isItemFocusable = useCallback((item: MemberWithSeparator): boolean => { const isItemFocusable = useCallback((item: MemberWithSeparator): boolean => {
return item !== SEPARATOR; return item !== SEPARATOR;
}, []); }, []);
@@ -112,7 +99,6 @@ const MemberListView: React.FC<IProps> = (props: IProps) => {
</Form.Root> </Form.Root>
<ListView <ListView
items={vm.members} items={vm.members}
onSelectItem={handleSelectItem}
getItemComponent={getItemComponent} getItemComponent={getItemComponent}
getItemKey={getItemKey} getItemKey={getItemKey}
isItemFocusable={isItemFocusable} isItemFocusable={isItemFocusable}

View File

@@ -21,7 +21,6 @@ const SEPARATOR_ITEM = "SEPARATOR" as const;
type TestItemWithSeparator = TestItem | typeof SEPARATOR_ITEM; type TestItemWithSeparator = TestItem | typeof SEPARATOR_ITEM;
describe("ListView", () => { describe("ListView", () => {
const mockOnSelectItem = jest.fn();
const mockGetItemComponent = jest.fn(); const mockGetItemComponent = jest.fn();
const mockIsItemFocusable = jest.fn(); const mockIsItemFocusable = jest.fn();
@@ -34,7 +33,6 @@ describe("ListView", () => {
const defaultProps: IListViewProps<TestItemWithSeparator, any> = { const defaultProps: IListViewProps<TestItemWithSeparator, any> = {
items: defaultItems, items: defaultItems,
onSelectItem: mockOnSelectItem,
getItemComponent: mockGetItemComponent, getItemComponent: mockGetItemComponent,
isItemFocusable: mockIsItemFocusable, isItemFocusable: mockIsItemFocusable,
getItemKey: (item) => (typeof item === "string" ? item : item.id), getItemKey: (item) => (typeof item === "string" ? item : item.id),
@@ -87,33 +85,6 @@ describe("ListView", () => {
}); });
describe("Keyboard Navigation", () => { 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", () => { it("should handle ArrowDown key navigation", () => {
renderListViewWithHeight(); renderListViewWithHeight();
const container = screen.getByRole("grid"); const container = screen.getByRole("grid");