Merge pull request #254 from matrix-org/rav/fix_rm_animation

Fix a bug where we tried to show two ghost read markers at once.

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() {
+    _startAnimation: function(ghostNode) {
-        // reset the ghostEventId when the animation finishes, so that
+        Velocity(ghostNode, {opacity: '0', width: '10%'},
-        // we can make a new one (and so that we don't run the
+                 {duration: 400, easing: 'easeInSine',
-        // animation code every time we render)
+                  delay: 1000});
-        var completeFunc = () => {
+    },
-            this.currentGhostEventId = null;
-        };
 
+    _getReadMarkerGhostTile: function() {
         var hr = <hr className="mx_RoomView_myReadMarker"
                   style={{opacity: 1, width: '99%'}}
-                  ref={function(n) {
+                  ref={this._startAnimation}
-                        Velocity(n, {opacity: '0', width: '10%'},
-                                    {duration: 400, easing: 'easeInSine',
-                                     delay: 1000, complete: completeFunc});
-                  }}
             />;
 
         // give it a key which depends on the event id. That will ensure that

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);
+    });
 });