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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the primary change in this PR: the new beta (which has been untouched for a year as of writing) actually does a better job of handling concurrent read receipts, this patching holes. The beta doesn't have the same leak as v1, so we can remove the metadata hack from our side (it doesn't use jQuery's data anymore). Note that this change on its own introduces an annoying bug where every second update to a read receipt will throw it 14px to the right - more on that in the next commit.
As mentioned in 208faf6, the velocity-animate update causes read receipts to occasionally show up 14px to the right of where they should be. This is because the read receipt width is 14px, and velocity-animate will *not* translate `left` if it isn't changing. Unfortunately, it's smart enough to realize that `-0px` is `0px`, so we end up having to specify `1px`. The comment already mentions it, but this should have no perceived effect for the user. During development I could not tell if the 1px was being applied during the animation, implying that it's a meaningless value. It's a bit unfortunate for those who know that it's translating left by 1px, but hopefully they'll be able to unsee that in time.
We can make read receipts more efficient (and avoid double-animation) by using `PureComponent` which no-ops useless updates for us.
This was referenced Feb 27, 2021
This appears to have been removed in the beta
ara4n
reviewed
Feb 27, 2021
1 task
jryans
approved these changes
Mar 2, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes element-hq/element-web#9769
Reviewer: this is split into a few commits for ease of review. There are lengthy explanations on the important commits - I highly encourage reviewing commit by commit.