Show ToolTip on Network Traffic graph #492

rebroad · 2021-12-01T14:46:59Z

Enables a tooltip on the graph to make it easier to see what point in time a part of the graph corresponds to.

This PR includes the change in #484, but also deals with the potential issues that the simplicity of 484 introduced.

TODO:

make it easier to select a point on the graph by factoring in the vertical position of the mouse pointer also.
maybe - add ridges on the X-axis to show where an interval change or time jump occurs.
make the Units translateable.

shaavan · 2021-12-02T10:41:42Z

Concept ACK

I think this PR is an improvement from a UX perspective. I shall be testing this PR when the TODO is completed.

rebroad · 2021-12-02T14:51:29Z

Concept ACK

I think this PR is an improvement from a UX perspective. I shall be testing this PR when the TODO is completed.

@shaavan TODOs now updated

rebroad · 2021-12-06T22:15:16Z

Had to keep changing the strUnit function as the printf test kept failing (it makes false assumptions).

src/util/strencodings.cpp

katesalazar · 2021-12-10T20:24:06Z

I'd fancy reviewing this, but my time is very limited. As you keep finishing PRs, I could use some meta issue (or something) helping me realize the dependency tree between PRs without me having to spend time figuring it out. Cheers,

…

On Tue, Dec 7, 2021 at 10:42 AM Rebroad ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/util/strencodings.cpp <#492 (comment)>: > + } else if (value < 1'000'000'000) { + letter = "M"; + value /= 1'000'000; + } else { + letter = "G"; + value /= 1'000'000'000; + } + if (value < 1) return strprintf(strprintf("%%.%df %s%%s", dp, letter), value, strUnit); + if (value < 10) return strprintf(strprintf("%%.%df %s%%s", std::max(dp - 1, 0), letter), value, strUnit); + if (value < 100) return strprintf(strprintf("%%.%df %s%%s", std::max(dp - 2, 0), letter), value, strUnit); + if (value < 1'000) return strprintf(strprintf("%%.%df %s%%s", std::max(dp - 3, 0), letter), value, strUnit); + + return strprintf(strprintf("%%.%df %s%%s", std::max(dp - 4, 0), letter), value, strUnit); +} + +std::string strBps(float bits) { This isn't used by this PR, but is planned to be used in #305 <#305> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#492 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMRS4WYRUK34NOVAYWUPPBDUPXJHPANCNFSM5JEVGYYQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

rebroad · 2021-12-10T23:29:22Z

I'd fancy reviewing this, but my time is very limited. As you keep finishing PRs, I could use some meta issue (or something) helping me realize the dependency tree between PRs without me having to spend time figuring it out. Cheers,

Hi. The latest push to this PR ought to be the last one - I had been trying to get the ToolTips to behave better, which they now do. They no longer time-out (disappear) and they also can now moved along with the graph regardless of the duration/speed selected. Also, the yellow circle will disappear more reliably now if the ToolTip also disappears (due to the mouse pointing elsewhere). It took a while to refine this functionality, which in the latest commit required an additional timer (tt_timer) to check the ToolTip status at an interval independent of the graph being updated, or the mouse moving.

DrahtBot · 2021-12-30T10:53:10Z

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)
#473 (Enable a non-linear network traffic option (click to toggle between linear and non-linear) by rebroad)

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.

katesalazar · 2022-01-04T16:32:29Z

Why isn't "Add formatBytesps function" (cdc5f0b) squashed in
"Show ToolTip on Network Traffic graph" (e5897a7)?

katesalazar · 2022-01-04T16:53:55Z

Built and tested this, seems cool.

katesalazar · 2022-01-04T16:57:50Z

src/qt/trafficgraphwidget.cpp

+        painter.setPen(Qt::yellow);
+        int w = width() - XMARGIN * 2;
+        int x = XMARGIN + w - w * ttpoint / DESIRED_SAMPLES;
+        int y = y_value(floatmax(vSamplesIn.at(ttpoint), vSamplesOut.at(ttpoint)));
+        painter.drawEllipse(QPointF(x, y), 3, 3);


What about fencing these in a function? Maybe a macro?
Maybe add a couple of comments?

katesalazar · 2022-01-04T16:59:18Z

src/qt/trafficgraphwidget.cpp

@@ -262,8 +262,7 @@ void TrafficGraphWidget::setGraphRangeMins(int mins)
    int msecsPerSample = nMins * 60 * 1000 / DESIRED_SAMPLES;
    timer->stop();
    timer->setInterval(msecsPerSample);
-
-    clear();
+    timer->start();


Why did you separate this change in a rev of its own?

DrahtBot · 2022-02-04T11:02:50Z

🐙 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 · 2022-05-26T17:21:57Z

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

