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

Admin Charts Constantly Updating #1064

Closed
chryton opened this issue Apr 13, 2017 · 5 comments
Closed

Admin Charts Constantly Updating #1064

chryton opened this issue Apr 13, 2017 · 5 comments
Assignees
Labels

Comments

@chryton
Copy link

chryton commented Apr 13, 2017

It appears that the interval is not being cleared upon completion of the cart update and it is causing divs to be constantly updated: http://rocketgoatin.space/jetsam/N51TyVJ2ja/

@rhukster
Copy link
Member

@flaviocopes you mind looking at this one?

@rhukster rhukster added the bug label Apr 13, 2017
@flaviocopes
Copy link
Contributor

Checking

@flaviocopes
Copy link
Contributor

I don't know how we missed this performance issue. I tracked the problem down to 8bd7f8d, which is fixing this issue #753.

So there is this loop:

// Sidebar auto-refresh
global.setInterval(() => {
    contentScrollbar.update();
    sidebar.scroller.update();
    Object.keys(Dashboard.Chart.Instances).forEach((chart) => {
        Dashboard.Chart.Instances[chart].chart.update();
    });
}, 150);

The charts refresh every 150ms. This is a comparison of doing the interval every 150ms, then every 2s, then every 10s:

schermata 2017-04-14 alle 15 41 08

Quite a deal. I am unsure how to fix this problem while avoiding the original problem to come up again. We can start by relaxing the graph refresh interval to 2s, so the graphs are refreshed upon various UI changes, but not as much frequently, but we need to figure out a better way to detect those changes.

@flaviocopes
Copy link
Contributor

Actually, I'm testing removing the JS altogether and I don't think we need those chart refreshes any more. Seems some recent CSS change is already fixing the issue mentioned in #753. I'm going to remove that JS in a PR so you can test as well.

@flaviocopes
Copy link
Contributor

I have pushed a fix in develop, which removes the JS refresh of the graphs. Is anyone able to replicate the issues listed in #753? I could not with Chrome / Firefox

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

No branches or pull requests

3 participants