Skip to content
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

Include thread replies in message previews #10631

Merged
merged 27 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4b0159c
Include thread replies to message previews
weeman1337 Apr 17, 2023
fa78ad5
Extend tests
weeman1337 Apr 18, 2023
0eb978d
Fix type issue
weeman1337 May 8, 2023
19dce39
Use currentColor for thread icon
weeman1337 May 9, 2023
f0c99bb
Fix long room name overflow
weeman1337 May 9, 2023
3adffec
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 9, 2023
0d5e8a4
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 10, 2023
49f6ef1
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 10, 2023
d8edf13
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 11, 2023
3fc7288
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 12, 2023
a65969e
Update snapshots
weeman1337 May 12, 2023
db42b6b
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 15, 2023
b13bbd6
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 15, 2023
6eceeaa
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 15, 2023
7d50bb4
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 16, 2023
ee917c3
Fix preview
weeman1337 May 16, 2023
e532d8b
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 16, 2023
cc2741a
Fix typing issue
weeman1337 May 16, 2023
b560c8e
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 24, 2023
cd15bf7
Fix type issues
weeman1337 May 24, 2023
c53e950
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 24, 2023
821f49a
Tweak thread reply detection
weeman1337 May 24, 2023
537ae6d
Extend tests
weeman1337 May 25, 2023
e006fe9
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 May 31, 2023
3f41809
Fix type issue
weeman1337 May 31, 2023
12b7b57
Fix test
weeman1337 May 31, 2023
eaf7e58
Merge branch 'develop' into weeman1337/thread-reply-preview
weeman1337 Jun 1, 2023
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
18 changes: 9 additions & 9 deletions res/css/views/rooms/_RoomTile.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,18 @@ 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_title,
.mx_RoomTile_subtitle_text {
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}

