Add smooth animation for new messages - #7688
Conversation
|
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
PR Checks
|
2c590ba to
7283530
Compare
|
This is an improvement from what we have, at the cost of making the timeline logic a bit more complex and presumably less performant - extracting read receipts when the timeline items change, I don't think it remembers anything to prevent new computations? Also, it still has the issue about expanding a group of events overlapping with the items above it. It's not terrible, but it does look weird: timeline.movI imagine there can be other issues like this, where components suddenly changing sizes can cause overlaps, but I couldn't find any, so maybe it's the only case where this could happen. So I'm a bit torn: it feels smoother than my implementation in #7686, but it's also more prone to having animation issues. |
|
@jmartinesp OK, I'll fix the issues you've mentioned. Are you generally OK with this PR though? |
|
I'm not sure if splitting the timeline items and the read receipts is a good idea, but it seems like it's the only thing that could make inserting an event seamless: I tested some animations a bit more without splitting the events and read receipts, and while they looked better than before, there's a point where the previous event has lost its sent state and the new one receives it where the timeline item sizes change and there's more movement than I'd like. Also, with the code in this PR there's a weird issue with read receipts that happens randomly: the animations for them are smooth at first, but then at some point they start being overlapped by the item above them and they kind of 'slide down' from them. I guess this too is caused by the reversed LazyList layout 🫤 : Record_2026-09-10-17-22-29_ce4edcc1f0a419f86d172129647bdbcb.mp4 |
7283530 to
b4381f9
Compare
b4381f9 to
7a299aa
Compare
|
@jmartinesp I've fixed the regressions you've mentioned, seems to be working so far. I'll regenerate screenshots to see what's up. |
|
This looks really good, the I'm still not a fan of separating the read receipts into their own items, and I'm a bit scared of the complexity of the mapper, but the result is really good. I'd be in favour of merging this, but I'd want to know what @bmarty thinks first. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7688 +/- ##
===========================================
- Coverage 80.90% 80.87% -0.04%
===========================================
Files 2780 2782 +2
Lines 81899 82065 +166
Branches 11284 11324 +40
===========================================
+ Hits 66264 66369 +105
- Misses 11290 11334 +44
- Partials 4345 4362 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
We've discussed this with the team, and given we're about to start working on a multi-message selection feature that could conflict with this PR, we'll implement that first and then try to figure out if this solution still fits, since having the read receipts as a separate timeline item could be a problem in this case. |
Content
Refactor the timeline row logic a bit to allow a smooth timeline animation for new rows
Motivation and context
Fixes #6966
Screenshots / GIFs
Tests
Tested devices
Checklist