Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

We shouldn't update the whole TimelinePanel every time we get a RR #1434

Open
matrixbot opened this issue Apr 19, 2016 · 3 comments
Open

We shouldn't update the whole TimelinePanel every time we get a RR #1434

matrixbot opened this issue Apr 19, 2016 · 3 comments
Labels
A-Performance A-Read-Receipts O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@matrixbot
Copy link

Created by @ matthew:matrix.org.

We should just update the two event tiles + render the anim

@germain-gg
Copy link
Contributor

germain-gg commented May 17, 2021

This issue can be extended to

  • Event decryption
  • Room redaction
  • Event replaced

The TimelinePanel component is doing a complete rerender quite often. Feels like we're throwing the baby out with the bathwater

@germain-gg germain-gg added this to Backlog (Unsorted) in Web App Team May 17, 2021
@novocaine
Copy link
Contributor

Does this cause a specific performance problem @gsouquet ?

@SimonBrandner SimonBrandner added X-Needs-Info This issue is blocked awaiting information from the reporter and removed P2 labels Nov 9, 2021
@germain-gg
Copy link
Contributor

It does. The TimelinePanel relies heavily on forceUpdate which will force a React tree reconciliation. It will not always end up updating the DOM, but it will spend a significant amount of time figuring out whether it needs to do it or not.

@SimonBrandner SimonBrandner added O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist labels Nov 9, 2021
@robintown robintown removed the X-Needs-Info This issue is blocked awaiting information from the reporter label Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Performance A-Read-Receipts O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

No branches or pull requests

8 participants