From 026aa6f88d76bc404b0b2789b9599bbf0af59f59 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Mar 2021 21:39:07 -0600 Subject: [PATCH 1/3] Track next event [tile] over group boundaries Fixes https://github.com/vector-im/element-web/issues/16745 --- src/components/structures/MessagePanel.js | 38 +++++++++++++++-------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 0a9af413d8a..6951aff3570 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -452,6 +452,20 @@ export default class MessagePanel extends React.Component { }); }; + _getNextEventInfo(arr, i) { + const nextEvent = i < arr.length - 1 + ? arr[i + 1] + : null; + + // The next event with tile is used to to determine the 'last successful' flag + // when rendering the tile. The shouldShowEvent function is pretty quick at what + // it does, so this should have no significant cost even when a room is used for + // not-chat purposes. + const nextTile = arr.slice(i + 1).find(e => this._shouldShowEvent(e)); + + return {nextEvent, nextTile}; + } + _getEventTiles() { this.eventNodes = {}; @@ -503,6 +517,7 @@ export default class MessagePanel extends React.Component { const mxEv = this.props.events[i]; const eventId = mxEv.getId(); const last = (mxEv === lastShownEvent); + const {nextEvent, nextTile} = this._getNextEventInfo(this.props.events, i); if (grouper) { if (grouper.shouldGroup(mxEv)) { @@ -519,21 +534,13 @@ export default class MessagePanel extends React.Component { for (const Grouper of groupers) { if (Grouper.canStartGroup(this, mxEv)) { - grouper = new Grouper(this, mxEv, prevEvent, lastShownEvent); + grouper = new Grouper(this, mxEv, prevEvent, lastShownEvent, nextEvent, nextTile); } } if (!grouper) { const wantTile = this._shouldShowEvent(mxEv); if (wantTile) { - const nextEvent = i < this.props.events.length - 1 - ? this.props.events[i + 1] - : null; - - // The next event with tile is used to to determine the 'last successful' flag - // when rendering the tile. The shouldShowEvent function is pretty quick at what - // it does, so this should have no significant cost even when a room is used for - // not-chat purposes. - const nextTile = this.props.events.slice(i + 1).find(e => this._shouldShowEvent(e)); + const {nextEvent, nextTile} = this._getNextEventInfo(this.props.events, i); // make sure we unpack the array returned by _getTilesForEvent, // otherwise react will auto-generate keys and we will end up @@ -982,7 +989,7 @@ class CreationGrouper { )); } - const eventTiles = this.events.map((e) => { + const eventTiles = this.events.map((e, i) => { // In order to prevent DateSeparators from appearing in the expanded form // of EventListSummary, render each member event as if the previous // one was itself. This way, the timestamp of the previous event === the @@ -1032,7 +1039,7 @@ class RedactionGrouper { return panel._shouldShowEvent(ev) && ev.isRedacted(); } - constructor(panel, ev, prevEvent, lastShownEvent) { + constructor(panel, ev, prevEvent, lastShownEvent, nextEvent, nextEventTile) { this.panel = panel; this.readMarker = panel._readMarkerForEvent( ev.getId(), @@ -1041,6 +1048,8 @@ class RedactionGrouper { this.events = [ev]; this.prevEvent = prevEvent; this.lastShownEvent = lastShownEvent; + this.nextEvent = nextEvent; + this.nextEventTile = nextEventTile; } shouldGroup(ev) { @@ -1089,7 +1098,10 @@ class RedactionGrouper { const senders = new Set(); let eventTiles = this.events.map((e, i) => { senders.add(e.sender); - return panel._getTilesForEvent(i === 0 ? this.prevEvent : this.events[i - 1], e, e === lastShownEvent); + return panel._getTilesForEvent( + i === 0 ? this.prevEvent : this.events[i - 1], + e, e === lastShownEvent, + this.nextEvent, this.nextEventTile); }).reduce((a, b) => a.concat(b), []); if (eventTiles.length === 0) { From fa54ca615a48cda4ff64357bb12b87b6b6871cce Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Mar 2021 21:41:13 -0600 Subject: [PATCH 2/3] Appease the linter --- src/components/structures/MessagePanel.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 6951aff3570..671d895a21b 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -989,7 +989,7 @@ class CreationGrouper { )); } - const eventTiles = this.events.map((e, i) => { + const eventTiles = this.events.map((e) => { // In order to prevent DateSeparators from appearing in the expanded form // of EventListSummary, render each member event as if the previous // one was itself. This way, the timestamp of the previous event === the @@ -1098,10 +1098,8 @@ class RedactionGrouper { const senders = new Set(); let eventTiles = this.events.map((e, i) => { senders.add(e.sender); - return panel._getTilesForEvent( - i === 0 ? this.prevEvent : this.events[i - 1], - e, e === lastShownEvent, - this.nextEvent, this.nextEventTile); + const prevEvent = i === 0 ? this.prevEvent : this.events[i - 1]; + return panel._getTilesForEvent(prevEvent, e, e === lastShownEvent, this.nextEvent, this.nextEventTile); }).reduce((a, b) => a.concat(b), []); if (eventTiles.length === 0) { From 2f2bb9456ff2757e752fa3f8910706b8ebfac14c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Mar 2021 18:17:41 -0600 Subject: [PATCH 3/3] Reduce code duplication --- src/components/structures/MessagePanel.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 671d895a21b..6d03c849c46 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -540,8 +540,6 @@ export default class MessagePanel extends React.Component { if (!grouper) { const wantTile = this._shouldShowEvent(mxEv); if (wantTile) { - const {nextEvent, nextTile} = this._getNextEventInfo(this.props.events, i); - // 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.