From d74127bd14f5f458a9a1573dd9b2abaad0c408a2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 21 Oct 2022 14:28:25 +0100 Subject: [PATCH 1/3] Ensure spaces in the spotlight dialog have rounded square avatars --- src/components/views/avatars/RoomAvatar.tsx | 16 ++++------------ .../views/dialogs/spotlight/SpotlightDialog.tsx | 15 +++++++++------ src/components/views/rooms/RoomPreviewBar.tsx | 6 +++--- src/stores/ThreepidInviteStore.ts | 2 +- 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/components/views/avatars/RoomAvatar.tsx b/src/components/views/avatars/RoomAvatar.tsx index f50232fb220..c09b598ceec 100644 --- a/src/components/views/avatars/RoomAvatar.tsx +++ b/src/components/views/avatars/RoomAvatar.tsx @@ -16,11 +16,10 @@ limitations under the License. import React, { ComponentProps } from 'react'; import { Room } from 'matrix-js-sdk/src/models/room'; -import { ResizeMethod } from 'matrix-js-sdk/src/@types/partials'; import { MatrixEvent } from 'matrix-js-sdk/src/models/event'; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import classNames from "classnames"; -import { EventType } from "matrix-js-sdk/src/@types/event"; +import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import BaseAvatar from './BaseAvatar'; import ImageView from '../elements/ImageView'; @@ -39,11 +38,7 @@ interface IProps extends Omit<ComponentProps<typeof BaseAvatar>, "name" | "idNam oobData?: IOOBData & { roomId?: string; }; - width?: number; - height?: number; - resizeMethod?: ResizeMethod; viewAvatarOnClick?: boolean; - className?: string; onClick?(): void; } @@ -72,10 +67,7 @@ export default class RoomAvatar extends React.Component<IProps, IState> { } public componentWillUnmount() { - const cli = MatrixClientPeg.get(); - if (cli) { - cli.removeListener(RoomStateEvent.Events, this.onRoomStateEvents); - } + MatrixClientPeg.get()?.removeListener(RoomStateEvent.Events, this.onRoomStateEvents); } public static getDerivedStateFromProps(nextProps: IProps): IState { @@ -133,7 +125,7 @@ export default class RoomAvatar extends React.Component<IProps, IState> { public render() { const { room, oobData, viewAvatarOnClick, onClick, className, ...otherProps } = this.props; - const roomName = room ? room.name : oobData.name; + const roomName = room?.name ?? oobData.name; // If the room is a DM, we use the other user's ID for the color hash // in order to match the room avatar with their avatar const idName = room ? (DMRoomMap.shared().getUserIdForRoomId(room.roomId) ?? room.roomId) : oobData.roomId; @@ -142,7 +134,7 @@ export default class RoomAvatar extends React.Component<IProps, IState> { <BaseAvatar {...otherProps} className={classNames(className, { - mx_RoomAvatar_isSpaceRoom: room?.isSpaceRoom(), + mx_RoomAvatar_isSpaceRoom: (room?.getType() ?? this.props.oobData?.roomType) === RoomType.Space, })} name={roomName} idName={idName} diff --git a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx index dfec2ab5097..6703afc0879 100644 --- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx +++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx @@ -93,6 +93,7 @@ import { TooltipOption } from "./TooltipOption"; import { isLocalRoom } from "../../../../utils/localRoom/isLocalRoom"; import { useSlidingSyncRoomSearch } from "../../../../hooks/useSlidingSyncRoomSearch"; import { shouldShowFeedback } from "../../../../utils/Feedback"; +import RoomAvatar from "../../avatars/RoomAvatar"; const MAX_RECENT_SEARCHES = 10; const SECTION_LIMIT = 50; // only show 50 results per section for performance reasons @@ -656,6 +657,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n shouldPeek: result.publicRoom.world_readable || cli.isGuest(), }, true, ev.type !== "click"); }; + return ( <Option id={`mx_SpotlightDialog_button_result_${result.publicRoom.room_id}`} @@ -674,13 +676,14 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n aria-describedby={`mx_SpotlightDialog_button_result_${result.publicRoom.room_id}_alias`} aria-details={`mx_SpotlightDialog_button_result_${result.publicRoom.room_id}_details`} > - <BaseAvatar + <RoomAvatar className="mx_SearchResultAvatar" - url={result?.publicRoom?.avatar_url - ? mediaFromMxc(result?.publicRoom?.avatar_url).getSquareThumbnailHttp(AVATAR_SIZE) - : null} - name={result.publicRoom.name} - idName={result.publicRoom.room_id} + oobData={{ + roomId: result.publicRoom.room_id, + name: result.publicRoom.name, + avatarUrl: result.publicRoom.avatar_url, + roomType: result.publicRoom.room_type, + }} width={AVATAR_SIZE} height={AVATAR_SIZE} /> diff --git a/src/components/views/rooms/RoomPreviewBar.tsx b/src/components/views/rooms/RoomPreviewBar.tsx index 39c82e4eeaa..eecdf00147a 100644 --- a/src/components/views/rooms/RoomPreviewBar.tsx +++ b/src/components/views/rooms/RoomPreviewBar.tsx @@ -263,9 +263,9 @@ export default class RoomPreviewBar extends React.Component<IProps, IState> { params: { email: this.props.invitedEmail, signurl: this.props.signUrl, - room_name: this.props.oobData ? this.props.oobData.room_name : null, - room_avatar_url: this.props.oobData ? this.props.oobData.avatarUrl : null, - inviter_name: this.props.oobData ? this.props.oobData.inviterName : null, + room_name: this.props.oobData?.name ?? null, + room_avatar_url: this.props.oobData?.avatarUrl ?? null, + inviter_name: this.props.oobData?.inviterName ?? null, }, }; } diff --git a/src/stores/ThreepidInviteStore.ts b/src/stores/ThreepidInviteStore.ts index 9b597ba877b..fe2df8f66e7 100644 --- a/src/stores/ThreepidInviteStore.ts +++ b/src/stores/ThreepidInviteStore.ts @@ -56,7 +56,7 @@ export interface IOOBData { inviterName?: string; // The display name of the person who invited us to the room // eslint-disable-next-line camelcase room_name?: string; // The name of the room, to be used until we are told better by the server - roomType?: RoomType; // The type of the room, to be used until we are told better by the server + roomType?: RoomType | string; // The type of the room, to be used until we are told better by the server } const STORAGE_PREFIX = "mx_threepid_invite_"; From 4ca9c8056e4d674e56ef73ffc0355146a4d62177 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 21 Oct 2022 14:55:25 +0100 Subject: [PATCH 2/3] Add mocks for `getType` on Room --- test/Avatar-test.ts | 4 +++- test/components/views/dialogs/InviteDialog-test.tsx | 2 ++ test/components/views/right_panel/UserInfo-test.tsx | 1 + test/stores/room-list/filters/VisibilityProvider-test.ts | 3 ++- test/test-utils/test-utils.ts | 3 +++ 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/test/Avatar-test.ts b/test/Avatar-test.ts index 214ada9486a..0ff064ed57d 100644 --- a/test/Avatar-test.ts +++ b/test/Avatar-test.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { mocked } from "jest-mock"; -import { Room, RoomMember } from "matrix-js-sdk/src/matrix"; +import { Room, RoomMember, RoomType } from "matrix-js-sdk/src/matrix"; import { avatarUrlForRoom } from "../src/Avatar"; import { Media, mediaFromMxc } from "../src/customisations/Media"; @@ -46,6 +46,7 @@ describe("avatarUrlForRoom", () => { roomId, getMxcAvatarUrl: jest.fn(), isSpaceRoom: jest.fn(), + getType: jest.fn(), getAvatarFallbackMember: jest.fn(), } as unknown as Room; dmRoomMap = { @@ -70,6 +71,7 @@ describe("avatarUrlForRoom", () => { it("should return null for a space room", () => { mocked(room.isSpaceRoom).mockReturnValue(true); + mocked(room.getType).mockReturnValue(RoomType.Space); expect(avatarUrlForRoom(room, 128, 128)).toBeNull(); }); diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx index 469cbde96b3..6a82506e761 100644 --- a/test/components/views/dialogs/InviteDialog-test.tsx +++ b/test/components/views/dialogs/InviteDialog-test.tsx @@ -16,6 +16,7 @@ limitations under the License. import React from "react"; import { render, screen } from "@testing-library/react"; +import { RoomType } from "matrix-js-sdk/src/@types/event"; import InviteDialog from "../../../../src/components/views/dialogs/InviteDialog"; import { KIND_INVITE } from "../../../../src/components/views/dialogs/InviteDialogTypes"; @@ -74,6 +75,7 @@ describe("InviteDialog", () => { it("should label with space name", () => { mockClient.getRoom(roomId).isSpaceRoom = jest.fn().mockReturnValue(true); + mockClient.getRoom(roomId).getType = jest.fn().mockReturnValue(RoomType.Space); mockClient.getRoom(roomId).name = "Space"; render(( <InviteDialog diff --git a/test/components/views/right_panel/UserInfo-test.tsx b/test/components/views/right_panel/UserInfo-test.tsx index 6ec36cb096b..f76661fc689 100644 --- a/test/components/views/right_panel/UserInfo-test.tsx +++ b/test/components/views/right_panel/UserInfo-test.tsx @@ -140,6 +140,7 @@ describe('<UserInfo />', () => { describe('with a room', () => { const room = { roomId: '!fkfk', + getType: jest.fn().mockReturnValue(undefined), isSpaceRoom: jest.fn().mockReturnValue(false), getMember: jest.fn().mockReturnValue(undefined), getMxcAvatarUrl: jest.fn().mockReturnValue('mock-avatar-url'), diff --git a/test/stores/room-list/filters/VisibilityProvider-test.ts b/test/stores/room-list/filters/VisibilityProvider-test.ts index f22901a40f1..ca6c67dfb19 100644 --- a/test/stores/room-list/filters/VisibilityProvider-test.ts +++ b/test/stores/room-list/filters/VisibilityProvider-test.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { mocked } from "jest-mock"; -import { Room } from "matrix-js-sdk/src/matrix"; +import { Room, RoomType } from "matrix-js-sdk/src/matrix"; import { VisibilityProvider } from "../../../../src/stores/room-list/filters/VisibilityProvider"; import LegacyCallHandler from "../../../../src/LegacyCallHandler"; @@ -43,6 +43,7 @@ jest.mock("../../../../src/customisations/RoomList", () => ({ const createRoom = (isSpaceRoom = false): Room => { return { isSpaceRoom: () => isSpaceRoom, + getType: () => isSpaceRoom ? RoomType.Space : undefined, } as unknown as Room; }; diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 4549190600d..8bd6e3d460e 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -31,6 +31,7 @@ import { IEventRelation, IUnsigned, IPusher, + RoomType, } from 'matrix-js-sdk/src/matrix'; import { normalize } from "matrix-js-sdk/src/utils"; import { ReEmitter } from "matrix-js-sdk/src/ReEmitter"; @@ -447,6 +448,7 @@ export function mkStubRoom(roomId: string = null, name: string, client: MatrixCl getAvatarUrl: () => 'mxc://avatar.url/room.png', getMxcAvatarUrl: () => 'mxc://avatar.url/room.png', isSpaceRoom: jest.fn().mockReturnValue(false), + getType: jest.fn().mockReturnValue(undefined), isElementVideoRoom: jest.fn().mockReturnValue(false), getUnreadNotificationCount: jest.fn(() => 0), getEventReadUpTo: jest.fn(() => null), @@ -544,6 +546,7 @@ export const mkSpace = ( ): MockedObject<Room> => { const space = mocked(mkRoom(client, spaceId, rooms)); space.isSpaceRoom.mockReturnValue(true); + space.getType.mockReturnValue(RoomType.Space); mocked(space.currentState).getStateEvents.mockImplementation(mockStateEventImplementation(children.map(roomId => mkEvent({ event: true, From 1353c7b283f94f4ed3302d6e27af5295f197ed3a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 24 Oct 2022 09:34:41 +0100 Subject: [PATCH 3/3] Add tests --- .../views/rooms/RoomPreviewBar-test.tsx | 71 ++++++++++++------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/test/components/views/rooms/RoomPreviewBar-test.tsx b/test/components/views/rooms/RoomPreviewBar-test.tsx index d785f5b0240..e7c8fa72814 100644 --- a/test/components/views/rooms/RoomPreviewBar-test.tsx +++ b/test/components/views/rooms/RoomPreviewBar-test.tsx @@ -15,18 +15,14 @@ limitations under the License. */ import React from 'react'; -import { - renderIntoDocument, - Simulate, - findRenderedDOMComponentWithClass, - act, -} from 'react-dom/test-utils'; +import { render, fireEvent, RenderResult, waitFor } from "@testing-library/react"; import { Room, RoomMember, MatrixError, IContent } from 'matrix-js-sdk/src/matrix'; import { stubClient } from '../../../test-utils'; import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; import DMRoomMap from '../../../../src/utils/DMRoomMap'; import RoomPreviewBar from '../../../../src/components/views/rooms/RoomPreviewBar'; +import defaultDispatcher from "../../../../src/dispatcher/dispatcher"; jest.mock('../../../../src/IdentityAuthClient', () => { return jest.fn().mockImplementation(() => { @@ -79,19 +75,18 @@ describe('<RoomPreviewBar />', () => { const defaultProps = { room: createRoom(roomId, userId), }; - const wrapper = renderIntoDocument<React.Component>( - <RoomPreviewBar {...defaultProps} {...props} />, - ) as React.Component; - return findRenderedDOMComponentWithClass(wrapper, 'mx_RoomPreviewBar') as HTMLDivElement; + return render(<RoomPreviewBar {...defaultProps} {...props} />); }; - const isSpinnerRendered = (element: Element) => !!element.querySelector('.mx_Spinner'); - const getMessage = (element: Element) => element.querySelector<HTMLDivElement>('.mx_RoomPreviewBar_message'); - const getActions = (element: Element) => element.querySelector<HTMLDivElement>('.mx_RoomPreviewBar_actions'); - const getPrimaryActionButton = (element: Element) => - getActions(element).querySelector('.mx_AccessibleButton_kind_primary'); - const getSecondaryActionButton = (element: Element) => - getActions(element).querySelector('.mx_AccessibleButton_kind_secondary'); + const isSpinnerRendered = (wrapper: RenderResult) => !!wrapper.container.querySelector('.mx_Spinner'); + const getMessage = (wrapper: RenderResult) => + wrapper.container.querySelector<HTMLDivElement>('.mx_RoomPreviewBar_message'); + const getActions = (wrapper: RenderResult) => + wrapper.container.querySelector<HTMLDivElement>('.mx_RoomPreviewBar_actions'); + const getPrimaryActionButton = (wrapper: RenderResult) => + getActions(wrapper).querySelector('.mx_AccessibleButton_kind_primary'); + const getSecondaryActionButton = (wrapper: RenderResult) => + getActions(wrapper).querySelector('.mx_AccessibleButton_kind_secondary'); beforeEach(() => { stubClient(); @@ -128,6 +123,36 @@ describe('<RoomPreviewBar />', () => { expect(getMessage(component).textContent).toEqual('Join the conversation with an account'); }); + it("should send room oob data to start login", async () => { + MatrixClientPeg.get().isGuest = jest.fn().mockReturnValue(true); + const component = getComponent({ + oobData: { + name: "Room Name", + avatarUrl: "mxc://foo/bar", + inviterName: "Charlie", + }, + }); + + const dispatcherSpy = jest.fn(); + const dispatcherRef = defaultDispatcher.register(dispatcherSpy); + + expect(getMessage(component).textContent).toEqual('Join the conversation with an account'); + fireEvent.click(getPrimaryActionButton(component)); + + await waitFor(() => expect(dispatcherSpy).toHaveBeenCalledWith(expect.objectContaining({ + screenAfterLogin: { + screen: 'room', + params: expect.objectContaining({ + room_name: "Room Name", + room_avatar_url: "mxc://foo/bar", + inviter_name: "Charlie", + }), + }, + }))); + + defaultDispatcher.unregister(dispatcherRef); + }); + it('renders kicked message', () => { const room = createRoom(roomId, otherUserId); jest.spyOn(room, 'getMember').mockReturnValue(makeMockRoomMember({ isKicked: true })); @@ -233,18 +258,14 @@ describe('<RoomPreviewBar />', () => { it('joins room on primary button click', () => { const component = getComponent({ inviterName, room, onJoinClick, onRejectClick }); - act(() => { - Simulate.click(getPrimaryActionButton(component)); - }); + fireEvent.click(getPrimaryActionButton(component)); expect(onJoinClick).toHaveBeenCalled(); }); it('rejects invite on secondary button click', () => { const component = getComponent({ inviterName, room, onJoinClick, onRejectClick }); - act(() => { - Simulate.click(getSecondaryActionButton(component)); - }); + fireEvent.click(getSecondaryActionButton(component)); expect(onRejectClick).toHaveBeenCalled(); }); @@ -296,9 +317,7 @@ describe('<RoomPreviewBar />', () => { await new Promise(setImmediate); expect(getPrimaryActionButton(component)).toBeTruthy(); expect(getSecondaryActionButton(component)).toBeFalsy(); - act(() => { - Simulate.click(getPrimaryActionButton(component)); - }); + fireEvent.click(getPrimaryActionButton(component)); expect(onJoinClick).toHaveBeenCalled(); };