From b6e4e9caf36c0abbb7a1fc4c6a7b2a76c5b3c914 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Thu, 31 Mar 2016 18:19:57 +0100
Subject: [PATCH] Fix a bug where we tried to show two ghost read markers at
 once.

I think this was responsible for the readmarker just disappearing rather than
animating.

While we're here, decrease the delay on the animation, and instead make it take
slightly longer to disappear.
---
 src/components/structures/MessagePanel.js     | 26 ++++++-----
 .../structures/MessagePanel-test.js           | 46 +++++++++++++++++++
 2 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js
index aaa00934cb..0e00b0dc7d 100644
--- a/src/components/structures/MessagePanel.js
+++ b/src/components/structures/MessagePanel.js
@@ -205,6 +205,13 @@ module.exports = React.createClass({
         // assume there is no read marker until proven otherwise
         var readMarkerVisible = false;
 
+        // if the readmarker has moved, cancel any active ghost.
+        if (this.currentReadMarkerEventId && this.props.readMarkerEventId &&
+                this.props.readMarkerVisible &&
+                this.currentReadMarkerEventId != this.props.readMarkerEventId) {
+            this.currentGhostEventId = null;
+        }
+
         for (i = 0; i < this.props.events.length; i++) {
             var mxEv = this.props.events[i];
             var wantTile = true;
@@ -336,21 +343,16 @@ module.exports = React.createClass({
         );
     },
 
-    _getReadMarkerGhostTile: function() {
-        // reset the ghostEventId when the animation finishes, so that
-        // we can make a new one (and so that we don't run the
-        // animation code every time we render)
-        var completeFunc = () => {
-            this.currentGhostEventId = null;
-        };
+    _startAnimation: function(ghostNode) {
+        Velocity(ghostNode, {opacity: '0', width: '10%'},
+                 {duration: 600, easing: 'easeInSine',
+                  delay: 200});
+    },
 
+    _getReadMarkerGhostTile: function() {
         var hr = <hr className="mx_RoomView_myReadMarker"
                   style={{opacity: 1, width: '99%'}}
-                  ref={function(n) {
-                        Velocity(n, {opacity: '0', width: '10%'},
-                                    {duration: 400, easing: 'easeInSine',
-                                     delay: 1000, complete: completeFunc});
-                  }}
+                  ref={this._startAnimation}
             />;
 
         // give it a key which depends on the event id. That will ensure that
diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js
index f775388617..9a4996b5da 100644
--- a/test/components/structures/MessagePanel-test.js
+++ b/test/components/structures/MessagePanel-test.js
@@ -130,4 +130,50 @@ describe('MessagePanel', function () {
             }, 100);
         }, 100);
     });
+
+    it('shows only one ghost when the RM moves twice', function() {
+        var parentDiv = document.createElement('div');
+
+        // first render with the RM in one place
+        var mp = ReactDOM.render(
+                <MessagePanel events={events} readMarkerEventId={events[4].getId()}
+                    readMarkerVisible={true}
+                />, parentDiv);
+
+        var tiles = TestUtils.scryRenderedComponentsWithType(
+            mp, sdk.getComponent('rooms.EventTile'));
+        var tileContainers = tiles.map(function (t) {
+            return ReactDOM.findDOMNode(t).parentNode;
+        });
+
+        // now move the RM
+        mp = ReactDOM.render(
+                <MessagePanel events={events} readMarkerEventId={events[6].getId()}
+                    readMarkerVisible={true}
+                />, parentDiv);
+
+        // now there should be two RM containers
+        var found = TestUtils.scryRenderedDOMComponentsWithClass(mp, 'mx_RoomView_myReadMarker_container');
+        expect(found.length).toEqual(2);
+
+        // the first should be the ghost
+        expect(tileContainers.indexOf(found[0].previousSibling)).toEqual(4);
+
+        // the second should be the real RM
+        expect(tileContainers.indexOf(found[1].previousSibling)).toEqual(6);
+
+        // and move the RM again
+        mp = ReactDOM.render(
+                <MessagePanel events={events} readMarkerEventId={events[8].getId()}
+                    readMarkerVisible={true}
+                />, parentDiv);
+
+        // still two RM containers
+        found = TestUtils.scryRenderedDOMComponentsWithClass(mp, 'mx_RoomView_myReadMarker_container');
+        expect(found.length).toEqual(2);
+
+        // they should have moved
+        expect(tileContainers.indexOf(found[0].previousSibling)).toEqual(6);
+        expect(tileContainers.indexOf(found[1].previousSibling)).toEqual(8);
+    });
 });