Expand All @@ -74,11 +79,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 */
}
Expand Down
3 changes: 3 additions & 0 deletions res/img/compound/thread-16px.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
55 changes: 29 additions & 26 deletions src/components/views/rooms/RoomTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand All @@ -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}`;
Expand Down Expand Up @@ -96,7 +96,7 @@ export class RoomTile extends React.PureComponent<ClassProps, State> {
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();

Expand Down Expand Up @@ -208,7 +208,7 @@ export class RoomTile extends React.PureComponent<ClassProps, State> {
}

const messagePreview =
(await MessagePreviewStore.instance.getPreviewForRoom(this.props.room, this.props.tag)) ?? undefined;
(await MessagePreviewStore.instance.getPreviewForRoom(this.props.room, this.props.tag)) ?? null;
this.setState({ messagePreview });
}

Expand Down Expand Up @@ -359,6 +359,20 @@ export class RoomTile extends React.PureComponent<ClassProps, State> {
);
}

/**
* 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,
Expand All @@ -385,26 +399,15 @@ export class RoomTile extends React.PureComponent<ClassProps, State> {
);
}

let subtitle;
if (this.state.call) {
subtitle = (
<div className="mx_RoomTile_subtitle">
<RoomTileCallSummary call={this.state.call} />
</div>
);
} else if (this.props.hasLiveVoiceBroadcast) {
subtitle = <VoiceBroadcastRoomSubtitle />;
} else if (this.showMessagePreview && this.state.messagePreview) {
subtitle = (
<div
className="mx_RoomTile_subtitle"
id={messagePreviewId(this.props.room.roomId)}
title={this.state.messagePreview}
>
{this.state.messagePreview}
</div>
);
}
const subtitle = this.shouldRenderSubtitle ? (
<RoomTileSubtitle
call={this.state.call}
hasLiveVoiceBroadcast={this.props.hasLiveVoiceBroadcast}
messagePreview={this.state.messagePreview}
roomId={this.props.room.roomId}
showMessagePreview={this.props.showMessagePreview}
/>
) : null;

const titleClasses = classNames({
mx_RoomTile_title: true,
Expand Down
71 changes: 71 additions & 0 deletions src/components/views/rooms/RoomTileSubtitle.tsx
Original file line number Diff line number Diff line change
@@ -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<Props> = ({
call,
hasLiveVoiceBroadcast,
messagePreview,
roomId,
showMessagePreview,
}) => {
if (call) {
return (
<div className="mx_RoomTile_subtitle">
<RoomTileCallSummary call={call} />
</div>
);
}

if (hasLiveVoiceBroadcast) {
return <VoiceBroadcastRoomSubtitle />;
}

if (showMessagePreview && messagePreview) {
const className = classNames("mx_RoomTile_subtitle", {
"mx_RoomTile_subtitle--thread-reply": messagePreview.isThreadReply,
});

const icon = messagePreview.isThreadReply ? <ThreadIcon className="mx_Icon mx_Icon_16" /> : null;

return (
<div className={className} id={messagePreviewId(roomId)} title={messagePreview.text}>
{icon}
<span className="mx_RoomTile_subtitle_text">{messagePreview.text}</span>
</div>
);
}

return null;
};
85 changes: 68 additions & 17 deletions src/stores/room-list/MessagePreviewStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { RelationType } from "matrix-js-sdk/src/matrix";

import { ActionPayload } from "../../dispatcher/payloads";
Expand Down Expand Up @@ -96,6 +97,43 @@ interface IState {
// Empty because we don't actually use the state
}

export interface MessagePreview {
event: MatrixEvent;
isThreadReply: boolean;
text: string;
}

const isThreadReply = (event: MatrixEvent): boolean => {
// a thread root event cannot be a thread reply
if (event.isThreadRoot) return false;

const thread = event.getThread();

// it cannot be a thread reply if there is no thread
if (!thread) return false;

const relation = event.getRelation();

if (
!!relation &&
relation.rel_type === RelationType.Annotation &&
relation.event_id === thread.rootEvent?.getId()
) {
// annotations on the thread root are not a thread reply
return false;
}

return true;
};

const mkMessagePreview = (text: string, event: MatrixEvent): MessagePreview => {
return {
event,
text,
isThreadReply: isThreadReply(event),
};
};

export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
private static readonly internalInstance = (() => {
const instance = new MessagePreviewStore();
Expand All @@ -111,7 +149,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
}

// null indicates the preview is empty / irrelevant
private previews = new Map<string, Map<TagID | TAG_ANY, [MatrixEvent, string] | null>>();
private previews = new Map<string, Map<TagID | TAG_ANY, MessagePreview | null>>();

private constructor() {
super(defaultDispatcher, {});
Expand All @@ -131,7 +169,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
* @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<string | null> {
public async getPreviewForRoom(room: Room, inTagId: TagID): Promise<MessagePreview | null> {
if (!room) return null; // invalid room, just return nothing

if (!this.previews.has(room.roomId)) await this.generatePreview(room, inTagId);
Expand All @@ -140,9 +178,9 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
if (!previews) return null;

if (previews.has(inTagId)) {
return previews.get(inTagId)![1];
return previews.get(inTagId)!;
}
return previews.get(TAG_ANY)?.[1] ?? null;
return previews.get(TAG_ANY) ?? null;
}

public generatePreviewForEvent(event: MatrixEvent): string {
Expand All @@ -166,16 +204,28 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
}

private async generatePreview(room: Room, tagId?: TagID): Promise<void> {
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<TagID | TAG_ANY, [MatrixEvent, string] | null>();
map = new Map<TagID | TAG_ANY, MessagePreview | null>();
this.previews.set(room.roomId, map);
}

const previousEventInAny = map.get(TAG_ANY)?.[0];
const previousEventInAny = map.get(TAG_ANY)?.event;

// Set the tags so we know what to generate
if (!map.has(TAG_ANY)) map.set(TAG_ANY, null);
Expand All @@ -196,27 +246,28 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
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

if (!this.shouldSkipPreview(event, previousEventInAny)) {
changed = changed || anyPreview !== map.get(TAG_ANY)?.[1];
map.set(TAG_ANY, [event, 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 previousEventInTag = map.get(genTagId)?.[0];
const previousEventInTag = map.get(genTagId)?.event;
if (this.shouldSkipPreview(event, previousEventInTag)) continue;

const realTagId = genTagId === TAG_ANY ? undefined : genTagId;
const preview = previewDef.previewer.getTextFor(event, realTagId);
if (preview === anyPreview) {
changed = changed || anyPreview !== map.get(genTagId)?.[1];

if (preview === anyPreviewText) {
changed = changed || anyPreviewText !== map.get(genTagId)?.text;
map.delete(genTagId);
} else {
changed = changed || preview !== map.get(genTagId)?.[1];
map.set(genTagId, preview ? [event, preview] : null);
changed = changed || preview !== map.get(genTagId)?.text;
map.set(genTagId, preview ? mkMessagePreview(anyPreviewText, event) : null);
}
}

Expand All @@ -230,7 +281,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
}

// At this point, we didn't generate a preview so clear it
this.previews.set(room.roomId, new Map<TagID | TAG_ANY, [MatrixEvent, string] | null>());
this.previews.set(room.roomId, new Map<TagID | TAG_ANY, MessagePreview | null>());
this.emit(UPDATE_EVENT, this);
this.emit(MessagePreviewStore.getPreviewChangedEventName(room), room);
}
Expand Down
10 changes: 0 additions & 10 deletions test/components/views/rooms/EventTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,6 @@ describe("EventTile", () => {
});

describe("EventTile renderingType: ThreadsList", () => {
beforeEach(() => {
const { rootEvent } = mkThread({
room,
client,
authorId: "@alice:example.org",
participantUserIds: ["@alice:example.org"],
});
mxEvent = rootEvent;
});

it("shows an unread notification badge", () => {
const { container } = getComponent({}, TimelineRenderingType.ThreadsList);

Expand Down
Loading
Loading