Don't clear traffic graph when changing interval

rebroad commented at 11:18 am on November 25, 2021: contributor

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.

Don't clear traffic graph when changing interval 59ce791e41

hebasto added the label UX on Nov 25, 2021

shaavan commented at 3:12 pm on November 25, 2021: contributor

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 commented at 10:44 am on November 26, 2021: contributor

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:

Screenshot from 2021-11-26 16-06-54

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

Screenshot from 2021-11-26 16-07-00

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 commented at 7:50 am on November 27, 2021: member

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 commented at 11:37 pm on November 27, 2021: contributor

Postponed until approach NAK is tackled or fighted.

rebroad commented at 2:53 pm on December 1, 2021: contributor

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

rebroad commented at 4:10 pm on December 1, 2021: contributor

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 commented at 4:11 pm on December 1, 2021: contributor

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 commented at 10:54 am on December 2, 2021: contributor

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 commented at 5:12 pm on December 13, 2021: contributor

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 commented at 2:28 pm on January 16, 2022: contributor

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 commented at 7:07 pm on January 24, 2022: contributor

REF: https://github.com/bitcoin-core/gui/issues/532

RandyMcMillan commented at 6:54 am on January 27, 2022: contributor

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

#539

https://user-images.githubusercontent.com/152159/151307221-f987e8e3-7079-46bd-9723-299f3875a039.mov

DrahtBot added the label Needs rebase on Feb 4, 2022

DrahtBot commented at 11:02 am on February 4, 2022: contributor

🐙 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 commented at 4:23 pm on May 26, 2022: member

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

hebasto closed this on May 26, 2022

bitcoin-core locked this on May 26, 2023

Don’t clear traffic graph when changing interval #484

Conflicts