Add Type column to peers window, update peer details name/tooltip

[jonatack]

This pull:

• adds a sortable Type column to the GUI Peers tab window
• updates the peer details row to Direction/Type, so the Type column without a direction makes sense (the tooltip is also updated)

[jarolrod]

ACK 09aa053

I love this! I was just thinking about how this would be a good feature today, you beat me to it. Code looks good to me. Tested on macOS Big Sur 11.1 with Qt 5.15.2

[jonatack]

Thanks! Though I should drop the relay type from inbound or check if full or block (same in -netinfo).

[hebasto]

Concept ACK.

[jonatack]

Rebased after merge of #163, added relay type for inbounds, and inversed the sort for the Direction column to be inbound first.

[jonatack]

Memo to myself, if we prefer to sort alphabetically by type name rather than by enum order:

-        return pLeft->m_conn_type < pRight->m_conn_type;
+        return GUIUtil::ConnectionTypeToShortQString(pLeft->m_conn_type, pLeft->fRelayTxes) < GUIUtil::ConnectionTypeToShortQString(pRight->m_conn_type, pRight->fRelayTxes);

[jonatack]

Thank you @hebasto for your feedback! Updated with your suggestions per git diff a657b0c 9f76ba6

[hebasto]

ACK 9f76ba6, tested on Linux Mint 20.1 (Qt 5.12.8):

[jarolrod]

re-ACK 9f76ba6, tested on macOS Big Sur 11.1 (Qt 5.15.2)

[luke-jr]

Why are Outbound/Inbound strings better than arrows?

[jonatack]

Why are Outbound/Inbound strings better than arrows?

Good question. I proposed words instead of arrows for these reasons:

1. The peer details area uses words. When it displays, say, "Outbound Full Relay", it seems more coherent for the column data to also display "Outbound" and "Full Relay", than up-arrow and "Full Relay"
2. -getinfo and -netinfo both also use words for the direction rather than arrows

[luke-jr]

Words use a lot more space than arrows... It's not unreasonable to have details show more .. detail :p

[hebasto]

Why are Outbound/Inbound strings better than arrows?

Words use a lot more space than arrows... It's not unreasonable to have details show more .. detail :p

Actually, words simplify the GUI for new-comers, as an "arrow up" does not imply "outbound" at all.

[jonatack]

Agree. And the GUI uses words for the direction in the main info tab.

[jonatack]

(If space is an issue we could use in/out instead of inbound/outbound, as that is the wording in the GUI info tab, -getinfo, and -netinfo)

[hebasto]

(If space is an issue we could use in/out instead of inbound/outbound, as that is the wording in the GUI info tab, -getinfo, and -netinfo)

I was thinking about it from the beginning, but it won't work for some translations.

[jarolrod]

(If space is an issue we could use in/out instead of inbound/outbound, as that is the wording in the GUI info tab, -getinfo, and -netinfo)

I think we should stick with inbound/outbound. Even if you changed to in/out wouldn't Direction keep the tab at about the same size

[luke-jr]

I was thinking something more like this:

jonatack/gui@add-peers-dir-and-type-columns...luke-jr:tmp_gui_peers_dir_arrows

[jarolrod]

I was thinking something more like this:

jonatack/[email protected]:tmp_gui_peers_dir_arrows

Below is a screenshot of what your branch looks like for me, font issue with the arrows. Anyways, my vote is still for inbound/outbound

[luke-jr]

I suppose we could also do icons with an arrow and tiny in/out text under it

[RandyMcMillan]

@jonatack @hebasto

This worked for me...

https://github.com/bitcoin-core/gui/pull/135/files#diff-e721b02fefce2a90ae95da0841adfdcc610ffccf93236ef236f81c1baead8e01R163

Most of the issues you may run into - I already figured out...

#135

[jonatack]

This current PR with 2 ACKs is my preferred version among the various proposals I have seen.

[jonatack]

I propose that further refinements be done in a follow-up (not by me ;).

[RandyMcMillan]

Something I didn't get to yet - was to separate the uacomment and give it its own column.
This could be very useful for some other ideas Ive been thinking about.

[luke-jr]

jonatack/gui@add-peers-dir-and-type-columns...luke-jr:tmp_gui_peers_dir_arrows is now translation-friendly...

I'm still weak concept nack on inbound/outbound as strings in the table. I think it's worse than the status quo, so would rather this not be merged as-is.

[jonatack]

Moving this back to Draft status until #201 is resolved.

[jonatack]

Ready for review again. Three changes from the previous version:

• It no longer replaces the arrows with a Direction column, which had 2 ACKs but also a light NACK and which can be decided later in another pull
• For now, inbound peers do not display anything in the Type column, following the logic of merged Display plain "Inbound" in peer details #203
• In the details area, the Connection Type field is renamed to Direction/Type, so the Type column without a direction makes sense. Its tooltip is also updated.

I hope the translations work correctly in this latest push.

@hebasto, @jarolrod, @luke-jr, @RandyMcMillan, would you mind having another look?

[jonatack]

Updated the PR description and screenshot with this one:

[jarolrod]

ACK be4cf48

Tested with Spanish translations. The text added in the tooltip and text associated with the Type column will need translations.

[leonardojobim]

Tested ACK be4cf48 on Ubuntu 20.04 on VMWare.

[RandyMcMillan]

Removing "Peer" from the "Peer Id" column would allow for more prudent use of space.
Otherwise it should be translatable. A simple "ID" or "id" would be better. I think all caps "ID" is better imo.

Maybe Type and Network should be left justified?

---

NOTE: #202

PR #202 is going to allow for more info/columns to be added - and more info displayed "at a glance". As more columns are added - the columns can be adjusted for a tighter fit and/or column stretching in a resize event.

---

ACK 6fc72bd

macOS 10.14.6 (18G103)