peers-tab: cleaner presentation - more functionality #135

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

luke-jr commented at 6:13 PM on November 30, 2020:

Leave this alone...

luke-jr commented at 6:20 PM on November 30, 2020:

Leave this alone...

RandyMcMillan commented at 9:37 PM on December 2, 2020:

This is Qt Creator diff junk - I will correct this if/when ready for merge. Thanks

 929 | +             <property name="whatsThis">
 930 | +              <string/>
 931 | +             </property>
 932 | +             <property name="accessibleName">
 933 | +              <string/>
 934 | +             </property>

luke-jr commented at 6:53 PM on November 30, 2020:

This shouldn't be here

RandyMcMillan commented at 9:37 PM on December 2, 2020:

This is Qt Creator diff junk - I will correct this if/when ready for merge. Thanks

RandyMcMillan commented at 8:49 PM on December 3, 2020:

#135 (review)

1024 | @@ -1001,45 +1025,45 @@
1025 |              <height>0</height>
1026 |             </size>
1027 |            </property>
1028 | +          <property name="toolTip">
1029 | +           <string/>
1030 | +          </property>
1031 | +          <property name="statusTip">
1032 | +           <string/>
1033 | +          </property>

luke-jr commented at 6:54 PM on November 30, 2020:

More pointless code

RandyMcMillan commented at 8:49 PM on December 3, 2020:

#135 (review)

1031 | +          <property name="statusTip">
1032 | +           <string/>
1033 | +          </property>
1034 | +          <property name="whatsThis">
1035 | +           <string>Peer Detail</string>
1036 | +          </property>

luke-jr commented at 6:54 PM on November 30, 2020:

Not sure this is useful

1071 |               </property>
1072 | -             <property name="alignment">
1073 | -              <set>Qt::AlignHCenter|Qt::AlignTop</set>
1074 | +             <property name="whatsThis">
1075 | +              <string>Detailed Info of Selected Node</string>
1076 |               </property>

luke-jr commented at 6:55 PM on November 30, 2020:

Is this reachable? (Didn't we remove the "What's This" button?)

1065 | -              <cursorShape>IBeamCursor</cursorShape>
1066 | -             </property>
1067 | -             <property name="text">
1068 | -              <string>Select a peer to view detailed information.</string>
1069 | +             <property name="statusTip">
1070 | +              <string>Select a Node from List</string>

luke-jr commented at 6:56 PM on November 30, 2020:

Where is this visible?

1242 |                   <property name="cursor">
1243 |                    <cursorShape>IBeamCursor</cursorShape>
1244 |                   </property>
1245 | +                 <property name="accessibleName">
1246 | +                  <string/>
1247 | +                 </property>

luke-jr commented at 7:07 PM on November 30, 2020:

blank

1393 |                   <property name="cursor">
1394 |                    <cursorShape>IBeamCursor</cursorShape>
1395 |                   </property>
1396 | +                 <property name="accessibleName">
1397 | +                  <string/>
1398 | +                 </property>

luke-jr commented at 7:08 PM on November 30, 2020:

blank

RandyMcMillan commented at 8:49 PM on December 3, 2020:

#135 (review)

 779 |  {
 780 |      QStringList strList;
 781 |  
 782 |      for (const auto& flag : serviceFlagsToStr(mask)) {
 783 | +        if (flag == "NETWORK_LIMITED"){
 784 | +        strList.append(QString::fromStdString("LIMITED"));

luke-jr commented at 7:09 PM on November 30, 2020:

If we're going to start renaming these, "recent-blocks" makes more sense than "LIMITED"...

 790 | +        sortLocaleAware(strList);
 791 |      }
 792 |  
 793 |      if (strList.size())
 794 | -        return strList.join(" & ");
 795 | +        return strList.join(" ");

luke-jr commented at 7:10 PM on November 30, 2020:

This seems less clear

RandyMcMillan commented at 2:01 AM on December 3, 2020:

This change allows the services string in the detail view to break. The ampersands are extraneous (imo).

 821 | +    }
 822 | +
 823 | +    if (strList.size())
 824 | +        return strList.join("");
 825 | +    else
 826 | +        return QObject::tr(""); //silence is golden

luke-jr commented at 7:11 PM on November 30, 2020:

This conditional seems pointless

 799 |  
 800 | +QString shortFormatServicesStr(quint64 mask)
 801 | +{
 802 | +    QStringList strList;
 803 | +
 804 | +    for (const auto& flag : serviceFlagsToStr(mask)) {

luke-jr commented at 7:11 PM on November 30, 2020:

Probably better to re-implement serviceFlagsToStr here...

RandyMcMillan commented at 2:10 AM on December 3, 2020:

The "Bumper" will be used to add more columns to the peertablemodel. The point of having abbreviated services flags is to conserve space so more information can be displayed in the table view. The underlying design principle is to display as much useful info as possible "at a glance" as well as allowing those columns to be sortable. Connection time and synced blocks will be added in a follow up PR.

 919 | @@ -920,9 +920,30 @@
 920 |            <layout class="QVBoxLayout" name="verticalLayout_7">
 921 |             <item>
 922 |              <widget class="QTableView" name="peerWidget">

RandyMcMillan commented at 8:31 PM on December 3, 2020:

Upon further investigation I see that Qt Creator validates these types of files and generates these elements when Qt expects them. If they are not present when Qt expects them they generate an innocuous log - So they aren't unnecessary code - these elements are created by Qt when it expects them and is VALID code. I recommend stop considering these auto generated elements as "unnecessary code".