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

Performance inprovements for chat drawer #63

Merged
merged 3 commits into from
Jan 12, 2015

Conversation

dimaursu
Copy link

@dimaursu dimaursu commented Jan 3, 2015

I switched the animation from a jQuery based one, to a pure CSS implementation.
The state of the drawer(open-closed) is kept in a checkbox.

The new animation uses transform: translate3D() - this moves the drawer in a separate render layer, and the performance improvements are at least 3x, in Firefox benchmarks. On my Thinkpad X201, in the jQuery implementation the FPS would drop to about 5, now it's at about 15 FPS, and the animation is much smoother (it's visible with the naked eye).

Some visual tradeoffs were made - I got rid of the box-shadow, and the drawer doesn't push the content (which triggers layout recalculations and repaints).

For now I didn't use any vendor-specific prefixes, those will be added later, maybe through AutoPrefixer (https://github.com/postcss/autoprefixer)

@Flaburgan
Copy link
Member

Thank you for this improvement! If you want to check the performance of the mobile version, I think the drawer animation can also be improved (see https://github.com/diaspora/diaspora/blob/develop/app/assets/stylesheets/mobile/header.css.scss#L172) even if I use a CSS translate here too, the animation is not really smooth.

@dimaursu
Copy link
Author

dimaursu commented Jan 3, 2015

Attaching the mobile browser through a remote debugger would help diagnose the problem.
Also, why use the star (*) selector? It's slow, even though I don't think that's the problem here. Translating just the container would do just fine, and would promote the entire container in it's own layer. Right now I guess all elements get their own layer, which is a waste of memory (~2MB / layer).
Box-shadows are also a performance killer on mobile phones, and using "opacity" outperforms a transparent background - it's also performed on the GPU.

I will test the mobile version today - if I will come up with good results, I will do a pull-request :)

@Flaburgan
Copy link
Member

@dimaursu you can also open issues about it, I'd like to learn how to improve CSS performance :)

@zauberstuhl
Copy link

@dimaursu thank you very much for your contribution. Could you do me a favor and remove the changes made in doc and build directory? You probably executed grunt. I decided to do that before we release something. Otherwise we would have on every PR huge changes which are unnecessary.

Cheers, Lukas

@dimaursu
Copy link
Author

dimaursu commented Jan 3, 2015

@zauberstuhl done :) Btw, that should be written in the README, the stuff about doc and build directories ;)

@zauberstuhl zauberstuhl added this to the next-build milestone Jan 3, 2015
zauberstuhl pushed a commit that referenced this pull request Jan 12, 2015
Performance inprovements for chat drawer
@zauberstuhl zauberstuhl merged commit 36a71a0 into diaspora:develop Jan 12, 2015
@svbergerem
Copy link
Member

With this PR the classes chat-roster-shown and chat-roster-hidden are no longer added to body > .container (I implemented that in #42) Because of that some parts of the diaspora* page are again hidden by the roster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants