Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Don't form continuations from thread roots #8166

Merged
merged 2 commits into from
Mar 26, 2022
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
23 changes: 17 additions & 6 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export function shouldFormContinuation(
prevEvent: MatrixEvent,
mxEvent: MatrixEvent,
showHiddenEvents: boolean,
threadsEnabled: boolean,
timelineRenderingType?: TimelineRenderingType,
): boolean {
if (timelineRenderingType === TimelineRenderingType.ThreadsList) return false;
Expand All @@ -90,6 +91,10 @@ export function shouldFormContinuation(
mxEvent.sender.name !== prevEvent.sender.name ||
mxEvent.sender.getMxcAvatarUrl() !== prevEvent.sender.getMxcAvatarUrl()) return false;

// Thread summaries in the main timeline should break up a continuation
if (threadsEnabled && prevEvent.isThreadRoot &&
timelineRenderingType !== TimelineRenderingType.Thread) return false;

// if we don't have tile for previous event then it was shown by showHiddenEvents and has no SenderProfile
if (!haveTileForEvent(prevEvent, showHiddenEvents)) return false;

Expand Down Expand Up @@ -241,6 +246,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
private readReceiptsByUserId: Record<string, IReadReceiptForUser> = {};

private readonly showHiddenEventsInTimeline: boolean;
private readonly threadsEnabled: boolean;
private isMounted = false;

private readMarkerNode = createRef<HTMLLIElement>();
Expand All @@ -264,10 +270,11 @@ export default class MessagePanel extends React.Component<IProps, IState> {
hideSender: this.shouldHideSender(),
};

// Cache hidden events setting on mount since Settings is expensive to
// query, and we check this in a hot code path. This is also cached in
// our RoomContext, however we still need a fallback for roomless MessagePanels.
// Cache these settings on mount since Settings is expensive to query,
// and we check this in a hot code path. This is also cached in our
// RoomContext, however we still need a fallback for roomless MessagePanels.
this.showHiddenEventsInTimeline = SettingsStore.getValue("showHiddenEventsInTimeline");
this.threadsEnabled = SettingsStore.getValue("feature_thread");

this.showTypingNotificationsWatcherRef =
SettingsStore.watchSetting("showTypingNotifications", null, this.onShowTypingNotificationsChange);
Expand Down Expand Up @@ -465,7 +472,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {

// TODO: Implement granular (per-room) hide options
public shouldShowEvent(mxEv: MatrixEvent, forceHideEvents = false): boolean {
if (this.props.hideThreadedMessages && SettingsStore.getValue("feature_thread")) {
if (this.props.hideThreadedMessages && this.threadsEnabled) {
if (mxEv.isThreadRelation) {
return false;
}
Expand Down Expand Up @@ -744,12 +751,16 @@ export default class MessagePanel extends React.Component<IProps, IState> {
lastInSection = willWantDateSeparator ||
mxEv.getSender() !== nextEv.getSender() ||
getEventDisplayInfo(nextEv).isInfoMessage ||
!shouldFormContinuation(mxEv, nextEv, this.showHiddenEvents, this.context.timelineRenderingType);
!shouldFormContinuation(
mxEv, nextEv, this.showHiddenEvents, this.threadsEnabled, this.context.timelineRenderingType,
);
}

// is this a continuation of the previous message?
const continuation = !wantsDateSeparator &&
shouldFormContinuation(prevEvent, mxEv, this.showHiddenEvents, this.context.timelineRenderingType);
shouldFormContinuation(
prevEvent, mxEv, this.showHiddenEvents, this.threadsEnabled, this.context.timelineRenderingType,
);

const eventId = mxEv.getId();
const highlight = (eventId === this.props.highlightedEventId);
Expand Down
3 changes: 3 additions & 0 deletions src/components/views/rooms/SearchResultTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export default class SearchResultTile extends React.Component<IProps> {
const layout = SettingsStore.getValue("layout");
const isTwelveHour = SettingsStore.getValue("showTwelveHourTimestamps");
const alwaysShowTimestamps = SettingsStore.getValue("alwaysShowTimestamps");
const threadsEnabled = SettingsStore.getValue("feature_thread");

const timeline = result.context.getTimeline();
for (let j = 0; j < timeline.length; j++) {
Expand All @@ -88,6 +89,7 @@ export default class SearchResultTile extends React.Component<IProps> {
prevEv,
mxEv,
this.context?.showHiddenEventsInTimeline,
threadsEnabled,
TimelineRenderingType.Search,
);

Expand All @@ -102,6 +104,7 @@ export default class SearchResultTile extends React.Component<IProps> {
mxEv,
nextEv,
this.context?.showHiddenEventsInTimeline,
threadsEnabled,
TimelineRenderingType.Search,
)
);
Expand Down
7 changes: 5 additions & 2 deletions src/utils/exportUtils/HtmlExport.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { EventType, MsgType } from "matrix-js-sdk/src/@types/event";
import { logger } from "matrix-js-sdk/src/logger";

import Exporter from "./Exporter";
import SettingsStore from "../../settings/SettingsStore";
import { mediaFromMxc } from "../../customisations/Media";
import { Layout } from "../../settings/enums/Layout";
import { shouldFormContinuation } from "../../components/structures/MessagePanel";
Expand All @@ -46,6 +47,7 @@ export default class HTMLExporter extends Exporter {
protected permalinkCreator: RoomPermalinkCreator;
protected totalSize: number;
protected mediaOmitText: string;
private threadsEnabled: boolean;

constructor(
room: Room,
Expand All @@ -60,6 +62,7 @@ export default class HTMLExporter extends Exporter {
this.mediaOmitText = !this.exportOptions.attachmentsIncluded
? _t("Media omitted")
: _t("Media omitted - file size limit exceeded");
this.threadsEnabled = SettingsStore.getValue("feature_thread");
}

protected async getRoomAvatar() {
Expand Down Expand Up @@ -406,8 +409,8 @@ export default class HTMLExporter extends Exporter {
if (!haveTileForEvent(event)) continue;

content += this.needsDateSeparator(event, prevEvent) ? this.getDateSeparator(event) : "";
const shouldBeJoined = !this.needsDateSeparator(event, prevEvent)
&& shouldFormContinuation(prevEvent, event, false);
const shouldBeJoined = !this.needsDateSeparator(event, prevEvent) &&
shouldFormContinuation(prevEvent, event, false, this.threadsEnabled);
const body = await this.createMessageBody(event, shouldBeJoined);
this.totalSize += Buffer.byteLength(body);
content += body;
Expand Down
25 changes: 23 additions & 2 deletions test/components/structures/MessagePanel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ import * as TestUtils from "react-dom/test-utils";

import { MatrixClientPeg } from '../../../src/MatrixClientPeg';
import sdk from '../../skinned-sdk';
import MessagePanel, { shouldFormContinuation } from "../../../src/components/structures/MessagePanel";
import SettingsStore from "../../../src/settings/SettingsStore";
import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
import RoomContext from "../../../src/contexts/RoomContext";
import DMRoomMap from "../../../src/utils/DMRoomMap";
import { UnwrappedEventTile } from "../../../src/components/views/rooms/EventTile";
import * as TestUtilsMatrix from "../../test-utils";

const MessagePanel = sdk.getComponent('structures.MessagePanel');

let client;
const room = new Matrix.Room("!roomId:server_name");

Expand Down Expand Up @@ -594,3 +593,25 @@ describe('MessagePanel', function() {
expect(els.last().prop("events").length).toEqual(5);
});
});

describe("shouldFormContinuation", () => {
it("does not form continuations from thread roots", () => {
const threadRoot = TestUtilsMatrix.mkMessage({
event: true,
room: "!room:id",
user: "@user:id",
msg: "Here is a thread",
});
jest.spyOn(threadRoot, "isThreadRoot", "get").mockReturnValue(true);

const message = TestUtilsMatrix.mkMessage({
event: true,
room: "!room:id",
user: "@user:id",
msg: "And here's another message in the main timeline",
});

expect(shouldFormContinuation(threadRoot, message, false, true)).toEqual(false);
expect(shouldFormContinuation(message, threadRoot, false, true)).toEqual(true);
});
});
15 changes: 14 additions & 1 deletion test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,20 @@ export function mkEvent(opts: MakeEventProps): MatrixEvent {
].indexOf(opts.type) !== -1) {
event.state_key = "";
}
return opts.event ? new MatrixEvent(event) : event as unknown as MatrixEvent;

const mxEvent = opts.event ? new MatrixEvent(event) : event as unknown as MatrixEvent;
if (!mxEvent.sender && opts.user && opts.room) {
mxEvent.sender = {
userId: opts.user,
membership: "join",
name: opts.user,
rawDisplayName: opts.user,
roomId: opts.room,
getAvatarUrl: () => {},
getMxcAvatarUrl: () => {},
} as unknown as RoomMember;
}
return mxEvent;
}

/**
Expand Down