Don't form continuations from thread roots (matrix-org#8166)

* Don't form continuations from thread roots

* Only apply the continuation break in the main timeline

Author: robintown

src/components/structures/MessagePanel.tsx

@@ -69,6 +69,7 @@ export function shouldFormContinuation(
     prevEvent: MatrixEvent,
     mxEvent: MatrixEvent,
     showHiddenEvents: boolean,
+    threadsEnabled: boolean,
     timelineRenderingType?: TimelineRenderingType,
 ): boolean {
     if (timelineRenderingType === TimelineRenderingType.ThreadsList) return false;
@@ -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;
 
@@ -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>();
@@ -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);
@@ -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;
             }
@@ -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);

src/components/views/rooms/SearchResultTile.tsx

@@ -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++) {
@@ -88,6 +89,7 @@ export default class SearchResultTile extends React.Component<IProps> {
                         prevEv,
                         mxEv,
                         this.context?.showHiddenEventsInTimeline,
+                        threadsEnabled,
                         TimelineRenderingType.Search,
                     );
 
@@ -102,6 +104,7 @@ export default class SearchResultTile extends React.Component<IProps> {
                             mxEv,
                             nextEv,
                             this.context?.showHiddenEventsInTimeline,
+                            threadsEnabled,
                             TimelineRenderingType.Search,
                         )
                     );

src/utils/exportUtils/HtmlExport.tsx

@@ -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";
@@ -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,
@@ -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() {
@@ -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;

test/components/structures/MessagePanel-test.js

@@ -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");
 
@@ -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);
+    });
+});

test/test-utils/test-utils.ts

@@ -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;
 }
 
 /**