PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Ensure EventListSummary key does not change during backpagination (#7915)

Browse source
This commit is contained in:
Michael Telatynski 2022-03-01 08:33:07 +00:00 committed by GitHub
parent 115e17b097
commit 482d756bd0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 105 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
src/components/structures/MessagePanel.tsx
View file

@ -254,6 +254,9 @@ export default class MessagePanel extends React.Component<IProps, IState> {
private readonly showTypingNotificationsWatcherRef: string;
private eventTiles: Record<string, EventTile> = {};
// A map to allow groupers to maintain consistent keys even if their first event is uprooted due to back-pagination.
public grouperKeyMap = new WeakMap<MatrixEvent, string>();
constructor(props, context) {
super(props, context);
@ -684,14 +687,13 @@ export default class MessagePanel extends React.Component<IProps, IState> {
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 +1129,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 +1137,7 @@ class CreationGrouper extends BaseGrouper {
if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) {
const ts = createEvent.getTs();
ret.push(
<li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={createEvent.getRoomId()} ts={ts} /></li>,
<li key={ts+'~'}><DateSeparator roomId={createEvent.getRoomId()} ts={ts} /></li>,
);
}
@ -1262,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
@ -1271,24 +1277,28 @@ 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(
<li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={this.events[0].getRoomId()} ts={ts} /></li>,
<li key={ts+'~'}><DateSeparator roomId={this.events[0].getRoomId()} ts={ts} /></li>,
);
}
// 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) => {

76
test/components/structures/MessagePanel-test.js
View file

@ -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(<WrappedMessagePanel events={events} />);
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(<WrappedMessagePanel events={events} />);
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);
});
});