From 998d0d2f62f21c424c5610dbbaa258ac16a263e1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 May 2022 11:41:03 +0100 Subject: [PATCH 1/4] Don't consider threads for breaking continuation until they've actually been created --- src/components/structures/MessagePanel.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 1a403ba5065..21e685a2494 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -66,6 +66,10 @@ const groupedEvents = [ EventType.RoomPinnedEvents, ]; +function hasThreadSummary(event: MatrixEvent): boolean { + return event.isThreadRoot && event.getThread().length && !!event.getThread().replyToEvent; +} + // check if there is a previous event and it has the same sender as this event // and the types are the same/is in continuedTypes and the time between them is <= CONTINUATION_MAX_INTERVAL export function shouldFormContinuation( @@ -96,7 +100,7 @@ export function shouldFormContinuation( // Thread summaries in the main timeline should break up a continuation on both sides if (threadsEnabled && - (mxEvent.isThreadRoot || prevEvent.isThreadRoot) && + (hasThreadSummary(mxEvent) || hasThreadSummary(prevEvent)) && timelineRenderingType !== TimelineRenderingType.Thread ) { return false; From c6c1b7f681d5d11418ee286e2532f502d78b26bc Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 May 2022 12:27:35 +0100 Subject: [PATCH 2/4] Update tests --- test/components/structures/MessagePanel-test.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 769e90c9bbb..426cc6f70e5 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -699,7 +699,7 @@ describe('MessagePanel', function() { }); describe("shouldFormContinuation", () => { - it("does not form continuations from thread roots", () => { + it("does not form continuations from thread roots which have summaries", () => { const message1 = TestUtilsMatrix.mkMessage({ event: true, room: "!room:id", @@ -730,6 +730,13 @@ describe("shouldFormContinuation", () => { }); expect(shouldFormContinuation(message1, message2, false, true)).toEqual(true); + expect(shouldFormContinuation(message2, threadRoot, false, true)).toEqual(true); + expect(shouldFormContinuation(threadRoot, message3, false, true)).toEqual(true); + + const thread = {}; + thread.length = 1; + thread.replyToEvent = {}; + jest.spyOn(threadRoot, "getThread").mockReturnValue(thread); expect(shouldFormContinuation(message2, threadRoot, false, true)).toEqual(false); expect(shouldFormContinuation(threadRoot, message3, false, true)).toEqual(false); }); From fb41776d90dd0f5bc58ecd2cad91cea3704341a8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 May 2022 12:35:19 +0100 Subject: [PATCH 3/4] Make hasThreadSummary null thread safe --- src/components/structures/MessagePanel.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 21e685a2494..a801049b5f0 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -67,7 +67,7 @@ const groupedEvents = [ ]; function hasThreadSummary(event: MatrixEvent): boolean { - return event.isThreadRoot && event.getThread().length && !!event.getThread().replyToEvent; + return event.isThreadRoot && event?.getThread().length && !!event.getThread().replyToEvent; } // check if there is a previous event and it has the same sender as this event From b56213f9498e83001b5a614d6a43f887ddcb1920 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 13 May 2022 12:52:49 +0100 Subject: [PATCH 4/4] Apply feedback from pr review --- src/components/structures/MessagePanel.tsx | 5 +---- src/utils/EventUtils.ts | 4 ++++ test/components/structures/MessagePanel-test.js | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index a801049b5f0..f0344727b60 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -56,6 +56,7 @@ import { getEventDisplayInfo } from "../../utils/EventRenderingUtils"; import { IReadReceiptInfo } from "../views/rooms/ReadReceiptMarker"; import { haveRendererForEvent } from "../../events/EventTileFactory"; import { editorRoomKey } from "../../Editing"; +import { hasThreadSummary } from "../../utils/EventUtils"; const CONTINUATION_MAX_INTERVAL = 5 * 60 * 1000; // 5 minutes const continuedTypes = [EventType.Sticker, EventType.RoomMessage]; @@ -66,10 +67,6 @@ const groupedEvents = [ EventType.RoomPinnedEvents, ]; -function hasThreadSummary(event: MatrixEvent): boolean { - return event.isThreadRoot && event?.getThread().length && !!event.getThread().replyToEvent; -} - // check if there is a previous event and it has the same sender as this event // and the types are the same/is in continuedTypes and the time between them is <= CONTINUATION_MAX_INTERVAL export function shouldFormContinuation( diff --git a/src/utils/EventUtils.ts b/src/utils/EventUtils.ts index 57df815ca38..34e24340fcf 100644 --- a/src/utils/EventUtils.ts +++ b/src/utils/EventUtils.ts @@ -280,3 +280,7 @@ export function canForward(event: MatrixEvent): boolean { M_POLL_START.matches(event.getType()) ); } + +export function hasThreadSummary(event: MatrixEvent): boolean { + return event.isThreadRoot && event.getThread()?.length && !!event.getThread().replyToEvent; +} diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 426cc6f70e5..c511dd63a6d 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -733,9 +733,10 @@ describe("shouldFormContinuation", () => { expect(shouldFormContinuation(message2, threadRoot, false, true)).toEqual(true); expect(shouldFormContinuation(threadRoot, message3, false, true)).toEqual(true); - const thread = {}; - thread.length = 1; - thread.replyToEvent = {}; + const thread = { + length: 1, + replyToEvent: {}, + }; jest.spyOn(threadRoot, "getThread").mockReturnValue(thread); expect(shouldFormContinuation(message2, threadRoot, false, true)).toEqual(false); expect(shouldFormContinuation(threadRoot, message3, false, true)).toEqual(false);