Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hide pre-join UTDs #3881

Merged
merged 16 commits into from Jan 28, 2020
Merged

Hide pre-join UTDs #3881

Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f16090c
initial attempt to hide pre-join UTDs
uhoreg Jan 20, 2020
361add0
lint
uhoreg Jan 20, 2020
3743052
move UTD hiding to message panel
uhoreg Jan 22, 2020
fcfee87
lint
uhoreg Jan 22, 2020
feb36ac
unbreak it
uhoreg Jan 22, 2020
8e0fe2a
use a separate component for warning message
uhoreg Jan 22, 2020
bfca241
lint
uhoreg Jan 22, 2020
e3e89e2
lint and use correct constant
uhoreg Jan 23, 2020
1bebba9
just hide pre-join UTDs, as per feedback from Nad, and other fixes
uhoreg Jan 23, 2020
f1a0430
fix jsdoc and i18n
uhoreg Jan 23, 2020
63a56f9
use the right index for the loop
uhoreg Jan 24, 2020
a41bb69
move logic (back) to TimelinePanel
uhoreg Jan 27, 2020
ee7988e
remove changes from TimelinePanel and update copyright of MessagePanel
uhoreg Jan 27, 2020
380a81b
use liveEvents instead of events
uhoreg Jan 28, 2020
187f1f4
apply changes from PR review
uhoreg Jan 28, 2020
a37df92
Merge branch 'develop' into uhoreg/hide_pre_join_utds
uhoreg Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
94 changes: 88 additions & 6 deletions src/components/structures/TimelinePanel.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@
Copyright 2016 OpenMarket Ltd
Copyright 2017 Vector Creations Ltd
Copyright 2019 New Vector Ltd
Copyright 2019 The Matrix.org Foundation C.I.C.
Copyright 2019-2020 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -146,6 +146,9 @@ const TimelinePanel = createReactClass({
liveEvents: [],
timelineLoading: true, // track whether our room timeline is loading

// the index of the first event that is to be shown
firstVisibleEventIndex: 0,

// canBackPaginate == false may mean:
//
// * we haven't (successfully) loaded the timeline yet, or:
Expand Down Expand Up @@ -333,11 +336,12 @@ const TimelinePanel = createReactClass({

// We can now paginate in the unpaginated direction
const canPaginateKey = (backwards) ? 'canBackPaginate' : 'canForwardPaginate';
const { events, liveEvents } = this._getEvents();
const { events, liveEvents, firstVisibleEventIndex } = this._getEvents();
this.setState({
[canPaginateKey]: true,
events,
liveEvents,
firstVisibleEventIndex,
});
}
},
Expand Down Expand Up @@ -369,6 +373,11 @@ const TimelinePanel = createReactClass({
return Promise.resolve(false);
}

if (backwards && this.state.firstVisibleEventIndex !== 0) {
debuglog("TimelinePanel: won't", dir, "paginate past first visible event");
return Promise.resolve(false);
}

debuglog("TimelinePanel: Initiating paginate; backwards:"+backwards);
this.setState({[paginatingKey]: true});

Expand All @@ -377,12 +386,13 @@ const TimelinePanel = createReactClass({

debuglog("TimelinePanel: paginate complete backwards:"+backwards+"; success:"+r);

const { events, liveEvents } = this._getEvents();
const { events, liveEvents, firstVisibleEventIndex } = this._getEvents();
const newState = {
[paginatingKey]: false,
[canPaginateKey]: r,
events,
liveEvents,
firstVisibleEventIndex,
};

// moving the window in this direction may mean that we can now
Expand All @@ -402,7 +412,11 @@ const TimelinePanel = createReactClass({
// itself into the right place
return new Promise((resolve) => {
this.setState(newState, () => {
resolve(r);
// we can continue paginating in the given direction if:
// - _timelineWindow.paginate says we can
// - we're paginating forwards, or we won't be trying to
// paginate backwards past the first visible event
resolve(r && (!backwards || firstVisibleEventIndex === 0));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment to explain this.

});
});
});
Expand Down Expand Up @@ -476,12 +490,13 @@ const TimelinePanel = createReactClass({
this._timelineWindow.paginate(EventTimeline.FORWARDS, 1, false).then(() => {
if (this.unmounted) { return; }

const { events, liveEvents } = this._getEvents();
const { events, liveEvents, firstVisibleEventIndex } = this._getEvents();
const lastLiveEvent = liveEvents[liveEvents.length - 1];

const updatedState = {
events,
liveEvents,
firstVisibleEventIndex,
};

let callRMUpdated;
Expand Down Expand Up @@ -1115,6 +1130,7 @@ const TimelinePanel = createReactClass({
// get the list of events from the timeline window and the pending event list
_getEvents: function() {
const events = this._timelineWindow.getEvents();
const firstVisibleEventIndex = this._checkForPreJoinUISI(events);

// Hold onto the live events separately. The read receipt and read marker
// should use this list, so that they don't advance into pending events.
Expand All @@ -1128,9 +1144,72 @@ const TimelinePanel = createReactClass({
return {
events,
liveEvents,
firstVisibleEventIndex,
};
},

/**
* Check for undecryptable messages that were sent while the user was not in
* the room.
*
* @param {Array<MatrixEvent>} events The timeline events to check
*
* @return {Number} The index within `events` of the event after the most recent
* undecryptable event that was sent while the user was not in the room. If no
* such events were found, then it returns 0.
*/
_checkForPreJoinUISI: function(events) {
const room = this.props.timelineSet.room;

if (events.length === 0 || !room ||
!MatrixClientPeg.get().isRoomEncrypted(room.roomId)) {
return 0;
}

const userId = MatrixClientPeg.get().credentials.userId;

// get the user's membership at the last event by getting the timeline
// that the event belongs to, and traversing the timeline looking for
// that event, while keeping track of the user's membership
const lastEvent = events[events.length - 1];
const timeline = room.getTimelineForEvent(lastEvent.getId());
const userMembershipEvent =
timeline.getState(EventTimeline.FORWARDS).getMember(userId);
let userMembership = userMembershipEvent
? userMembershipEvent.membership : "leave";
const timelineEvents = timeline.getEvents();
for (let i = timelineEvents.length - 1; i >= 0; i--) {
const event = timelineEvents[i];
if (event.getId() === lastEvent.getId()) {
// found the last event, so we can stop looking through the timeline
break;
} else if (event.getStateKey() === userId
&& event.getType() === "m.room.member") {
const prevContent = event.getPrevContent();
userMembership = prevContent.membership || "leave";
}
}

// now go through the events that we have and find the first undecryptable
// one that was sent when the user wasn't in the room
for (let i = events.length - 1; i >= 0; i--) {
const event = events[i];
if (event.getStateKey() === userId
&& event.getType() === "m.room.member") {
const prevContent = event.getPrevContent();
userMembership = prevContent.membership || "leave";
} else if (userMembership === "leave" &&
(event.isDecryptionFailure() || event.isBeingDecrypted())) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One result of moving this to TimelinePanel is that I had to make it also hide things that are in the process of being decrypted (which is probably a wrong thing to do), rather than just things that failed to decrypt, presumably because it hasn't had enough time to finish trying to decrypt. It would be nice if there was a better way to do this, but I can't see anything better.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure I follow why that's only an issue up here in TimelinePanel, I would have assumed it would apply in either component...

What is the user experience with and without including events being decrypted here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why it only happens when it's in TimelinePanel and not in MessagePanel. My best guess is that it's a timing issue -- by the time MessagePanel gets called, the message decryption has completed, whereas in TimelinePanel, it's still trying to decrypt.

If I don't include isBeingDecrypted, then it will just show all the events, without dropping the pre-join UISIs.

// reached an undecryptable message when the user wasn't in
// the room -- don't try to load any more
// Note: for now, we assume that events that are being decrypted are
// not decryptable
return i + 1;
}
}
return 0;
},

_indexForEventId: function(evId) {
for (let i = 0; i < this.state.events.length; ++i) {
if (evId == this.state.events[i].getId()) {
Expand Down Expand Up @@ -1323,6 +1402,9 @@ const TimelinePanel = createReactClass({
this.state.forwardPaginating ||
['PREPARED', 'CATCHUP'].includes(this.state.clientSyncState)
);
const events = this.state.firstVisibleEventIndex
? this.state.events.slice(this.state.firstVisibleEventIndex)
: this.state.events;
return (
<MessagePanel
ref={this._messagePanel}
Expand All @@ -1331,7 +1413,7 @@ const TimelinePanel = createReactClass({
hidden={this.props.hidden}
backPaginating={this.state.backPaginating}
forwardPaginating={forwardPaginating}
events={this.state.events}
events={events}
highlightedEventId={this.props.highlightedEventId}
readMarkerEventId={this.state.readMarkerEventId}
readMarkerVisible={this.state.readMarkerVisible}
Expand Down