Don't clear traffic graph when changing interval

[rebroad]

This change stops the graph from being reset when the duration is changed. If the user wants it to be cleared, then they can click the reset button that is already present.

[shaavan]

Strong Concept ACK

It felt kind of annoying when you had network traffic data for the last 30 mins, and you, by mistake or intentionally moved the slider, and all data was lost.
I didn't think it was such a simple fix. Thanks for catching it @rebroad!
Let me do the testing and post the tested review soon!

[shaavan]

Approach NACK

The (x-axis) scale of the graph doesn’t update accordingly with the change in timer.
For example,

When I set the timer to 45 min the graph looks like following:

But when I changed the scale to far-right, i.e., 1d. The graph remained the same:

The change done in this PR allows displaying the graph in the future according to the latest timer setting. However, since the time-scaling of the graph that is already displayed is not changing accordingly, it could lead to wrong conclusions on the user’s side.

The optimal behavior would be to expand or contract the displayed graph according to the latest time-scale setting.

[jarolrod]

Approach NACK, instead you should do more sampling and caching. Then display the appropriate data as the slider moves instead of clearing and drawing.

[katesalazar]

Postponed until approach NAK is tackled
or fighted.

[rebroad]

@shaavan PR #492 hopefully addresses your concerns raised here.

[rebroad]

The (x-axis) scale of the graph doesn’t update accordingly with the change in timer. For example,

I would argue that the user would have to be extraordinarily unintelligent to have an expectation of the X-axis being linear, if it is the same user that changed the duration and could see (at the time) that the pre-existing data was not resampled.

The only scenario I can see confusion arising is it someone viewed the graph without being aware that the scale has been changed mid-way through the data. I think there ought to be a limit to the extent we (developers) take responsibility for all the ways people can misinterpret data that has been manipulated by others.

The assumption here is there is only one user - the same user viewing the data as the user configuring the sample rate of that data.

And even if a 3rd party was involved - there's really very little damage that could occur from any false assumptions.

Finally, bear in mind, that the X-axis has never been guaranteed to be linear, for example, if the OS running it is suspended or hibernated, then the graph could have time-jumps within it, which are not immediately apparent. #492 helps to make these more apparent also.

[rebroad]

Approach NACK, instead you should do more sampling and caching. Then display the appropriate data as the slider moves instead of clearing and drawing.

Personally, I would not want this functionality (I did consider it). I much prefer maximizing the use of the data already sampled (which resampling would not achieve).

[shaavan]

PR #492 hopefully addresses your concerns raised here.

#492 would be an improvement over the current implementation and can be helpful in many cases other than this PR.
However, it still does not fix the issue I described in its entirety.

Usually, we don't take the cursor to all the positions on the graph and read the specific time of that instance with focus. We typically glance over the chart as a whole to get a gist of the network traffic. So there is a risk of possible misinterpretation, even for an individual user.

Though @reboard has addressed this in this comment, I still think this is a bad UX design. And personally, for me, the possible declination in the user's experience outweighs the functionality this PR is introducing.

[promag]

How about replacing the slider with something like

Then it just needs some vertical lines with timestamps on the x-axis.

This suggestion is also an alternative to #483.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #539 (gui: scale network graph based on time interval by RandyMcMillan)
• #524 (Replace int with std::chrono in for the timer->setInterval() argument by shaavan)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[RandyMcMillan]

REF: #532

[RandyMcMillan]

@rebroad - I based this PR on the commit in this PR...

#539

Screen.Recording.2022-01-27.at.1.47.21.AM.mov

[DrahtBot]

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

[hebasto]

Closing due to a long period of inactivity here. Feel free to reopen.