Picking up #289
This adds a Direction column, making the peers tab the same as the Direction/Type row in the peer details and the direction and type columns in our other user-facing peer connections table in -netinfo.
Users can now sort the peers table by direction. The default sort is set to inbound, then outbound.
| Master | PR |
|---|---|
 |
 |
25@@ -26,6 +26,8 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd
26 return left_stats.nodeid < right_stats.nodeid;
27 case PeerTableModel::Address:
28 return left_stats.addrName.compare(right_stats.addrName) < 0;
29+ case PeerTableModel::Direction:
30+ return left_stats.fInbound > right_stats.fInbound; // default sort Inbound, then Outbound
665@@ -666,10 +666,12 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction
666 {
667 QString prefix;
668 if (prepend_direction) {
669+ //: Connection direction. Also used in CONNECTION_TYPE_DOC and PeerTableModel::data().
This translator comment will be assigned to the first string only, i.e., “Inbound”.
We should add a translator comment to one string at a time (here and other places).
Direction column. See: #317 (comment)
updated from a44eec9 -> 5d7463a (pr317.01, pr317.02, diff)
Changes:
Direction column. The work done in those commits will be picked up in another PR.Direction column and the translator comment for itDirection column in PeerTableModel::data25@@ -26,6 +26,8 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd
26 return left_stats.nodeid < right_stats.nodeid;
27 case PeerTableModel::Address:
28 return left_stats.addrName.compare(right_stats.addrName) < 0;
29+ case PeerTableModel::Direction:
30+ return left_stats.fInbound > right_stats.fInbound;
> rather than < operator is used here, unlike the other columns
updated from 5d7463a -> 0183415 (pr317.02 -> pr317.03)
Changes:
Co-authored-by: Jarol Rodriguez <jarolrod@tutanota.com>
This still takes up an annoying chunk of horizontal size. #363 ?
Recently merged PRs gained us a lot of horizontal space so I thought it might no longer be an obstacle.
The issue with arrows is they would not resolve the discrepancy between the connection direction and type, which are displayed in full in the details area (“Outbound Full Relay”) but lack the direction in the main table (“Full Relay”). This causes confusion, e.g. for a bitcoin dev in a bitcoin/bitcoin issue who thought it was a bug.
Users can now sort the peers table by direction.
~With the up/down arrow before the address in the same column, you can already sort by direction. The column is sorted (or you could also say “grouped” like in https://github.com/bitcoin/bitcoin/pull/13537 where the arrow prefix was introduced) by direction first (since the arrow is the first character), then by address (the following characters). You only cannot sort by address regardless of the direction, but I wonder what the use case could be for that.~ I only thought that would be the case. See issue #397
This still takes up an annoying chunk of horizontal size. #363 ?
#256 now allows a user to customize the widths of the table columns. It does not take up an annoying chunk of horizontal size if the user has resized the columns to their preference. @rebroad
NACK - I prefer the existing functionality.
The existing functionality of no Direction column? 🤔
tACK 6971e790c32253ecddb6108e796afd3892ced7fe
Compiled and tested successfully on Ubuntu 20.04
Displaying Connection Direction would help clarify some confusion regarding connection direction, as was suggested in #289. I like the approach of this PR over #363, because for the following reasons:
I would, however, like to make a suggestion. Instead of removing the arrow, they should be moved to the Direction Column. And also‘OUTBOUND’ and ‘INBOUND’ should be replaced by ‘OUT’ and ‘IN” as it is done here (under PR alternative diff header in the table) as it would convey the information correctly at the same time being aesthetically more pleasing.
ACK 6971e790c32253ecddb6108e796afd3892ced7fe, tested on Linux Mint 20.2 (Qt 5.12.8).
This change is an improvement for the following reasons:
I do not consider taking horizontal space by the new “Direction” column as a blocker for approving this PR.
A screenshot from my system:
It looks really great!
I like #401 very much, please see my suggestion B.3 there.
An empty connection type I find confusing. If the user sees (like I did with 22.0rc2), that always only all inbound connections have an empty type, the user thinks, OK I guess it’s not detectable or something for inbound. But it leaves uncertainty, is confusing.
Here is an example of a simple arrow based direction column for comparison.
Using that much visual real estate for direction seems impractical.
https://github.com/bitcoincore-dev/gui/pull/1
()
Here is an example of a simple arrow based direction column for comparison.
What does “↑” mean in the column with unnamed header title?
one possible option is to put a unicode earth symbol in the column title.
🜨
This is both prudent on visual real-estate and universally recognized.
arrow up (as in upload) means outbound data to most people these days.
arrow down (as in download) means inbound data these days.
arrow up (as in upload) means outbound data to most people these days.
arrow down (as in download) means inbound data these days.
Upload/download are about a direction of data transferring. Inbound/outbound are about a connection initiator.
Let’s don’t add more mess :)
I suspect the long term solution is to leverage existing symbolic/unicode libraries to communicate with the node operator. This is provide a lot of information about the node “at a glance” and minimize the need to translate. Using a symbolic library has the added benefit of being prudent in its use of gui real-estate. This will also affect on-going Android mobile GUI design.
Reference: https://google.github.io/material-design-icons/
Explaining fundamental GUI designs and concepts is fun. :) (not really)
As I mentioned in the comment below - drawing inspiration from the established iconography of git is going to go a long way. In fact reusing the established icons for some gui elements may avoid reinventing the wheel on this…
117- return QString::fromStdString((rec->nodeStats.fInbound ? "↓ " : "↑ ") + rec->nodeStats.addrName);
118+ return QString::fromStdString(rec->nodeStats.addrName);
119+ case Direction:
120+ return QString(rec->nodeStats.fInbound ?
121+ //: An Inbound Connection from a Peer.
122+ tr("Inbound") :