Merge pull request #5697 from matrix-org/travis/fix-tetris

Fix tetris effect (holes) in read receipts

package.json

@@ -101,7 +101,7 @@
     "tar-js": "^0.3.0",
     "text-encoding-utf-8": "^1.0.2",
     "url": "^0.11.0",
-    "velocity-animate": "^1.5.2",
+    "velocity-animate": "^2.0.6",
     "what-input": "^5.2.10",
     "zxcvbn": "^4.4.2"
   },

src/Velociraptor.js

@@ -118,25 +118,10 @@ export default class Velociraptor extends React.Component {
                     domNode.style.visibility = restingStyle.visibility;
                 });
 
-            /*
-            console.log("enter:",
-                        JSON.stringify(transitionOpts[i-1]),
-                        "->",
-                        JSON.stringify(restingStyle));
-            */
-        } else if (node === null) {
-            // Velocity stores data on elements using the jQuery .data()
-            // method, and assumes you'll be using jQuery's .remove() to
-            // remove the element, but we don't use jQuery, so we need to
-            // blow away the element's data explicitly otherwise it will leak.
-            // This uses Velocity's internal jQuery compatible wrapper.
-            // See the bug at
-            // https://github.com/julianshapiro/velocity/issues/300
-            // and the FAQ entry, "Preventing memory leaks when
-            // creating/destroying large numbers of elements"
-            // (https://github.com/julianshapiro/velocity/issues/47)
-            const domNode = ReactDom.findDOMNode(this.nodes[k]);
-            if (domNode) Velocity.Utilities.removeData(domNode);
+            // console.log("enter:",
+            //             JSON.stringify(transitionOpts[i-1]),
+            //             "->",
+            //             JSON.stringify(restingStyle));
         }
         this.nodes[k] = node;
     }

src/components/views/rooms/ReadReceiptMarker.js

@@ -32,7 +32,7 @@ try {
 } catch (e) {
 }
 
-export default class ReadReceiptMarker extends React.Component {
+export default class ReadReceiptMarker extends React.PureComponent {
     static propTypes = {
         // the RoomMember to show the RR for
         member: PropTypes.object,
@@ -155,7 +155,15 @@ export default class ReadReceiptMarker extends React.Component {
 
         // then shift to the rightmost column,
         // and then it will drop down to its resting position
-        startStyles.push({ top: startTopOffset+'px', left: '0px' });
+        //
+        // XXX: We use a fractional left value to trick velocity-animate into actually animating.
+        // This is a very annoying bug where if it thinks there's no change to `left` then it'll
+        // skip applying it, thus making our read receipt at +14px instead of +0px like it
+        // should be. This does cause a tiny amount of drift for read receipts, however with a
+        // value so small it's not perceived by a user.
+        // Note: Any smaller values (or trying to interchange units) might cause read receipts to
+        // fail to fall down or cause gaps.
+        startStyles.push({ top: startTopOffset+'px', left: '0.001px' });
         enterTransitionOpts.push({
             duration: bounce ? Math.min(Math.log(Math.abs(startTopOffset)) * 200, 3000) : 300,
             easing: bounce ? 'easeOutBounce' : 'easeOutCubic',

test/components/structures/MessagePanel-test.js

@@ -35,7 +35,6 @@ const mockclock = require('../../mock-clock');
 import Adapter from "enzyme-adapter-react-16";
 import { configure, mount } from "enzyme";
 
-import Velocity from 'velocity-animate';
 import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
 import RoomContext from "../../../src/contexts/RoomContext";
 import DMRoomMap from "../../../src/utils/DMRoomMap";
@@ -75,18 +74,10 @@ describe('MessagePanel', function() {
             return arg === "showDisplaynameChanges";
         });
 
-        // This option clobbers the duration of all animations to be 1ms
-        // which makes unit testing a lot simpler (the animation doesn't
-        // complete without this even if we mock the clock and tick it
-        // what should be the correct amount of time).
-        Velocity.mock = true;
-
         DMRoomMap.makeShared();
     });
 
     afterEach(function() {
-        delete Velocity.mock;
-
         clock.uninstall();
     });
 

yarn.lock

@@ -8114,10 +8114,10 @@ validate-npm-package-license@^3.0.1:
     spdx-correct "^3.0.0"
     spdx-expression-parse "^3.0.0"
 
-velocity-animate@^1.5.2:
-  version "1.5.2"
-  resolved "https://registry.yarnpkg.com/velocity-animate/-/velocity-animate-1.5.2.tgz#5a351d75fca2a92756f5c3867548b873f6c32105"
-  integrity sha512-m6EXlCAMetKztO1ppBhGU1/1MR3IiEevO6ESq6rcrSQ3Q77xYSW13jkfXW88o4xMrkXJhy/U7j4wFR/twMB0Eg==
+velocity-animate@^2.0.6:
+  version "2.0.6"
+  resolved "https://registry.yarnpkg.com/velocity-animate/-/velocity-animate-2.0.6.tgz#1811ca14df7fbbef05740256f6cec0fd1b76575f"
+  integrity sha512-tU+/UtSo3GkIjEfk2KM4e24DvpgX0+FzfLr7XqNwm9BCvZUtbCHPq/AFutx/Mkp2bXlUS9EcX8yxu8XmzAv2Kw==
 
 [email protected]:
   version "1.10.0"