Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix tetris effect (holes) in read receipts #5697

Merged
merged 5 commits into from
Mar 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
23 changes: 4 additions & 19 deletions src/Velociraptor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 10 additions & 2 deletions src/components/views/rooms/ReadReceiptMarker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down
9 changes: 0 additions & 9 deletions test/components/structures/MessagePanel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
});

Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down