Skip to content

New room list: add selection decoration #29531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions playwright/e2e/left-panel/room-list-panel/room-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,20 @@ test.describe("Room list", () => {
await page.getByRole("menuitem", { name: "leave room" }).click();
await expect(roomItem).not.toBeVisible();
});

test("should scroll to the current room", async ({ page, app, user }) => {
const roomListView = getRoomList(page);
await roomListView.hover();
// Scroll to the end of the room list
await page.mouse.wheel(0, 1000);

await roomListView.getByRole("gridcell", { name: "Open room room0" }).click();

const filters = page.getByRole("listbox", { name: "Room list filters" });
await filters.getByRole("option", { name: "People" }).click();
await expect(roomListView.getByRole("gridcell", { name: "Open room room0" })).not.toBeVisible();

await filters.getByRole("option", { name: "People" }).click();
await expect(roomListView.getByRole("gridcell", { name: "Open room room0" })).toBeVisible();
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 0 additions & 5 deletions res/css/views/rooms/RoomListPanel/_RoomList.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,4 @@

.mx_RoomList {
height: 100%;

.mx_RoomList_List {
/* Avoid when on hover, the background color to be on top of the right border */
padding-right: 1px;
}
}
4 changes: 4 additions & 0 deletions res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,7 @@
.mx_RoomListItemView_menu_open {
background-color: var(--cpd-color-bg-action-secondary-hovered);
}

.mx_RoomListItemView_selected {
background-color: var(--cpd-color-bg-action-secondary-pressed);
}
9 changes: 6 additions & 3 deletions src/components/views/rooms/RoomListPanel/RoomList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ interface RoomListProps {
/**
* A virtualized list of rooms.
*/
export function RoomList({ vm: { rooms } }: RoomListProps): JSX.Element {
export function RoomList({ vm: { rooms, activeIndex } }: RoomListProps): JSX.Element {
const roomRendererMemoized = useCallback(
({ key, index, style }: ListRowProps) => <RoomListItemView room={rooms[index]} key={key} style={style} />,
[rooms],
({ key, index, style }: ListRowProps) => (
<RoomListItemView room={rooms[index]} key={key} style={style} isSelected={activeIndex === index} />
),
[rooms, activeIndex],
);

// The first div is needed to make the virtualized list take all the remaining space and scroll correctly
Expand All @@ -41,6 +43,7 @@ export function RoomList({ vm: { rooms } }: RoomListProps): JSX.Element {
rowHeight={48}
height={height}
width={width}
scrollToIndex={activeIndex}
/>
)}
</AutoSizer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,16 @@ interface RoomListItemViewPropsProps extends React.HTMLAttributes<HTMLButtonElem
* The room to display
*/
room: Room;
/**
* Whether the room is selected
*/
isSelected: boolean;
}

/**
* An item in the room list
*/
export function RoomListItemView({ room, ...props }: RoomListItemViewPropsProps): JSX.Element {
export function RoomListItemView({ room, isSelected, ...props }: RoomListItemViewPropsProps): JSX.Element {
const vm = useRoomListItemViewModel(room);

const [isHover, setIsHover] = useState(false);
Expand All @@ -38,8 +42,10 @@ export function RoomListItemView({ room, ...props }: RoomListItemViewPropsProps)
<button
className={classNames("mx_RoomListItemView", {
mx_RoomListItemView_menu_open: showHoverDecoration,
mx_RoomListItemView_selected: isSelected,
})}
type="button"
aria-selected={isSelected}
aria-label={_t("room_list|room|open_room", { roomName: room.name })}
onClick={() => vm.openRoom()}
onMouseOver={() => setIsHover(true)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ describe("<RoomListItemView />", () => {

test("should render a room item", () => {
const onClick = jest.fn();
const { asFragment } = render(<RoomListItemView room={room} onClick={onClick} />);
const { asFragment } = render(<RoomListItemView room={room} onClick={onClick} isSelected={false} />);
expect(asFragment()).toMatchSnapshot();
});

test("should call openRoom when clicked", async () => {
const user = userEvent.setup();
render(<RoomListItemView room={room} />);
render(<RoomListItemView room={room} isSelected={false} />);

await user.click(screen.getByRole("button", { name: `Open room ${room.name}` }));
expect(defaultValue.openRoom).toHaveBeenCalled();
Expand All @@ -58,11 +58,20 @@ describe("<RoomListItemView />", () => {
mocked(useRoomListItemViewModel).mockReturnValue({ ...defaultValue, showHoverMenu: true });

const user = userEvent.setup();
render(<RoomListItemView room={room} />, withClientContextRenderOptions(matrixClient));
render(<RoomListItemView room={room} isSelected={false} />, withClientContextRenderOptions(matrixClient));
const listItem = screen.getByRole("button", { name: `Open room ${room.name}` });
expect(screen.queryByRole("button", { name: "More Options" })).toBeNull();

await user.hover(listItem);
await waitFor(() => expect(screen.getByRole("button", { name: "More Options" })).toBeInTheDocument());
});

test("should be selected if isSelected=true", async () => {
const { asFragment } = render(<RoomListItemView room={room} isSelected={true} />);
expect(screen.queryByRole("button", { name: `Open room ${room.name}` })).toHaveAttribute(
"aria-selected",
"true",
);
expect(asFragment()).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<button
aria-label="Open room room0"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 0px; width: 100%;"
Expand Down Expand Up @@ -70,6 +71,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room1"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 48px; width: 100%;"
Expand Down Expand Up @@ -116,6 +118,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room2"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 96px; width: 100%;"
Expand Down Expand Up @@ -162,6 +165,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room3"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 144px; width: 100%;"
Expand Down Expand Up @@ -208,6 +212,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room4"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 192px; width: 100%;"
Expand Down Expand Up @@ -254,6 +259,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room5"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 240px; width: 100%;"
Expand Down Expand Up @@ -300,6 +306,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room6"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 288px; width: 100%;"
Expand Down Expand Up @@ -346,6 +353,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room7"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 336px; width: 100%;"
Expand Down Expand Up @@ -392,6 +400,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room8"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 384px; width: 100%;"
Expand Down Expand Up @@ -438,6 +447,7 @@ exports[`<RoomList /> should render a room list 1`] = `
</button>
<button
aria-label="Open room room9"
aria-selected="false"
class="mx_RoomListItemView"
role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 432px; width: 100%;"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,60 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<RoomListItemView /> should be selected if isSelected=true 1`] = `
<DocumentFragment>
<button
aria-label="Open room room1"
aria-selected="true"
class="mx_RoomListItemView mx_RoomListItemView_selected"
type="button"
>
<div
class="mx_Flex mx_RoomListItemView_container"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x); --mx-flex-wrap: nowrap;"
>
<div
class="mx_DecoratedRoomAvatar"
>
<span
aria-label="Avatar"
class="_avatar_1qbcf_8 mx_BaseAvatar"
data-color="3"
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 32px;"
>
<img
alt=""
class="_image_1qbcf_41"
data-type="round"
height="32px"
loading="lazy"
referrerpolicy="no-referrer"
src="http://this.is.a.url/avatar.url/room.png"
width="32px"
/>
</span>
</div>
<div
class="mx_Flex mx_RoomListItemView_content"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: space-between; --mx-flex-gap: var(--cpd-space-3x); --mx-flex-wrap: nowrap;"
>
<span
title="room1"
>
room1
</span>
</div>
</div>
</button>
</DocumentFragment>
`;

exports[`<RoomListItemView /> should render a room item 1`] = `
<DocumentFragment>
<button
aria-label="Open room room1"
aria-selected="false"
class="mx_RoomListItemView"
type="button"
>
Expand Down
Loading