From fe6528b53c29144d64639758e9741cf14612f1b8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 28 Feb 2022 15:30:35 +0000 Subject: [PATCH 1/3] Tidy --- src/components/structures/MessagePanel.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 7407ff220f0..0269cb783ef 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -684,14 +684,13 @@ export default class MessagePanel extends React.Component { break; // break on first grouper } } + if (!grouper) { - const wantTile = this.shouldShowEvent(mxEv); - const isGrouped = false; - if (wantTile) { + if (this.shouldShowEvent(mxEv)) { // make sure we unpack the array returned by getTilesForEvent, - // otherwise react will auto-generate keys and we will end up - // replacing all of the DOM elements every time we paginate. - ret.push(...this.getTilesForEvent(prevEvent, mxEv, last, isGrouped, nextEvent, nextTile)); + // 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; } @@ -1127,7 +1126,7 @@ class CreationGrouper extends BaseGrouper { if (!this.events || !this.events.length) return []; const panel = this.panel; - const ret = []; + const ret: ReactNode[] = []; const isGrouped = true; const createEvent = this.event; const lastShownEvent = this.lastShownEvent; @@ -1135,7 +1134,7 @@ class CreationGrouper extends BaseGrouper { if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) { const ts = createEvent.getTs(); ret.push( -
    • , +
    • , ); } @@ -1271,12 +1270,12 @@ class MainGrouper extends BaseGrouper { const isGrouped = true; const panel = this.panel; const lastShownEvent = this.lastShownEvent; - const ret = []; + const ret: ReactNode[] = []; if (panel.wantsDateSeparator(this.prevEvent, this.events[0].getDate())) { const ts = this.events[0].getTs(); ret.push( -
    • , +
    • , ); } From 7233a6e90d62044ad2bfd8af17ce44ec4d8bb1ad Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 28 Feb 2022 15:47:15 +0000 Subject: [PATCH 2/3] Ensure EventListSummary key does not change during backpagination --- src/components/structures/MessagePanel.tsx | 29 +++++++++++++++------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 0269cb783ef..98057e32cd7 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -254,6 +254,9 @@ export default class MessagePanel extends React.Component { private readonly showTypingNotificationsWatcherRef: string; private eventTiles: Record = {}; + // A map to allow groupers to maintain consistent keys even if their first event is uprooted due to back-pagination. + public grouperKeyMap = new WeakMap(); + constructor(props, context) { super(props, context); @@ -1261,6 +1264,10 @@ class MainGrouper extends BaseGrouper { this.events.push(ev); } + private generateKey(): string { + return "eventlistsummary-" + this.events[0].getId(); + } + public getTiles(): ReactNode[] { // If we don't have any events to group, don't even try to group them. The logic // below assumes that we have a group of events to deal with, but we might not if @@ -1279,15 +1286,19 @@ class MainGrouper extends BaseGrouper { ); } - // Ensure that the key of the EventListSummary does not change with new events. - // This will prevent it from being re-created unnecessarily, and - // instead will allow new props to be provided. In turn, the shouldComponentUpdate - // method on ELS can be used to prevent unnecessary renderings. - // - // Whilst back-paginating with an ELS at the top of the panel, prevEvent will be null, - // so use the key "eventlistsummary-initial". Otherwise, use the ID of the first - // membership event, which will not change during forward pagination. - const key = "eventlistsummary-" + (this.prevEvent ? this.events[0].getId() : "initial"); + // Ensure that the key of the EventListSummary does not change with new events in either direction. + // This will prevent it from being re-created unnecessarily, and instead will allow new props to be provided. + // In turn, the shouldComponentUpdate method on ELS can be used to prevent unnecessary renderings. + const keyEvent = this.events.find(e => this.panel.grouperKeyMap.get(e)); + const key = keyEvent ? this.panel.grouperKeyMap.get(keyEvent) : this.generateKey(); + if (!keyEvent) { + // Populate the weak map with the key that we are using for all related events. + this.events.forEach(e => { + if (!this.panel.grouperKeyMap.has(e)) { + this.panel.grouperKeyMap.set(e, key); + } + }); + } let highlightInSummary = false; let eventTiles = this.events.map((e, i) => { From 48dc90b5bb4bf5966a7fc797a8c5b6766967e59a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 28 Feb 2022 16:11:22 +0000 Subject: [PATCH 3/3] Add tests --- .../structures/MessagePanel-test.js | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index c0ca398ce19..baaf262ace5 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -496,4 +496,80 @@ describe('MessagePanel', function() { expect(Dates.length).toEqual(1); }); + + it('appends events into summaries during forward pagination without changing key', () => { + const events = mkMelsEvents().slice(1, 11); + + const res = mount(); + let els = res.find("EventListSummary"); + expect(els.length).toEqual(1); + expect(els.key()).toEqual("eventlistsummary-" + events[0].getId()); + expect(els.prop("events").length).toEqual(10); + + res.setProps({ + events: [ + ...events, + TestUtilsMatrix.mkMembership({ + event: true, + room: "!room:id", + user: "@user:id", + target: { + userId: "@user:id", + name: "Bob", + getAvatarUrl: () => { + return "avatar.jpeg"; + }, + getMxcAvatarUrl: () => 'mxc://avatar.url/image.png', + }, + ts: Date.now(), + mship: 'join', + prevMship: 'join', + name: 'A user', + }), + ], + }); + + els = res.find("EventListSummary"); + expect(els.length).toEqual(1); + expect(els.key()).toEqual("eventlistsummary-" + events[0].getId()); + expect(els.prop("events").length).toEqual(11); + }); + + it('prepends events into summaries during backward pagination without changing key', () => { + const events = mkMelsEvents().slice(1, 11); + + const res = mount(); + let els = res.find("EventListSummary"); + expect(els.length).toEqual(1); + expect(els.key()).toEqual("eventlistsummary-" + events[0].getId()); + expect(els.prop("events").length).toEqual(10); + + res.setProps({ + events: [ + TestUtilsMatrix.mkMembership({ + event: true, + room: "!room:id", + user: "@user:id", + target: { + userId: "@user:id", + name: "Bob", + getAvatarUrl: () => { + return "avatar.jpeg"; + }, + getMxcAvatarUrl: () => 'mxc://avatar.url/image.png', + }, + ts: Date.now(), + mship: 'join', + prevMship: 'join', + name: 'A user', + }), + ...events, + ], + }); + + els = res.find("EventListSummary"); + expect(els.length).toEqual(1); + expect(els.key()).toEqual("eventlistsummary-" + events[0].getId()); + expect(els.prop("events").length).toEqual(11); + }); });