From 4b0159c2afcfa9fa20cc2cbf669b5882a43ee07d Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 17 Apr 2023 15:56:51 +0200 Subject: [PATCH 01/13] Include thread replies to message previews --- res/css/views/rooms/_RoomTile.pcss | 17 +- res/img/compound/thread-16px.svg | 3 + src/components/views/rooms/RoomTile.tsx | 53 ++-- .../views/rooms/RoomTileSubtitle.tsx | 71 +++++ src/stores/room-list/MessagePreviewStore.ts | 51 +++- test/components/views/rooms/RoomTile-test.tsx | 259 ++++++++++++------ .../__snapshots__/RoomTile-test.tsx.snap | 243 +++++++++++++++- 7 files changed, 559 insertions(+), 138 deletions(-) create mode 100644 res/img/compound/thread-16px.svg create mode 100644 src/components/views/rooms/RoomTileSubtitle.tsx diff --git a/res/css/views/rooms/_RoomTile.pcss b/res/css/views/rooms/_RoomTile.pcss index d0db686da81..e9071d58acd 100644 --- a/res/css/views/rooms/_RoomTile.pcss +++ b/res/css/views/rooms/_RoomTile.pcss @@ -55,13 +55,17 @@ limitations under the License. flex-direction: column; justify-content: center; - .mx_RoomTile_title, .mx_RoomTile_subtitle { - width: 100%; + align-items: center; + color: $secondary-content; + display: flex; + gap: $spacing-4; + line-height: $font-18px; + } - /* Ellipsize any text overflow */ - text-overflow: ellipsis; + .mx_RoomTile_subtitle_text { overflow: hidden; + text-overflow: ellipsis; white-space: nowrap; } @@ -74,11 +78,6 @@ limitations under the License. } } - .mx_RoomTile_subtitle { - line-height: $font-18px; - color: $secondary-content; - } - .mx_RoomTile_titleWithSubtitle { margin-top: -3px; /* shift the title up a bit more */ } diff --git a/res/img/compound/thread-16px.svg b/res/img/compound/thread-16px.svg new file mode 100644 index 00000000000..fa6880588d4 --- /dev/null +++ b/res/img/compound/thread-16px.svg @@ -0,0 +1,3 @@ + + + diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 58664951df0..9367263cd36 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -27,7 +27,7 @@ import { Action } from "../../../dispatcher/actions"; import { _t } from "../../../languageHandler"; import { ChevronFace, ContextMenuTooltipButton, MenuProps } from "../../structures/ContextMenu"; import { DefaultTagID, TagID } from "../../../stores/room-list/models"; -import { MessagePreviewStore } from "../../../stores/room-list/MessagePreviewStore"; +import { MessagePreview, MessagePreviewStore } from "../../../stores/room-list/MessagePreviewStore"; import DecoratedRoomAvatar from "../avatars/DecoratedRoomAvatar"; import { RoomNotifState } from "../../../RoomNotifs"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; @@ -44,11 +44,11 @@ import PosthogTrackers from "../../../PosthogTrackers"; import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload"; import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { getKeyBindingsManager } from "../../../KeyBindingsManager"; -import { RoomTileCallSummary } from "./RoomTileCallSummary"; import { RoomGeneralContextMenu } from "../context_menus/RoomGeneralContextMenu"; import { CallStore, CallStoreEvent } from "../../../stores/CallStore"; import { SdkContextClass } from "../../../contexts/SDKContext"; -import { useHasRoomLiveVoiceBroadcast, VoiceBroadcastRoomSubtitle } from "../../../voice-broadcast"; +import { useHasRoomLiveVoiceBroadcast } from "../../../voice-broadcast"; +import { RoomTileSubtitle } from "./RoomTileSubtitle"; interface Props { room: Room; @@ -68,7 +68,7 @@ interface State { notificationsMenuPosition: PartialDOMRect | null; generalMenuPosition: PartialDOMRect | null; call: Call | null; - messagePreview?: string; + messagePreview: MessagePreview | null; } const messagePreviewId = (roomId: string): string => `mx_RoomTile_messagePreview_${roomId}`; @@ -96,7 +96,7 @@ export class RoomTile extends React.PureComponent { generalMenuPosition: null, call: CallStore.instance.getCall(this.props.room.roomId), // generatePreview() will return nothing if the user has previews disabled - messagePreview: "", + messagePreview: null, }; this.generatePreview(); @@ -359,6 +359,20 @@ export class RoomTile extends React.PureComponent { ); } + /** + * RoomTile has a subtile if one of the following applies: + * - there is a call + * - there is a live voice broadcast + * - message previews are enabled and there is a previewable message + */ + private get shouldRenderSubtitle(): boolean { + return ( + !!this.state.call || + this.props.hasLiveVoiceBroadcast || + (this.props.showMessagePreview && !!this.state.messagePreview) + ); + } + public render(): React.ReactElement { const classes = classNames({ mx_RoomTile: true, @@ -385,26 +399,15 @@ export class RoomTile extends React.PureComponent { ); } - let subtitle; - if (this.state.call) { - subtitle = ( -
- -
- ); - } else if (this.props.hasLiveVoiceBroadcast) { - subtitle = ; - } else if (this.showMessagePreview && this.state.messagePreview) { - subtitle = ( -
- {this.state.messagePreview} -
- ); - } + const subtitle = this.shouldRenderSubtitle ? ( + + ) : null; const titleClasses = classNames({ mx_RoomTile_title: true, diff --git a/src/components/views/rooms/RoomTileSubtitle.tsx b/src/components/views/rooms/RoomTileSubtitle.tsx new file mode 100644 index 00000000000..55ff0dea9ae --- /dev/null +++ b/src/components/views/rooms/RoomTileSubtitle.tsx @@ -0,0 +1,71 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import classNames from "classnames"; + +import { MessagePreview } from "../../../stores/room-list/MessagePreviewStore"; +import { Call } from "../../../models/Call"; +import { RoomTileCallSummary } from "./RoomTileCallSummary"; +import { VoiceBroadcastRoomSubtitle } from "../../../voice-broadcast"; +import { Icon as ThreadIcon } from "../../../../res/img/compound/thread-16px.svg"; + +interface Props { + call: Call | null; + hasLiveVoiceBroadcast: boolean; + messagePreview: MessagePreview | null; + roomId: string; + showMessagePreview: boolean; +} + +const messagePreviewId = (roomId: string): string => `mx_RoomTile_messagePreview_${roomId}`; + +export const RoomTileSubtitle: React.FC = ({ + call, + hasLiveVoiceBroadcast, + messagePreview, + roomId, + showMessagePreview, +}) => { + if (call) { + return ( +
+ +
+ ); + } + + if (hasLiveVoiceBroadcast) { + return ; + } + + if (showMessagePreview && messagePreview) { + const className = classNames("mx_RoomTile_subtitle", { + "mx_RoomTile_subtitle--thread-reply": messagePreview.isThreadReply, + }); + + const icon = messagePreview.isThreadReply ? : null; + + return ( +
+ {icon} + {messagePreview.text} +
+ ); + } + + return null; +}; diff --git a/src/stores/room-list/MessagePreviewStore.ts b/src/stores/room-list/MessagePreviewStore.ts index f8af88aed8d..86cada36645 100644 --- a/src/stores/room-list/MessagePreviewStore.ts +++ b/src/stores/room-list/MessagePreviewStore.ts @@ -18,6 +18,7 @@ import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { M_POLL_START } from "matrix-js-sdk/src/@types/polls"; +import { Thread } from "matrix-js-sdk/src/models/thread"; import { ActionPayload } from "../../dispatcher/payloads"; import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; @@ -95,6 +96,18 @@ interface IState { // Empty because we don't actually use the state } +export interface MessagePreview { + isThreadReply: boolean; + text: string; +} + +const mkMessagePreview = (text: string, event: MatrixEvent): MessagePreview => { + return { + text, + isThreadReply: !!event.getThread() && !event.isThreadRoot, + }; +}; + export class MessagePreviewStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { const instance = new MessagePreviewStore(); @@ -103,7 +116,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient { })(); // null indicates the preview is empty / irrelevant - private previews = new Map>(); + private previews = new Map>(); private constructor() { super(defaultDispatcher, {}); @@ -123,7 +136,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient { * @param inTagId The tag ID in which the room resides * @returns The preview, or null if none present. */ - public async getPreviewForRoom(room: Room, inTagId: TagID): Promise { + public async getPreviewForRoom(room: Room, inTagId: TagID): Promise { if (!room) return null; // invalid room, just return nothing if (!this.previews.has(room.roomId)) await this.generatePreview(room, inTagId); @@ -143,12 +156,24 @@ export class MessagePreviewStore extends AsyncStoreWithClient { } private async generatePreview(room: Room, tagId?: TagID): Promise { - const events = room.timeline; + const events = [...room.getLiveTimeline().getEvents()]; + + // add last reply from each thread + room.getThreads().forEach((thread: Thread): void => { + const lastReply = thread.lastReply(); + if (lastReply) events.push(lastReply); + }); + + // sort events from oldest to newest + events.sort((a: MatrixEvent, b: MatrixEvent) => { + return a.getTs() - b.getTs(); + }); + if (!events) return; // should only happen in tests let map = this.previews.get(room.roomId); if (!map) { - map = new Map(); + map = new Map(); this.previews.set(room.roomId, map); } @@ -171,22 +196,22 @@ export class MessagePreviewStore extends AsyncStoreWithClient { if (!previewDef) continue; if (previewDef.isState && isNullOrUndefined(event.getStateKey())) continue; - const anyPreview = previewDef.previewer.getTextFor(event); - if (!anyPreview) continue; // not previewable for some reason + const anyPreviewText = previewDef.previewer.getTextFor(event); + if (!anyPreviewText) continue; // not previewable for some reason - changed = changed || anyPreview !== map.get(TAG_ANY); - map.set(TAG_ANY, anyPreview); + changed = changed || anyPreviewText !== map.get(TAG_ANY)?.text; + map.set(TAG_ANY, mkMessagePreview(anyPreviewText, event)); const tagsToGenerate = Array.from(map.keys()).filter((t) => t !== TAG_ANY); // we did the any tag above for (const genTagId of tagsToGenerate) { const realTagId = genTagId === TAG_ANY ? undefined : genTagId; const preview = previewDef.previewer.getTextFor(event, realTagId); - if (preview === anyPreview) { - changed = changed || anyPreview !== map.get(genTagId); + if (preview === anyPreviewText) { + changed = changed || anyPreviewText !== map.get(genTagId)?.text; map.delete(genTagId); } else { - changed = changed || preview !== map.get(genTagId); - map.set(genTagId, preview); + changed = changed || preview !== map.get(genTagId)?.text; + map.set(genTagId, mkMessagePreview(anyPreviewText, event)); } } @@ -200,7 +225,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient { } // At this point, we didn't generate a preview so clear it - this.previews.set(room.roomId, new Map()); + this.previews.set(room.roomId, new Map()); this.emit(UPDATE_EVENT, this); this.emit(MessagePreviewStore.getPreviewChangedEventName(room), room); } diff --git a/test/components/views/rooms/RoomTile-test.tsx b/test/components/views/rooms/RoomTile-test.tsx index b4bfde243c1..25f53af9d74 100644 --- a/test/components/views/rooms/RoomTile-test.tsx +++ b/test/components/views/rooms/RoomTile-test.tsx @@ -22,6 +22,7 @@ import { Room } from "matrix-js-sdk/src/models/room"; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { Widget } from "matrix-widget-api"; import { MatrixEvent } from "matrix-js-sdk/src/matrix"; +import { Thread } from "matrix-js-sdk/src/models/thread"; import type { RoomMember } from "matrix-js-sdk/src/models/room-member"; import type { ClientWidgetApi } from "matrix-widget-api"; @@ -33,6 +34,7 @@ import { setupAsyncStoreWithClient, filterConsole, flushPromises, + mkMessage, } from "../../../test-utils"; import { CallStore } from "../../../../src/stores/CallStore"; import RoomTile from "../../../../src/components/views/rooms/RoomTile"; @@ -45,6 +47,7 @@ import { VoiceBroadcastInfoState } from "../../../../src/voice-broadcast"; import { mkVoiceBroadcastInfoStateEvent } from "../../../voice-broadcast/utils/test-utils"; import { TestSdkContext } from "../../../TestSdkContext"; import { SDKContext } from "../../../../src/contexts/SDKContext"; +import { MessagePreviewStore } from "../../../../src/stores/room-list/MessagePreviewStore"; describe("RoomTile", () => { jest.spyOn(PlatformPeg, "get").mockReturnValue({ @@ -69,7 +72,12 @@ describe("RoomTile", () => { const renderRoomTile = (): void => { renderResult = render( - + , ); }; @@ -79,6 +87,7 @@ describe("RoomTile", () => { let room: Room; let renderResult: RenderResult; let sdkContext: TestSdkContext; + let showMessagePreview = false; filterConsole( // irrelevant for this test @@ -99,130 +108,200 @@ describe("RoomTile", () => { client.getRoom.mockImplementation((roomId) => (roomId === room.roomId ? room : null)); client.getRooms.mockReturnValue([room]); client.reEmitter.reEmit(room, [RoomStateEvent.Events]); - - renderRoomTile(); }); afterEach(() => { - jest.clearAllMocks(); - }); - - it("should render the room", () => { - expect(renderResult.container).toMatchSnapshot(); + // @ts-ignore + MessagePreviewStore.instance.previews = new Map>(); + jest.restoreAllMocks(); }); - describe("when a call starts", () => { - let call: MockedCall; - let widget: Widget; - + describe("when message previews are not enabled", () => { beforeEach(() => { - setupAsyncStoreWithClient(CallStore.instance, client); - setupAsyncStoreWithClient(WidgetMessagingStore.instance, client); - - MockedCall.create(room, "1"); - const maybeCall = CallStore.instance.getCall(room.roomId); - if (!(maybeCall instanceof MockedCall)) throw new Error("Failed to create call"); - call = maybeCall; - - widget = new Widget(call.widget); - WidgetMessagingStore.instance.storeMessaging(widget, room.roomId, { - stop: () => {}, - } as unknown as ClientWidgetApi); + renderRoomTile(); }); - afterEach(() => { - renderResult.unmount(); - call.destroy(); - client.reEmitter.stopReEmitting(room, [RoomStateEvent.Events]); - WidgetMessagingStore.instance.stopMessaging(widget, room.roomId); + it("should render the room", () => { + expect(renderResult.container).toMatchSnapshot(); }); - it("tracks connection state", async () => { - screen.getByText("Video"); - - // Insert an await point in the connection method so we can inspect - // the intermediate connecting state - let completeConnection: () => void = () => {}; - const connectionCompleted = new Promise((resolve) => (completeConnection = resolve)); - jest.spyOn(call, "performConnection").mockReturnValue(connectionCompleted); - - await Promise.all([ - (async () => { - await screen.findByText("Joining…"); - const joinedFound = screen.findByText("Joined"); - completeConnection(); - await joinedFound; - })(), - call.connect(), - ]); - - await Promise.all([screen.findByText("Video"), call.disconnect()]); - }); + describe("when a call starts", () => { + let call: MockedCall; + let widget: Widget; - it("tracks participants", () => { - const alice: [RoomMember, Set] = [mkRoomMember(room.roomId, "@alice:example.org"), new Set(["a"])]; - const bob: [RoomMember, Set] = [ - mkRoomMember(room.roomId, "@bob:example.org"), - new Set(["b1", "b2"]), - ]; - const carol: [RoomMember, Set] = [mkRoomMember(room.roomId, "@carol:example.org"), new Set(["c"])]; + beforeEach(() => { + setupAsyncStoreWithClient(CallStore.instance, client); + setupAsyncStoreWithClient(WidgetMessagingStore.instance, client); - expect(screen.queryByLabelText(/participant/)).toBe(null); + MockedCall.create(room, "1"); + const maybeCall = CallStore.instance.getCall(room.roomId); + if (!(maybeCall instanceof MockedCall)) throw new Error("Failed to create call"); + call = maybeCall; - act(() => { - call.participants = new Map([alice]); + widget = new Widget(call.widget); + WidgetMessagingStore.instance.storeMessaging(widget, room.roomId, { + stop: () => {}, + } as unknown as ClientWidgetApi); }); - expect(screen.getByLabelText("1 participant").textContent).toBe("1"); - act(() => { - call.participants = new Map([alice, bob, carol]); + afterEach(() => { + renderResult.unmount(); + call.destroy(); + client.reEmitter.stopReEmitting(room, [RoomStateEvent.Events]); + WidgetMessagingStore.instance.stopMessaging(widget, room.roomId); }); - expect(screen.getByLabelText("4 participants").textContent).toBe("4"); - act(() => { - call.participants = new Map(); + it("tracks connection state", async () => { + screen.getByText("Video"); + + // Insert an await point in the connection method so we can inspect + // the intermediate connecting state + let completeConnection: () => void = () => {}; + const connectionCompleted = new Promise((resolve) => (completeConnection = resolve)); + jest.spyOn(call, "performConnection").mockReturnValue(connectionCompleted); + + await Promise.all([ + (async () => { + await screen.findByText("Joining…"); + const joinedFound = screen.findByText("Joined"); + completeConnection(); + await joinedFound; + })(), + call.connect(), + ]); + + await Promise.all([screen.findByText("Video"), call.disconnect()]); + }); + + it("tracks participants", () => { + const alice: [RoomMember, Set] = [ + mkRoomMember(room.roomId, "@alice:example.org"), + new Set(["a"]), + ]; + const bob: [RoomMember, Set] = [ + mkRoomMember(room.roomId, "@bob:example.org"), + new Set(["b1", "b2"]), + ]; + const carol: [RoomMember, Set] = [ + mkRoomMember(room.roomId, "@carol:example.org"), + new Set(["c"]), + ]; + + expect(screen.queryByLabelText(/participant/)).toBe(null); + + act(() => { + call.participants = new Map([alice]); + }); + expect(screen.getByLabelText("1 participant").textContent).toBe("1"); + + act(() => { + call.participants = new Map([alice, bob, carol]); + }); + expect(screen.getByLabelText("4 participants").textContent).toBe("4"); + + act(() => { + call.participants = new Map(); + }); + expect(screen.queryByLabelText(/participant/)).toBe(null); + }); + + describe("and a live broadcast starts", () => { + beforeEach(async () => { + await setUpVoiceBroadcast(VoiceBroadcastInfoState.Started); + }); + + it("should still render the call subtitle", () => { + expect(screen.queryByText("Video")).toBeInTheDocument(); + expect(screen.queryByText("Live")).not.toBeInTheDocument(); + }); }); - expect(screen.queryByLabelText(/participant/)).toBe(null); }); - describe("and a live broadcast starts", () => { + describe("when a live voice broadcast starts", () => { beforeEach(async () => { await setUpVoiceBroadcast(VoiceBroadcastInfoState.Started); }); - it("should still render the call subtitle", () => { - expect(screen.queryByText("Video")).toBeInTheDocument(); - expect(screen.queryByText("Live")).not.toBeInTheDocument(); + it("should render the »Live« subtitle", () => { + expect(screen.queryByText("Live")).toBeInTheDocument(); + }); + + describe("and the broadcast stops", () => { + beforeEach(async () => { + const stopEvent = mkVoiceBroadcastInfoStateEvent( + room.roomId, + VoiceBroadcastInfoState.Stopped, + client.getSafeUserId(), + client.getDeviceId()!, + voiceBroadcastInfoEvent, + ); + await act(async () => { + room.currentState.setStateEvents([stopEvent]); + await flushPromises(); + }); + }); + + it("should not render the »Live« subtitle", () => { + expect(screen.queryByText("Live")).not.toBeInTheDocument(); + }); }); }); }); - describe("when a live voice broadcast starts", () => { - beforeEach(async () => { - await setUpVoiceBroadcast(VoiceBroadcastInfoState.Started); + describe("when message previews are enabled", () => { + beforeEach(() => { + showMessagePreview = true; }); - it("should render the »Live« subtitle", () => { - expect(screen.queryByText("Live")).toBeInTheDocument(); + it("should render a room without a message as expected", async () => { + renderRoomTile(); + // flush promises here because the preview is created asynchronously + await flushPromises(); + expect(renderResult.asFragment()).toMatchSnapshot(); }); - describe("and the broadcast stops", () => { - beforeEach(async () => { - const stopEvent = mkVoiceBroadcastInfoStateEvent( - room.roomId, - VoiceBroadcastInfoState.Stopped, - client.getSafeUserId(), - client.getDeviceId()!, - voiceBroadcastInfoEvent, - ); - await act(async () => { - room.currentState.setStateEvents([stopEvent]); - await flushPromises(); + describe("and there is a message in the room", () => { + beforeEach(() => { + const message = mkMessage({ + event: true, + room: room.roomId, + msg: "test message", + user: client.getSafeUserId(), }); + + room.timeline.push(message); + }); + + it("should render as expected", async () => { + renderRoomTile(); + expect(await screen.findByText("test message")).toBeInTheDocument(); + expect(renderResult.asFragment()).toMatchSnapshot(); + }); + }); + + describe("and there is a message in a thread", () => { + beforeEach(() => { + const message = mkMessage({ + event: true, + room: room.roomId, + msg: "test thread reply", + user: client.getSafeUserId(), + }); + + // Mock thread reply for tests. + jest.spyOn(room, "getThreads").mockReturnValue([ + // @ts-ignore + { + lastReply: () => message, + timeline: [], + } as Thread, + ]); }); - it("should not render the »Live« subtitle", () => { - expect(screen.queryByText("Live")).not.toBeInTheDocument(); + it("should render as expected", async () => { + renderRoomTile(); + expect(await screen.findByText("test thread reply")).toBeInTheDocument(); + expect(renderResult.asFragment()).toMatchSnapshot(); }); }); }); diff --git a/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap b/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap index bcbb7932c69..e19534d6bef 100644 --- a/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap +++ b/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap @@ -1,6 +1,247 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`RoomTile should render the room 1`] = ` +exports[`RoomTile when message previews are enabled and there is a message in a thread should render as expected 1`] = ` + +
+
+ + + + +
+
+
+ + !1:​example.org + +
+
+ + test thread reply + +
+
+