From 6c3e1912ef8ea4025b9664a35efd66e0faef4d0d Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 10 Feb 2023 10:42:23 +0000 Subject: [PATCH 1/9] Rename mxEv to event --- src/components/structures/MessagePanel.tsx | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 14224a081c5..b568baaa6dd 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -564,7 +564,7 @@ export default class MessagePanel extends React.Component { /** * Find the next event in the list, and the next visible event in the list. * - * @param event - the list of events to look in and whether they are shown + * @param events - the list of events to look in and whether they are shown * @param i - where in the list we are now * * @returns { nextEvent, nextTile } @@ -615,16 +615,16 @@ export default class MessagePanel extends React.Component { let lastShownNonLocalEchoIndex = -1; for (let i = events.length - 1; i >= 0; i--) { - const { event: mxEv, shouldShow } = events[i]; + const { event, shouldShow } = events[i]; if (!shouldShow) { continue; } if (lastShownEvent === undefined) { - lastShownEvent = mxEv; + lastShownEvent = event; } - if (mxEv.status) { + if (event.status) { // this is a local echo continue; } @@ -647,14 +647,14 @@ export default class MessagePanel extends React.Component { let grouper: BaseGrouper | null = null; for (let i = 0; i < events.length; i++) { - const { event: mxEv, shouldShow } = events[i]; - const eventId = mxEv.getId(); - const last = mxEv === lastShownEvent; + const { event, shouldShow } = events[i]; + const eventId = event.getId(); + const last = event === lastShownEvent; const { nextEvent, nextTile } = this.getNextEventInfo(events, i); if (grouper) { - if (grouper.shouldGroup(mxEv)) { - grouper.add(mxEv); + if (grouper.shouldGroup(event)) { + grouper.add(event); continue; } else { // not part of group, so get the group tiles, close the @@ -666,8 +666,8 @@ export default class MessagePanel extends React.Component { } for (const Grouper of groupers) { - if (Grouper.canStartGroup(this, mxEv) && !this.props.disableGrouping) { - grouper = new Grouper(this, mxEv, prevEvent, lastShownEvent, nextEvent, nextTile); + if (Grouper.canStartGroup(this, event) && !this.props.disableGrouping) { + grouper = new Grouper(this, event, prevEvent, lastShownEvent, nextEvent, nextTile); break; // break on first grouper } } @@ -677,8 +677,8 @@ export default class MessagePanel extends React.Component { // make sure we unpack the array returned by getTilesForEvent, // otherwise React will auto-generate keys, and we will end up // replacing all the DOM elements every time we paginate. - ret.push(...this.getTilesForEvent(prevEvent, mxEv, last, false, nextEvent, nextTile)); - prevEvent = mxEv; + ret.push(...this.getTilesForEvent(prevEvent, event, last, false, nextEvent, nextTile)); + prevEvent = event; } const readMarker = this.readMarkerForEvent(eventId, i >= lastShownNonLocalEchoIndex); From 3c2e015033d18b6bec0b80a91380e24607b53d46 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 10 Mar 2023 14:48:59 +0000 Subject: [PATCH 2/9] Add a comment about why we're not using slice --- src/components/structures/MessagePanel.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index b568baaa6dd..10fddadd68a 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -1399,6 +1399,10 @@ const groupers = [CreationGrouper, MainGrouper]; * event that is >start items through the list, and is shown. */ function findFirstShownAfter(start: number, events: EventAndShouldShow[]): MatrixEvent | null { + // Note: this could be done with something like: + // events.slice(i + 1).find((e) => e.shouldShow)?.event ?? null; + // but it is ~10% slower, and this is on the critical path. + for (let n = start + 1; n < events.length; n++) { const { event, shouldShow } = events[n]; if (shouldShow) { From 1bcf6353ebdc64fd2194207473b8acda46b2b552 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 10 Mar 2023 15:48:43 +0000 Subject: [PATCH 3/9] Avoid re-calculating shouldShowEvent in getReadReceiptsByShownEvent --- src/components/structures/MessagePanel.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 10fddadd68a..f2b57c89e7f 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -641,7 +641,7 @@ export default class MessagePanel extends React.Component { // to assume that sent receipts are to be shown more often. this.readReceiptsByEvent = {}; if (this.props.showReadReceipts) { - this.readReceiptsByEvent = this.getReadReceiptsByShownEvent(); + this.readReceiptsByEvent = this.getReadReceiptsByShownEvent(events); } let grouper: BaseGrouper | null = null; @@ -861,7 +861,7 @@ export default class MessagePanel extends React.Component { // Get an object that maps from event ID to a list of read receipts that // should be shown next to that event. If a hidden event has read receipts, // they are folded into the receipts of the last shown event. - private getReadReceiptsByShownEvent(): Record { + private getReadReceiptsByShownEvent(events: EventAndShouldShow[]): Record { const receiptsByEvent: Record = {}; const receiptsByUserId: Record< string, @@ -872,8 +872,8 @@ export default class MessagePanel extends React.Component { > = {}; let lastShownEventId; - for (const event of this.props.events) { - if (this.shouldShowEvent(event)) { + for (const { event, shouldShow } of events) { + if (shouldShow) { lastShownEventId = event.getId(); } if (!lastShownEventId) { From 7dc3cc1e8721512e712c8f13739226384ee7c5d9 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 14 Mar 2023 10:59:47 +0000 Subject: [PATCH 4/9] Test that uses a MainGrouper --- .../structures/MessagePanel-test.tsx | 50 +++++++ .../__snapshots__/MessagePanel-test.tsx.snap | 129 ++++++++++++++++++ 2 files changed, 179 insertions(+) diff --git a/test/components/structures/MessagePanel-test.tsx b/test/components/structures/MessagePanel-test.tsx index f9b4597699b..79d6ce963c8 100644 --- a/test/components/structures/MessagePanel-test.tsx +++ b/test/components/structures/MessagePanel-test.tsx @@ -690,6 +690,56 @@ describe("MessagePanel", function () { const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false })); expect(asFragment()).toMatchSnapshot(); }); + + it("should handle lots of room creation events quickly", () => { + // Increase the length of the loop here to test performance issues with + // rendering + + const events = [TestUtilsMatrix.mkRoomCreateEvent("@user:id", "!room:id")]; + for (let i = 0; i < 100; i++) { + events.push( + TestUtilsMatrix.mkMembership({ + mship: "join", + prevMship: "join", + room: "!room:id", + user: "@user:id", + event: true, + skey: "123", + }), + ); + } + const { asFragment } = render(getComponent({ events }, { showHiddenEvents: false })); + expect(asFragment()).toMatchSnapshot(); + }); + + it("should handle lots of membership events quickly", () => { + // Increase the length of the loop here to test performance issues with + // rendering + + const events = []; + for (let i = 0; i < 100; i++) { + events.push( + TestUtilsMatrix.mkMembership({ + mship: "join", + prevMship: "join", + room: "!room:id", + user: "@user:id", + event: true, + skey: "123", + }), + ); + } + const { asFragment } = render(getComponent({ events }, { showHiddenEvents: true })); + const cpt = asFragment(); + + // Ignore properties that change every time + cpt.querySelectorAll("li").forEach((li) => { + li.setAttribute("data-scroll-tokens", "__scroll_tokens__"); + li.setAttribute("data-testid", "__testid__"); + }); + + expect(cpt).toMatchSnapshot(); + }); }); describe("shouldFormContinuation", () => { diff --git a/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap b/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap index 4397e05860d..68e95ec8d95 100644 --- a/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap +++ b/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap @@ -19,3 +19,132 @@ exports[`MessagePanel should handle large numbers of hidden events quickly 1`] = ); `; + +exports[`MessagePanel should handle lots of membership events quickly 1`] = ` + +
+
+
    +
  1. + +
  2. +
    +
    + You can't see earlier messages +
    +
    +
  3. + +
    +
    + + + + + + + + + @user:id made no changes 1000 times + + +
    +
    +
  4. +
+
+
+ ); +
+`; + +exports[`MessagePanel should handle lots of room creation events quickly 1`] = ` + +
+
+
    +
+
+ ); +
+`; From efda76279f9af91c9ef90d6b1183c123c43694e8 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 10 Mar 2023 16:11:56 +0000 Subject: [PATCH 5/9] Cache more calls to shouldShowEvent --- src/components/structures/MessagePanel.tsx | 96 ++++++++++++---------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index f2b57c89e7f..cd023706ca5 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -578,10 +578,10 @@ export default class MessagePanel extends React.Component { private getNextEventInfo( events: EventAndShouldShow[], i: number, - ): { nextEvent: MatrixEvent | null; nextTile: MatrixEvent | null } { + ): { nextEvent: EventAndShouldShow | null; nextTile: MatrixEvent | null } { // WARNING: this method is on a hot path. - const nextEvent = i < events.length - 1 ? events[i + 1].event : null; + const nextEvent = i < events.length - 1 ? events[i + 1] : null; const nextTile = findFirstShownAfter(i, events); @@ -647,14 +647,15 @@ export default class MessagePanel extends React.Component { let grouper: BaseGrouper | null = null; for (let i = 0; i < events.length; i++) { - const { event, shouldShow } = events[i]; + const ev = events[i]; + const { event, shouldShow } = ev; const eventId = event.getId(); const last = event === lastShownEvent; const { nextEvent, nextTile } = this.getNextEventInfo(events, i); if (grouper) { - if (grouper.shouldGroup(event)) { - grouper.add(event); + if (grouper.shouldGroup(ev)) { + grouper.add(ev); continue; } else { // not part of group, so get the group tiles, close the @@ -666,8 +667,8 @@ export default class MessagePanel extends React.Component { } for (const Grouper of groupers) { - if (Grouper.canStartGroup(this, event) && !this.props.disableGrouping) { - grouper = new Grouper(this, event, prevEvent, lastShownEvent, nextEvent, nextTile); + if (Grouper.canStartGroup(this, ev) && !this.props.disableGrouping) { + grouper = new Grouper(this, ev, prevEvent, lastShownEvent, nextEvent, nextTile); break; // break on first grouper } } @@ -698,7 +699,7 @@ export default class MessagePanel extends React.Component { mxEv: MatrixEvent, last = false, isGrouped = false, - nextEvent?: MatrixEvent, + nextEvent?: EventAndShouldShow, nextEventWithTile?: MatrixEvent, ): ReactNode[] { const ret: ReactNode[] = []; @@ -747,10 +748,10 @@ export default class MessagePanel extends React.Component { let isLastSuccessful = false; const isSentState = (s: EventStatus): boolean => !s || s === EventStatus.SENT; const isSent = isSentState(mxEv.getAssociatedStatus()); - const hasNextEvent = nextEvent && this.shouldShowEvent(nextEvent); + const hasNextEvent = nextEvent?.shouldShow; if (!hasNextEvent && isSent) { isLastSuccessful = true; - } else if (hasNextEvent && isSent && !isSentState(nextEvent.getAssociatedStatus())) { + } else if (hasNextEvent && isSent && !isSentState(nextEvent.event.getAssociatedStatus())) { isLastSuccessful = true; } @@ -758,7 +759,7 @@ export default class MessagePanel extends React.Component { // hidden then we're not the last successful. if ( nextEventWithTile && - nextEventWithTile !== nextEvent && + nextEventWithTile !== nextEvent?.event && isSentState(nextEventWithTile.getAssociatedStatus()) ) { isLastSuccessful = false; @@ -1065,7 +1066,7 @@ interface EventAndShouldShow { } abstract class BaseGrouper { - public static canStartGroup = (panel: MessagePanel, ev: MatrixEvent): boolean => true; + public static canStartGroup = (panel: MessagePanel, ev: EventAndShouldShow): boolean => true; public events: MatrixEvent[] = []; // events that we include in the group but then eject out and place above the group. @@ -1074,17 +1075,17 @@ abstract class BaseGrouper { public constructor( public readonly panel: MessagePanel, - public readonly event: MatrixEvent, + public readonly event: EventAndShouldShow, public readonly prevEvent: MatrixEvent | null, public readonly lastShownEvent: MatrixEvent, - public readonly nextEvent?: MatrixEvent, + public readonly nextEvent?: EventAndShouldShow, public readonly nextEventTile?: MatrixEvent, ) { - this.readMarker = panel.readMarkerForEvent(event.getId(), event === lastShownEvent); + this.readMarker = panel.readMarkerForEvent(event.event.getId(), event.event === lastShownEvent); } - public abstract shouldGroup(ev: MatrixEvent): boolean; - public abstract add(ev: MatrixEvent): void; + public abstract shouldGroup(ev: EventAndShouldShow): boolean; + public abstract add(ev: EventAndShouldShow): void; public abstract getTiles(): ReactNode[]; public abstract getNewPrevEvent(): MatrixEvent; } @@ -1105,28 +1106,30 @@ abstract class BaseGrouper { // Grouping only events sent by the same user that sent the `m.room.create` and only until // the first non-state event, beacon_info event or membership event which is not regarding the sender of the `m.room.create` event class CreationGrouper extends BaseGrouper { - public static canStartGroup = function (panel: MessagePanel, ev: MatrixEvent): boolean { + public static canStartGroup = function ( + panel: MessagePanel, + { event: ev, shouldShow }: EventAndShouldShow, + ): boolean { return ev.getType() === EventType.RoomCreate; }; - public shouldGroup(ev: MatrixEvent): boolean { + public shouldGroup({ event: ev, shouldShow }: EventAndShouldShow): boolean { const panel = this.panel; const createEvent = this.event; - if (!panel.shouldShowEvent(ev)) { + if (!shouldShow) { return true; } - if (panel.wantsDateSeparator(this.event, ev.getDate())) { + if (panel.wantsDateSeparator(this.event.event, ev.getDate())) { return false; } + const eventType = ev.getType(); if ( - ev.getType() === EventType.RoomMember && - (ev.getStateKey() !== createEvent.getSender() || ev.getContent()["membership"] !== "join") + eventType === EventType.RoomMember && + (ev.getStateKey() !== createEvent.event.getSender() || ev.getContent()["membership"] !== "join") ) { return false; } - const eventType = ev.getType(); - // beacons are not part of room creation configuration // should be shown in timeline if (M_BEACON_INFO.matches(eventType)) { @@ -1138,17 +1141,17 @@ class CreationGrouper extends BaseGrouper { return false; } - if (ev.isState() && ev.getSender() === createEvent.getSender()) { + if (ev.isState() && ev.getSender() === createEvent.event.getSender()) { return true; } return false; } - public add(ev: MatrixEvent): void { + public add({ event: ev, shouldShow }: EventAndShouldShow): void { const panel = this.panel; this.readMarker = this.readMarker || panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent); - if (!panel.shouldShowEvent(ev)) { + if (!shouldShow) { return; } if (ev.getType() === EventType.RoomEncryption) { @@ -1170,23 +1173,25 @@ class CreationGrouper extends BaseGrouper { const createEvent = this.event; const lastShownEvent = this.lastShownEvent; - if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) { - const ts = createEvent.getTs(); + if (panel.wantsDateSeparator(this.prevEvent, createEvent.event.getDate())) { + const ts = createEvent.event.getTs(); ret.push(
  • - +
  • , ); } // If this m.room.create event should be shown (room upgrade) then show it before the summary - if (panel.shouldShowEvent(createEvent)) { + if (createEvent.shouldShow) { // pass in the createEvent as prevEvent as well so no extra DateSeparator is rendered - ret.push(...panel.getTilesForEvent(createEvent, createEvent)); + ret.push(...panel.getTilesForEvent(createEvent.event, createEvent.event)); } for (const ejected of this.ejectedEvents) { - ret.push(...panel.getTilesForEvent(createEvent, ejected, createEvent === lastShownEvent, isGrouped)); + ret.push( + ...panel.getTilesForEvent(createEvent.event, ejected, createEvent.event === lastShownEvent, isGrouped), + ); } const eventTiles = this.events @@ -1233,14 +1238,17 @@ class CreationGrouper extends BaseGrouper { } public getNewPrevEvent(): MatrixEvent { - return this.event; + return this.event.event; } } // Wrap consecutive grouped events in a ListSummary class MainGrouper extends BaseGrouper { - public static canStartGroup = function (panel: MessagePanel, ev: MatrixEvent): boolean { - if (!panel.shouldShowEvent(ev)) return false; + public static canStartGroup = function ( + panel: MessagePanel, + { event: ev, shouldShow }: EventAndShouldShow, + ): boolean { + if (!shouldShow) return false; if (ev.isState() && groupedStateEvents.includes(ev.getType() as EventType)) { return true; @@ -1259,18 +1267,18 @@ class MainGrouper extends BaseGrouper { public constructor( public readonly panel: MessagePanel, - public readonly event: MatrixEvent, + public readonly event: EventAndShouldShow, public readonly prevEvent: MatrixEvent | null, public readonly lastShownEvent: MatrixEvent, - nextEvent: MatrixEvent, + nextEvent: EventAndShouldShow, nextEventTile: MatrixEvent, ) { super(panel, event, prevEvent, lastShownEvent, nextEvent, nextEventTile); - this.events = [event]; + this.events = [event.event]; } - public shouldGroup(ev: MatrixEvent): boolean { - if (!this.panel.shouldShowEvent(ev)) { + public shouldGroup({ event: ev, shouldShow }: EventAndShouldShow): boolean { + if (!shouldShow) { // absorb hidden events so that they do not break up streams of messages & redaction events being grouped return true; } @@ -1289,13 +1297,13 @@ class MainGrouper extends BaseGrouper { return false; } - public add(ev: MatrixEvent): void { + public add({ event: ev, shouldShow }: EventAndShouldShow): void { if (ev.getType() === EventType.RoomMember) { // We can ignore any events that don't actually have a message to display if (!hasText(ev, this.panel.showHiddenEvents)) return; } this.readMarker = this.readMarker || this.panel.readMarkerForEvent(ev.getId(), ev === this.lastShownEvent); - if (!this.panel.showHiddenEvents && !this.panel.shouldShowEvent(ev)) { + if (!this.panel.showHiddenEvents && !shouldShow) { // absorb hidden events to not split the summary return; } From a8203f67048654c44791aaa3c6a93a2c2e3c9cb3 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 14 Mar 2023 15:48:17 +0000 Subject: [PATCH 6/9] Fix snapshot --- .../structures/__snapshots__/MessagePanel-test.tsx.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap b/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap index 68e95ec8d95..98465671f0c 100644 --- a/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap +++ b/test/components/structures/__snapshots__/MessagePanel-test.tsx.snap @@ -116,7 +116,7 @@ exports[`MessagePanel should handle lots of membership events quickly 1`] = ` class="mx_TextualEvent mx_GenericEventListSummary_summary" > - @user:id made no changes 1000 times + @user:id made no changes 100 times From 592a7b08684d091bee2b5fb253f0b3879cd67c24 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 14 Mar 2023 15:50:54 +0000 Subject: [PATCH 7/9] Fix type of nextEvent --- 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 cd023706ca5..32a7e6a4d82 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -699,7 +699,7 @@ export default class MessagePanel extends React.Component { mxEv: MatrixEvent, last = false, isGrouped = false, - nextEvent?: EventAndShouldShow, + nextEvent: EventAndShouldShow = null, nextEventWithTile?: MatrixEvent, ): ReactNode[] { const ret: ReactNode[] = []; From 94101d271d72a0dc990d3e8e2b4f132910d3f7ef Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Thu, 16 Mar 2023 13:53:37 +0000 Subject: [PATCH 8/9] Improve typing slightly --- src/components/structures/MessagePanel.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 32a7e6a4d82..3e5d12ae2c0 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -699,8 +699,8 @@ export default class MessagePanel extends React.Component { mxEv: MatrixEvent, last = false, isGrouped = false, - nextEvent: EventAndShouldShow = null, - nextEventWithTile?: MatrixEvent, + nextEvent: EventAndShouldShow | null = null, + nextEventWithTile: MatrixEvent | null = null, ): ReactNode[] { const ret: ReactNode[] = []; @@ -1078,7 +1078,7 @@ abstract class BaseGrouper { public readonly event: EventAndShouldShow, public readonly prevEvent: MatrixEvent | null, public readonly lastShownEvent: MatrixEvent, - public readonly nextEvent?: EventAndShouldShow, + public readonly nextEvent: EventAndShouldShow | null, public readonly nextEventTile?: MatrixEvent, ) { this.readMarker = panel.readMarkerForEvent(event.event.getId(), event.event === lastShownEvent); From 49499d1c656abede23078d2968449bfeda6fa84b Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 17 Mar 2023 15:41:00 +0000 Subject: [PATCH 9/9] Apply review comments --- src/components/structures/MessagePanel.tsx | 79 ++++++++++++---------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 3e5d12ae2c0..650313b9670 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -578,14 +578,14 @@ export default class MessagePanel extends React.Component { private getNextEventInfo( events: EventAndShouldShow[], i: number, - ): { nextEvent: EventAndShouldShow | null; nextTile: MatrixEvent | null } { + ): { nextEventAndShouldShow: EventAndShouldShow | null; nextTile: MatrixEvent | null } { // WARNING: this method is on a hot path. - const nextEvent = i < events.length - 1 ? events[i + 1] : null; + const nextEventAndShouldShow = i < events.length - 1 ? events[i + 1] : null; const nextTile = findFirstShownAfter(i, events); - return { nextEvent, nextTile }; + return { nextEventAndShouldShow, nextTile }; } private get pendingEditItem(): string | undefined { @@ -647,15 +647,15 @@ export default class MessagePanel extends React.Component { let grouper: BaseGrouper | null = null; for (let i = 0; i < events.length; i++) { - const ev = events[i]; - const { event, shouldShow } = ev; + const eventAndShouldShow = events[i]; + const { event, shouldShow } = eventAndShouldShow; const eventId = event.getId(); const last = event === lastShownEvent; - const { nextEvent, nextTile } = this.getNextEventInfo(events, i); + const { nextEventAndShouldShow, nextTile } = this.getNextEventInfo(events, i); if (grouper) { - if (grouper.shouldGroup(ev)) { - grouper.add(ev); + if (grouper.shouldGroup(eventAndShouldShow)) { + grouper.add(eventAndShouldShow); continue; } else { // not part of group, so get the group tiles, close the @@ -667,8 +667,15 @@ export default class MessagePanel extends React.Component { } for (const Grouper of groupers) { - if (Grouper.canStartGroup(this, ev) && !this.props.disableGrouping) { - grouper = new Grouper(this, ev, prevEvent, lastShownEvent, nextEvent, nextTile); + if (Grouper.canStartGroup(this, eventAndShouldShow) && !this.props.disableGrouping) { + grouper = new Grouper( + this, + eventAndShouldShow, + prevEvent, + lastShownEvent, + nextEventAndShouldShow, + nextTile, + ); break; // break on first grouper } } @@ -678,7 +685,7 @@ export default class MessagePanel extends React.Component { // make sure we unpack the array returned by getTilesForEvent, // otherwise React will auto-generate keys, and we will end up // replacing all the DOM elements every time we paginate. - ret.push(...this.getTilesForEvent(prevEvent, event, last, false, nextEvent, nextTile)); + ret.push(...this.getTilesForEvent(prevEvent, event, last, false, nextEventAndShouldShow, nextTile)); prevEvent = event; } @@ -746,7 +753,7 @@ export default class MessagePanel extends React.Component { const readReceipts = this.readReceiptsByEvent[eventId]; let isLastSuccessful = false; - const isSentState = (s: EventStatus): boolean => !s || s === EventStatus.SENT; + const isSentState = (s: EventStatus | null): boolean => !s || s === EventStatus.SENT; const isSent = isSentState(mxEv.getAssociatedStatus()); const hasNextEvent = nextEvent?.shouldShow; if (!hasNextEvent && isSent) { @@ -1066,7 +1073,7 @@ interface EventAndShouldShow { } abstract class BaseGrouper { - public static canStartGroup = (panel: MessagePanel, ev: EventAndShouldShow): boolean => true; + public static canStartGroup = (_panel: MessagePanel, _ev: EventAndShouldShow): boolean => true; public events: MatrixEvent[] = []; // events that we include in the group but then eject out and place above the group. @@ -1075,13 +1082,16 @@ abstract class BaseGrouper { public constructor( public readonly panel: MessagePanel, - public readonly event: EventAndShouldShow, + public readonly firstEventAndShouldShow: EventAndShouldShow, public readonly prevEvent: MatrixEvent | null, - public readonly lastShownEvent: MatrixEvent, + public readonly lastShownEvent: MatrixEvent | undefined, public readonly nextEvent: EventAndShouldShow | null, - public readonly nextEventTile?: MatrixEvent, + public readonly nextEventTile?: MatrixEvent | null, ) { - this.readMarker = panel.readMarkerForEvent(event.event.getId(), event.event === lastShownEvent); + this.readMarker = panel.readMarkerForEvent( + firstEventAndShouldShow.event.getId(), + firstEventAndShouldShow.event === lastShownEvent, + ); } public abstract shouldGroup(ev: EventAndShouldShow): boolean; @@ -1106,26 +1116,23 @@ abstract class BaseGrouper { // Grouping only events sent by the same user that sent the `m.room.create` and only until // the first non-state event, beacon_info event or membership event which is not regarding the sender of the `m.room.create` event class CreationGrouper extends BaseGrouper { - public static canStartGroup = function ( - panel: MessagePanel, - { event: ev, shouldShow }: EventAndShouldShow, - ): boolean { - return ev.getType() === EventType.RoomCreate; + public static canStartGroup = function (_panel: MessagePanel, { event }: EventAndShouldShow): boolean { + return event.getType() === EventType.RoomCreate; }; - public shouldGroup({ event: ev, shouldShow }: EventAndShouldShow): boolean { + public shouldGroup({ event, shouldShow }: EventAndShouldShow): boolean { const panel = this.panel; - const createEvent = this.event; + const createEvent = this.firstEventAndShouldShow.event; if (!shouldShow) { return true; } - if (panel.wantsDateSeparator(this.event.event, ev.getDate())) { + if (panel.wantsDateSeparator(this.firstEventAndShouldShow.event, event.getDate())) { return false; } - const eventType = ev.getType(); + const eventType = event.getType(); if ( eventType === EventType.RoomMember && - (ev.getStateKey() !== createEvent.event.getSender() || ev.getContent()["membership"] !== "join") + (event.getStateKey() !== createEvent.getSender() || event.getContent()["membership"] !== "join") ) { return false; } @@ -1141,7 +1148,7 @@ class CreationGrouper extends BaseGrouper { return false; } - if (ev.isState() && ev.getSender() === createEvent.event.getSender()) { + if (event.isState() && event.getSender() === createEvent.getSender()) { return true; } @@ -1170,7 +1177,7 @@ class CreationGrouper extends BaseGrouper { const panel = this.panel; const ret: ReactNode[] = []; const isGrouped = true; - const createEvent = this.event; + const createEvent = this.firstEventAndShouldShow; const lastShownEvent = this.lastShownEvent; if (panel.wantsDateSeparator(this.prevEvent, createEvent.event.getDate())) { @@ -1238,7 +1245,7 @@ class CreationGrouper extends BaseGrouper { } public getNewPrevEvent(): MatrixEvent { - return this.event.event; + return this.firstEventAndShouldShow.event; } } @@ -1267,14 +1274,14 @@ class MainGrouper extends BaseGrouper { public constructor( public readonly panel: MessagePanel, - public readonly event: EventAndShouldShow, + public readonly firstEventAndShouldShow: EventAndShouldShow, public readonly prevEvent: MatrixEvent | null, - public readonly lastShownEvent: MatrixEvent, - nextEvent: EventAndShouldShow, - nextEventTile: MatrixEvent, + public readonly lastShownEvent: MatrixEvent | undefined, + nextEvent: EventAndShouldShow | null, + nextEventTile: MatrixEvent | null, ) { - super(panel, event, prevEvent, lastShownEvent, nextEvent, nextEventTile); - this.events = [event.event]; + super(panel, firstEventAndShouldShow, prevEvent, lastShownEvent, nextEvent, nextEventTile); + this.events = [firstEventAndShouldShow.event]; } public shouldGroup({ event: ev, shouldShow }: EventAndShouldShow): boolean {