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

Improve loading placeholder animation performance #4554

Conversation

PVince81
Copy link
Member

@PVince81 PVince81 commented Nov 6, 2020

Fixes #4553

Replaced the SVG gradient animation with a CSS opacity animation that fades between two lists, one with the regular gradient and one with the reversed one.

See ticket for details about analysis.

The fix reduces the CPU load for me from 54% to ~20 %

@PVince81 PVince81 added this to the 💚 Next Major (21) milestone Nov 6, 2020
@PVince81 PVince81 self-assigned this Nov 6, 2020
@PVince81 PVince81 force-pushed the bugfix/noid/optimize-placeholder-animation-double-gradient branch from 232765a to e99519c Compare November 6, 2020 14:40
@PVince81 PVince81 changed the title Bugfix/noid/optimize placeholder animation double gradient Improve loading placeholder animation performance Nov 6, 2020
@PVince81
Copy link
Member Author

PVince81 commented Nov 6, 2020

If that feels a bit over the top, I had another local version where I only pulsed a single list. The CPU usage was almost the same, so if we want we could still stick with the gradient+opacity thing.

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Nice, the only thing is that for me the placeholders are aligned to the left instead of centered!

Replaced the SVG gradient animation with a CSS opacity animation that
fades between two lists, one with the regular gradient and one with the
reversed one.

Signed-off-by: Vincent Petry <[email protected]>
@PVince81 PVince81 force-pushed the bugfix/noid/optimize-placeholder-animation-double-gradient branch from e99519c to 296ab41 Compare November 12, 2020 10:36
@PVince81
Copy link
Member Author

fixed centering and squashed.

I've compared the animation with the old one side by side and couldn't see any significant difference, they look visually the same.

Note: it seems the avatar size and bar alignment doesn't match whatever the actual list has. I attempted to adjust it but failed as the alignment changes with different zoom sizes... If we want alignment identical to the message list we might need to rework the svg markup and use HTML / CSS for alignment instead of CSS calculations. This is out of scope for the current PR. (no quick win sadly)

Signed-off-by: Vincent Petry <[email protected]>
Co-authored-by: Joas Schilling <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High CPU with LoadingPlaceholder animation
4 participants