Merge pull request #244 from matrix-org/rav/no_rm_on_my_events

Move read-marker past our own events when we switch to a room

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(<li key={eventId} data-scroll-token={eventId}/>);
             }
 
+            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
+                // if the read marker comes at the end of the timeline (except
-                // to show it, but we still want to create the <li/> for it so that the
+                // for local echoes, which are excluded from RMs, because they
-                // algorithms which depend on its position on the screen aren't confused.
+                // don't have useful event ids), we don't want to show it, but
-                if (i >= lastShownEventIndex) {
+                // we still want to create the <li/> 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());
             }
         }
 

src/components/structures/TimelinePanel.js

@@ -343,8 +343,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.
@@ -375,6 +373,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() {
@@ -527,6 +557,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) {