Enlarge Network Traffic Graph

RandyMcMillan commented at 9:52 pm on September 14, 2020: contributor

This minor GUI change allows for a larger display of the Network Graph.

Before/master:

Screen Shot 2020-09-20 at 1 28 32 AM

After/PR:

Screen Shot 2020-09-20 at 1 36 11 AM

Before/master:

Screen Shot 2020-09-20 at 1 32 07 AM

After/PR:

Screen Shot 2020-09-20 at 1 35 10 AM

I capitalized “Window” in the title per standard “Human Interface Guidelines” and common sense.

I added some accessibility settings as part of an on going personal effort to make the GUI more user friendly for impaired users.

hebasto commented at 6:47 am on September 15, 2020: member

@RandyMcMillan

This minor GUI change allows for a larger display of the Network Graph.

On master one can maximize the “Node window” with activated “Network Traffic” at will. How this PR changes that behavior?

I added some accessibility settings as part of an on going personal effort to make the GUI more user friendly for impaired users.

These changes seem unrelated to the PR goal, no?

RandyMcMillan commented at 8:29 pm on September 18, 2020: contributor

@hebasto - this change has to do with better use of space within the confines of the tab itself.

As you can see - there is a lot of unused space in the presentation…

Screen Shot 2020-09-18 at 4 18 31 PM

This change simply emphasizes the graph and minimizes the drab gray currently occupying a 1/4 of this tabs real estate…

hebasto commented at 8:58 pm on September 18, 2020: member

Concept ACK on reordering child widgets. @RandyMcMillan Could you place a screenshot “before/master” in the OP to visually compare with the screenshot of “after/PR”?

RandyMcMillan commented at 6:07 am on September 20, 2020: contributor

Before and After added above. @hebasto - I am currently using

0Qt Creator 4.13.0
1Based on Qt 5.15.0 (Clang 11.0 (Apple), 64 bit)
2
3Built on Aug 25 2020 10:08:56

I noticed the window icon isn’t included when building on macOS. It appears to be a bug in QT for macOS and hasn’t been removed in this PR.

Screen Shot 2020-09-19 at 11 01 55 PM

MarcoFalke renamed this:
~~gui: Enlarge Network Traffic Graph~~
Enlarge Network Traffic Graph
on Sep 20, 2020

in src/qt/forms/debugwindow.ui:14 in 6848c41315 outdated

12+    <height>475</height>
13    </rect>
14   </property>
15   <property name="windowTitle">
16-   <string>Node window</string>
17+   <string>Node Window</string>

luke-jr commented at 8:05 pm on October 24, 2020:

Please leave these alone

in src/qt/forms/debugwindow.ui:39 in 6848c41315 outdated

35@@ -36,7 +36,7 @@
36    <item>
37     <widget class="QTabWidget" name="tabWidget">
38      <property name="currentIndex">
39-      <number>0</number>
40+      <number>2</number>

luke-jr commented at 8:05 pm on October 24, 2020:

No need to change this

luke-jr changes_requested

in src/qt/forms/debugwindow.ui:1093 in f994281550 outdated

1086@@ -1074,8 +1087,8 @@
1087                <rect>
1088                 <x>0</x>
1089                 <y>0</y>
1090-                <width>300</width>
1091-                <height>426</height>
1092+                <width>274</width>

jarolrod commented at 5:31 am on November 5, 2020:

why are you including changes to tab_peers in this? While the peers tab should be redesigned to maximize space, I don’t think this PR (focused on the network traffic graph) should carry any changes to tab_peers

jarolrod changes_requested

RandyMcMillan commented at 8:14 pm on November 7, 2020: contributor

rebased to a commit with the current gui ci configurations

jarolrod approved

jarolrod commented at 3:28 am on November 12, 2020: member

Tested ACK 5036d9a. Built and visually verified on Arch Linux (Kernel 5.8.16) and macOS 10.15.7 both with Qt 5.15. Everything looks good to me.

luke-jr commented at 4:01 pm on November 19, 2020: member

@RandyMcMillan you thumbs-up’d my review comments, but didn’t fix them? O.o

luke-jr commented at 4:55 pm on November 20, 2020: member

Why did you rebase this? Please undo… git reset --hard 7a05be5e47d and force-push.

in src/qt/forms/debugwindow.ui:765 in 7a05be5e47 outdated

760+                 </widget>
761+                </item>
762+                <item>
763+                 <widget class="QLabel" name="label_24">
764+                  <property name="accessibleName">
765+                   <string>Recieved</string>

luke-jr commented at 3:17 pm on November 25, 2020:

0                   <string>Received</string>

RandyMcMillan commented at 4:51 pm on December 5, 2020:

Thank you - I value your input.

in src/qt/forms/debugwindow.ui:768 in 7a05be5e47 outdated

763+                 <widget class="QLabel" name="label_24">
764+                  <property name="accessibleName">
765+                   <string>Recieved</string>
766+                  </property>
767+                  <property name="accessibleDescription">
768+                   <string>Recieved</string>

luke-jr commented at 3:17 pm on November 25, 2020:

0                   <string>Received</string>

in src/qt/forms/debugwindow.ui:784 in 7a05be5e47 outdated

779+                    <width>50</width>
780+                    <height>0</height>
781+                   </size>
782+                  </property>
783+                  <property name="accessibleName">
784+                   <string>Bytes Recieved</string>

luke-jr commented at 3:17 pm on November 25, 2020:

0                   <string>Bytes Received</string>

luke-jr changes_requested

RandyMcMillan commented at 8:24 am on November 29, 2020: contributor

@luke-jr The CI timed out. It passes on my travis-ci account. Please restart. Thanks

luke-jr commented at 2:32 am on November 30, 2020: member

I don’t know how to restart CI on this GUI repo… but presumably if you fix those misspellings, it will re-run… (please don’t rebase in the process)

RandyMcMillan commented at 5:14 pm on December 5, 2020: contributor

@luke-jr

I realized that I can trigger my own rebuilds after I posted. Really useful.

Also: I noticed codespell ignores the /forms folder and test/lint/lint-qt.sh is quite limited. There seems to be room for improvement in qt linting. Worthwhile work?

luke-jr commented at 5:15 pm on December 5, 2020: member

I only noticed the misspellings because the linter caught them…

Enlarge Network Traffic Graph 8b79225642

luke-jr commented at 7:33 pm on December 6, 2020: member

Again, please don’t rebase unless necessary…

git reset --hard 334f0961de8

RandyMcMillan commented at 7:42 pm on December 6, 2020: contributor

This PR wasn’t passing CI. All my other PRs are based on 81d5af42f and pass CI with no conflicts. Help me understand why this PR should stay on a failing commit? I always welcome feedback - thanks! :)

hebasto commented at 4:42 pm on December 7, 2020: member

This PR wasn’t passing CI.

Seem all is ok :)

hebasto changes_requested

hebasto commented at 11:51 am on December 25, 2020: member

I have reviewed the code (8b79225642a5ffc3ea44dcc300ea536139173ad3), and tested it.

I think that adding accessibleName and accessibleDescription properties is out of scope of this PR, and they should be removed.

RandyMcMillan marked this as a draft on Jan 23, 2021

RandyMcMillan closed this on Jan 23, 2021

bitcoin-core locked this on Aug 16, 2022

Enlarge Network Traffic Graph #90