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.

Sign up for GitHub

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

Jump to bottom

Improve Firefox performance #2795

Merged
merged 7 commits into from
Aug 8, 2017
Merged

Improve Firefox performance #2795

merged 7 commits into from
Aug 8, 2017

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Aug 4, 2017 •
edited
Loading

Improves on #2302.

Changes
  • Added a non-reactive semi-transparent layer between background and foreground nodes and edges in the graph view, removing excessive opacity transitions for individual elements (which also fixes Graph edges are visible through the nodes in the background #2431).
  • The semi-transparent layer mentioned above is now applied instantly (without a transition) to avoid the glitches and is made a bit lighter (I thought it looked better that way, but that can easily be reverted).
  • Made default spring animation a bit faster so that it appears smoother and more reactive on Firefox.
  • Decreased the graph animation treshold again to make it bearable on Firefox.

@fbarl fbarl self-assigned this Aug 4, 2017
@fbarl fbarl requested a review from rndstr August 7, 2017 17:29
@fbarl fbarl changed the title [WIP] Improve Firefox performance Improve Firefox performance Aug 7, 2017
@rade
Copy link
Member

rade commented Aug 7, 2017 •
edited
Loading

This supersedes #2432, right?

@rade
Copy link
Member

rade commented Aug 8, 2017

Some of the commits should probably be squashed.

@fbarl
Copy link
Contributor Author

fbarl commented Aug 8, 2017

This supersedes #2432, right?

Yes, good catch @rade!

Some of the commits should probably be squashed.

I actually always squash all of my commits into one when merging a PR - it would make it way more readable when I was working on big PRs of 20+ commits, so I just adopted that practice.

@fbarl fbarl force-pushed the improve-firefox-performance branch from 768ff15 to 322414b Compare August 8, 2017 09:53
rndstr
rndstr approved these changes Aug 8, 2017
View reviewed changes
Copy link
Contributor

@rndstr rndstr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

Firefox fps doubled, nice!

client/app/scripts/charts/node-container.js Outdated
const forwardedProps = omit(this.props, 'dx', 'dy', 'isAnimated', 'scale', 'blurred');
const opacity = blurred ? nodeBlurOpacity : 1;
const { dx, dy, isAnimated, scale } = this.props;
const forwardedProps = omit(this.props, 'dx', 'dy', 'isAnimated', 'scale');

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@fbarl fbarl mentioned this pull request Aug 8, 2017
Closed
@fbarl fbarl merged commit 0823e2e into master Aug 8, 2017
@fbarl fbarl deleted the improve-firefox-performance branch August 8, 2017 14:59
This was referenced Aug 9, 2017
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rndstr rndstr rndstr approved these changes

Assignees

@fbarl fbarl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Graph edges are visible through the nodes in the background
3 participants
@fbarl @rade @rndstr