katesalazar · 2022-09-18T19:16:38Z

#473 is similarly inactive.

…

On Thu, May 26, 2022 at 7:22 PM Hennadii Stepanov ***@***.***> wrote: Closing due to a long period of inactivity here. Feel free to reopen. — Reply to this email directly, view it on GitHub <#492 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMRS4W7L4JVJ3LULH5S767TVL6XMDANCNFSM5JEVGYYQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

rebroad mentioned this pull request Dec 1, 2021

Don't clear traffic graph when changing interval #484

Closed

rebroad force-pushed the NetworkGraphTooltip branch 2 times, most recently from e1561e0 to 45d8f34 Compare December 1, 2021 23:29

rebroad force-pushed the NetworkGraphTooltip branch 2 times, most recently from b617de1 to ec67f11 Compare December 2, 2021 14:42

rebroad force-pushed the NetworkGraphTooltip branch from 094c5bf to b12b28a Compare December 6, 2021 16:45

rebroad mentioned this pull request Dec 6, 2021

Enable a non-linear network traffic option (click to toggle between linear and non-linear) #473

Closed

rebroad force-pushed the NetworkGraphTooltip branch 8 times, most recently from 12b583a to d672263 Compare December 6, 2021 22:13

rebroad marked this pull request as ready for review December 7, 2021 09:39

rebroad commented Dec 7, 2021

View reviewed changes

src/util/strencodings.cpp Outdated Show resolved Hide resolved

rebroad force-pushed the NetworkGraphTooltip branch 2 times, most recently from 03e8413 to 30fca02 Compare December 9, 2021 21:51

rebroad force-pushed the NetworkGraphTooltip branch from 30fca02 to d563044 Compare December 10, 2021 23:24

rebroad force-pushed the NetworkGraphTooltip branch 2 times, most recently from 6679d1f to 968fb4b Compare December 12, 2021 18:51

rebroad added 3 commits December 12, 2021 20:25

Network Graph - Create y_value function

878be64

Add formatBytesps function

cdc5f0b

Show ToolTip on Network Traffic graph

e5897a7

Don't clear graph when changing interval

6c139eb

rebroad force-pushed the NetworkGraphTooltip branch from 968fb4b to 6c139eb Compare December 12, 2021 19:26

katesalazar reviewed Jan 4, 2022

View reviewed changes

DrahtBot mentioned this pull request Jan 13, 2022

Replace int with std::chrono in for the timer->setInterval() argument #524

Merged

DrahtBot mentioned this pull request Jan 27, 2022

Scale network graph based on time interval #539

Closed

DrahtBot added the Needs rebase label Feb 4, 2022

rebroad mentioned this pull request Feb 24, 2022

Network graph improvements #559

Closed

hebasto closed this May 26, 2022

bitcoin-core locked and limited conversation to collaborators Sep 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show ToolTip on Network Traffic graph #492

Show ToolTip on Network Traffic graph #492

rebroad commented Dec 1, 2021 •

edited

Loading

shaavan commented Dec 2, 2021

rebroad commented Dec 2, 2021 •

edited

Loading

rebroad commented Dec 6, 2021 •

edited

Loading

katesalazar commented Dec 10, 2021 via email

rebroad commented Dec 10, 2021

DrahtBot commented Dec 30, 2021 •

edited

Loading

katesalazar commented Jan 4, 2022

katesalazar commented Jan 4, 2022

katesalazar Jan 4, 2022

katesalazar Jan 4, 2022

DrahtBot commented Feb 4, 2022

hebasto commented May 26, 2022

katesalazar commented Sep 18, 2022 via email

Show ToolTip on Network Traffic graph #492

Show ToolTip on Network Traffic graph #492

Conversation

rebroad commented Dec 1, 2021 • edited Loading

shaavan commented Dec 2, 2021

rebroad commented Dec 2, 2021 • edited Loading

rebroad commented Dec 6, 2021 • edited Loading

katesalazar commented Dec 10, 2021 via email

rebroad commented Dec 10, 2021

DrahtBot commented Dec 30, 2021 • edited Loading

Conflicts

katesalazar commented Jan 4, 2022

katesalazar commented Jan 4, 2022

katesalazar Jan 4, 2022

Choose a reason for hiding this comment

katesalazar Jan 4, 2022

Choose a reason for hiding this comment

DrahtBot commented Feb 4, 2022

hebasto commented May 26, 2022

katesalazar commented Sep 18, 2022 via email

rebroad commented Dec 1, 2021 •

edited

Loading

rebroad commented Dec 2, 2021 •

edited

Loading

rebroad commented Dec 6, 2021 •

edited

Loading

DrahtBot commented Dec 30, 2021 •

edited

Loading