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

Change read markers to use CSS transitions #3674

Merged
merged 1 commit into from
Nov 27, 2019

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Nov 26, 2019

Removes one of the two places we use Velocity, so we're one step
closer to getting rid of it for good.

Should therefore fix the fact that Velocity is leaking data entries
and therefore <hr> elements.

Hopefully also makes the logic in getEventTiles incrementally simpler,
if still somwewhat byzantine.

Requires element-hq/element-web#11521

Removes one of the two places we use Velocity, so we're one step
closer to getting rid of it for good.

Should therefore fix the fact that Velocity is leaking data entries
and therefore <hr> elements.

Hopefully also makes the logic in getEventTiles incrementally simpler,
if still somwewhat byzantine.
dbkr added a commit to element-hq/element-web that referenced this pull request Nov 26, 2019
So we can use names like easeInSine in CSS transitions rather than
cubic-bezier(0.47, 0, 0.745, 0.715)

Required for matrix-org/matrix-react-sdk#3674
@dbkr dbkr requested a review from a team November 26, 2019 19:14
@turt2live
Copy link
Member

turt2live commented Nov 26, 2019

Fixes element-hq/element-web#11496 ? Edit: nope, this is read markers, not receipts

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks right, but there seems to be a lot of hidden traps in read markers

@dbkr
Copy link
Member Author

dbkr commented Nov 26, 2019

Fixes vector-im/riot-web#11496 ?

Sadly not - read markers == the green lines, read receipts == the little faces

Yeah, they are quite edge-casey, especially around the MELS stuff: adding some tests made me a bit more confident about doing this.

@@ -191,50 +263,4 @@ describe('MessagePanel', function() {
}, 100);
}, 100);
});

it('shows only one ghost when the RM moves twice', function() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should say: this test was really testing that the animations didn't break (#254). The animations will now work fine however many ghosts we end up showing so this test is no longer useful as it shows multiple ghosts by design.

@dbkr dbkr merged commit dd7b08e into develop Nov 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants