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

When you start to zoom a graph, the auto graph refresh should be disabled #4274

Closed
TheWitness opened this issue May 19, 2021 · 8 comments
Closed
Labels
bug Undesired behaviour resolved A fixed issue
Milestone

Comments

@TheWitness
Copy link
Member

Describe the bug

Once you zoom into a graph, the graphs page auto-refresh should be disabled, and by default presently it is not, which causes the graph to go back to the original timespan after the refresh operation is complete.

To Reproduce

Steps to reproduce the behavior:

  1. Goto the Edit Profile page
  2. Set auto-refresh to 15 seconds
  3. Return to the graphs page
  4. Zoom to some custom range
  5. Wait 15 seconds and watch the view return to the previous one.

Expected behavior

Refresh should be off until a new preset is set.

@TheWitness TheWitness added the bug Undesired behaviour label May 19, 2021
@TheWitness TheWitness added this to the 1.2.18 milestone May 19, 2021
@netniV
Copy link
Member

netniV commented May 19, 2021

I would say the problem here is that auto-refresh is ignoring the applied filter. If you refresh the page manually, it does keep it.

@netniV
Copy link
Member

netniV commented May 19, 2021

Interestingly, we have two variables that handle this, var refreshTime and var refreshMSeconds so i'm looking at why we have two and not just a single value too.

@netniV
Copy link
Member

netniV commented May 19, 2021

So refreshTime seems to be a left over of something:

Searching for refreshTime
./graph.php:157:        var refreshTime   = <?php print read_user_setting('page_refresh')*1000;?>;
./graph.php:261:                graphTimeout = setTimeout(initializeGraph, refreshTime);
./lib/clog_webapi.php:280:      $refreshTime  = get_request_var('refresh');
./utilities.php:1286:   $refreshTime  = get_request_var('refresh');
./plugins/cycle/cycle.js:76:            timerID = setInterval(refreshTime, 1000)
./plugins/cycle/cycle.js:191:function refreshTime() {

I think we can should unify the usage to be consistent.

@netniV
Copy link
Member

netniV commented May 19, 2021

One other thing I noticed whilst watching this, the body had a load of pace classes applied to it during zoom...

pace-running  pace-running pace-running pace-running pace-running pace-running      pace-running pace-done

@netniV
Copy link
Member

netniV commented May 19, 2021

OK, so the issue seems to be that during the automated refresh, the Preset is not being remembered. However, if you refresh the entire page, it is being picked up.

@netniV
Copy link
Member

netniV commented May 19, 2021

Confirmed, the current timespan is not being updated via the autorefresh but if you zoom twice, it does pick up the change... The bottom was the auto-refresh after zooming, the top was the full page refresh

2021-05-20 00:08:02 - CMDPHP sess_current_timespan=0, graph_start=1621068823, graph_end=1621128759, columns=2
2021-05-20 00:07:47 - CMDPHP sess_current_timespan=11, graph_start=1620860867, graph_end=1621465667, columns=2

@netniV
Copy link
Member

netniV commented May 19, 2021

There seems to be a pointless call to action=update_timespan as that doesn't seem to do anything (unless its part of the general code).

@netniV
Copy link
Member

netniV commented May 20, 2021

I've given up for the night. nothing I do seems to want to set this session variable properly.

TheWitness added a commit that referenced this issue Jun 25, 2021
When you start to zoom a graph, the auto graph refresh should be disabled
@TheWitness TheWitness added the resolved A fixed issue label Jun 25, 2021
@netniV netniV closed this as completed Jun 30, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

2 participants