From 3736fcf80e87af76f70910aa6d8948f99ae12aff Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 22 Mar 2016 16:34:12 +0000 Subject: [PATCH] Move read-marker past our own events when we switch to a room This fixes an issue where the RM appeared before any events which were pending when you switched away from that room (https://github.com/vector-im/vector-web/issues/1241). Also, fix a buglet in the MessagePanel which meant we didn't animate the disappearance of a RM when it stayed at the same event but became invisible. This didn't really cause any user-visible problems (because typically we advance the RM at the same time as it became invisible), but confused me a bit while I was trying to debug this. --- src/components/structures/MessagePanel.js | 41 +++++++++++++++++----- src/components/structures/TimelinePanel.js | 39 ++++++++++++++++++-- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 972e9cf7f1..66e2daddd5 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -173,15 +173,29 @@ module.exports = React.createClass({ // first figure out which is the last event in the list which we're // actually going to show; this allows us to behave slightly // differently for the last event in the list. + // + // we also need to figure out which is the last event we show which isn't + // a local echo, to manage the read-marker. + var lastShownEventIndex = -1; + var lastShownNonLocalEchoIndex = -1; for (i = this.props.events.length-1; i >= 0; i--) { var mxEv = this.props.events[i]; if (!EventTile.haveTileForEvent(mxEv)) { continue; } + if (lastShownEventIndex < 0) { + lastShownEventIndex = i; + } + + if (mxEv.status) { + // this is a local echo + continue; + } + + lastShownNonLocalEchoIndex = i; break; } - var lastShownEventIndex = i; var ret = []; @@ -213,25 +227,34 @@ module.exports = React.createClass({ ret.push(
  • ); } + var isVisibleReadMarker = false; + if (eventId == this.props.readMarkerEventId) { var visible = this.props.readMarkerVisible; - // if the read marker comes at the end of the timeline, we don't want - // to show it, but we still want to create the
  • for it so that the - // algorithms which depend on its position on the screen aren't confused. - if (i >= lastShownEventIndex) { + // if the read marker comes at the end of the timeline (except + // for local echoes, which are excluded from RMs, because they + // don't have useful event ids), we don't want to show it, but + // we still want to create the
  • for it so that the + // algorithms which depend on its position on the screen aren't + // confused. + if (i >= lastShownNonLocalEchoIndex) { visible = false; } ret.push(this._getReadMarkerTile(visible)); readMarkerVisible = visible; - } else if (eventId == this.currentReadMarkerEventId && !this.currentGhostEventId) { + isVisibleReadMarker = visible; + } + + if (eventId == this.currentGhostEventId) { + // if we're showing an animation, continue to show it. + ret.push(this._getReadMarkerGhostTile()); + } else if (!isVisibleReadMarker && + eventId == this.currentReadMarkerEventId) { // there is currently a read-up-to marker at this point, but no // more. Show an animation of it disappearing. ret.push(this._getReadMarkerGhostTile()); this.currentGhostEventId = eventId; - } else if (eventId == this.currentGhostEventId) { - // if we're showing an animation, continue to show it. - ret.push(this._getReadMarkerGhostTile()); } } diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 7cb8a22e7c..cc4cf98f7e 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -360,8 +360,6 @@ var TimelinePanel = React.createClass({ return; } - var currentIndex = this._indexForEventId(this.state.readMarkerEventId); - // move the RM to *after* the message at the bottom of the screen. This // avoids a problem whereby we never advance the RM if there is a huge // message which doesn't fit on the screen. @@ -392,6 +390,38 @@ var TimelinePanel = React.createClass({ } }, + + // advance the read marker past any events we sent ourselves. + _advanceReadMarkerPastMyEvents: function() { + // we call _timelineWindow.getEvents() rather than using + // this.state.events, because react batches the update to the latter, so it + // may not have been updated yet. + var events = this._timelineWindow.getEvents(); + + // first find where the current RM is + for (var i = 0; i < events.length; i++) { + if (events[i].getId() == this.state.readMarkerEventId) + break; + } + if (i >= events.length) { + return; + } + + // now think about advancing it + var myUserId = MatrixClientPeg.get().credentials.userId; + for (; i < events.length; i++) { + var ev = events[i]; + if (!ev.sender || ev.sender.userId != myUserId) { + break; + } + } + // i is now the first unread message which we didn't send ourselves. + i--; + + var ev = events[i]; + this._setReadMarker(ev.getId(), ev.getTs()); + }, + /* jump down to the bottom of this room, where new events are arriving */ jumpToLiveTimeline: function() { @@ -544,6 +574,11 @@ var TimelinePanel = React.createClass({ var onLoaded = () => { this._reloadEvents(); + // If we switched away from the room while there were pending + // outgoing events, the read-marker will be before those events. + // We need to skip over any which have subsequently been sent. + this._advanceReadMarkerPastMyEvents(); + this.setState({timelineLoading: false}, () => { // initialise the scroll state of the message panel if (!this.refs.messagePanel) {