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

Fix read receipt animation #12923

Merged
merged 3 commits into from
Sep 3, 2024
Merged

Fix read receipt animation #12923

merged 3 commits into from
Sep 3, 2024

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Aug 23, 2024

The way it was done involved remembering dom nodes and then getting their position later when animating the receipt to its next position, but I'm not sure how this worked since the DOM node may not neccessarily be in the DOM anymore. Instead, just remember the bounding box coordinates. At worst it might go weird if the window is resized but seems fine in practice. Also, keeping references to dom nodes feels like a fast road to memory leaks.

Fixes element-hq/element-web#27916

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

The way it was done involved remembering dom nodes and then getting
their position later when animating the receipt to its next position,
but I'm not sure how this worked since the DOM node may not neccessarily
be in the DOM anymore. Instead, just remember the bounding box coordinates.
At worst it might go weird if the window is resized but seems fine in
practice. Also, keeping references to dom nodes feels like a fast road
to memory leaks.

Fixes element-hq/element-web#27916
also change a condition to make it testable
@dbkr dbkr added this pull request to the merge queue Sep 3, 2024
Merged via the queue into develop with commit 6bfdb3e Sep 3, 2024
29 checks passed
@dbkr dbkr deleted the dbkr/fix_read_receipts_skyfall branch September 3, 2024 12:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read receipts are falling from the sky once again
2 participants