qt: peers-tab resizeColumnsToContents #338

RandyMcMillan commented at 6:31 AM on May 17, 2021: contributor

No description provided.

RandyMcMillan commented at 6:34 AM on May 17, 2021: contributor

Related to #256

This PR handles resizing column widths programmatically.

RandyMcMillan commented at 6:39 AM on May 17, 2021: contributor

Fields that commonly need to be resized to view are resized based on content length.

Screen Shot 2021-05-17 at 2 35 09 AM

hebasto added the label UI on May 17, 2021

in src/qt/bantablemodel.cpp:126 in 10921a92bb outdated

 121 | @@ -120,6 +122,8 @@ QVariant BanTableModel::data(const QModelIndex &index, int role) const
 122 |      const auto column = static_cast<ColumnIndex>(index.column());
 123 |      if (role == Qt::DisplayRole) {
 124 |          switch (column) {
 125 | +        case Bumper:
 126 | +            return QString::fromStdString("");

jarolrod commented at 9:42 PM on May 17, 2021:

why does this need to be QString::fromStdString("")

in src/qt/peertablemodel.cpp:107 in 10921a92bb outdated

 141 | @@ -140,10 +142,12 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const
 142 |          case Ping:
 143 |          case Sent:
 144 |          case Received:
 145 | -            return QVariant(Qt::AlignRight | Qt::AlignVCenter);
 146 | -        case Subversion:
 147 |              return {};
 148 | -        } // no default case, so the compiler can warn about missing cases
 149 | +        case Subversion:
 150 | +            return QVariant(Qt::AlignLeft | Qt::AlignVCenter);

jarolrod commented at 9:44 PM on May 17, 2021:

isn't this unrelated to what you want to do?

in src/qt/peertablesortproxy.cpp:42 in 10921a92bb outdated

  37 | @@ -38,6 +38,8 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd
  38 |          return left_stats.nRecvBytes < right_stats.nRecvBytes;
  39 |      case PeerTableModel::Subversion:
  40 |          return left_stats.cleanSubVer.compare(right_stats.cleanSubVer) < 0;
  41 | +    case PeerTableModel::Bumper:
  42 | +        return left_stats.nodeid < right_stats.nodeid;

jarolrod commented at 9:45 PM on May 17, 2021:

Why this sorting scheme?

RandyMcMillan commented at 7:03 PM on May 20, 2021:

Sorting by nodeid was simply to have sorting functionality for that column as opposed to no sorting functionality.

jarolrod commented at 9:45 PM on May 17, 2021: member

I'm failing to understand what this is trying to address, can you clarify a bit? 🤔

promag commented at 10:12 PM on May 17, 2021: contributor

Please describe the goal, what is trying to improve and how it relates to #256. Looks like #256 conflicts with QHeaderView::ResizeToContents?

RandyMcMillan commented at 7:41 PM on May 20, 2021: contributor

Goal of this PR/Discussion

Ideally formatting the initial column widths can reduce the number of click events the user needs to perform to be able to view the data presented.
Reducing click events where ever possible is ideal for normal users & tremendously valuable for scenarios where the user doesn't interact with this software primarily with mouse/touch methods.

Motivations for /gui fork as far as I can tell...

It is rooted in improving UI/UX (User Experience)
REVIEW ISO 9241-210:2019(en)
It is really impractical to require /gui PRs have to restate basic GUI/UI/UX motivations every time.

hebasto commented at 9:17 AM on May 23, 2021: member

Concept NACK.

There are some concerns about QHeaderView::ResizeToContents.

It increases complexity in such a way that could lead, at some scale, to deteriorating the GUI responsiveness
From Qt docs:

The size cannot be changed by the user...

Usually, in this project, we strive do not make decisions instead of users in such questions :)

Some users could feel uncomfortable with possible continuous reflowing of the peers table. I'm among of them.

jarolrod commented at 8:31 PM on May 24, 2021: member

Concept NACK

This unnecessarily restricts the user-configurability of the GUI. Testing this PR, I am no longer able to resize the tabs on my own to my liking. Per Qt docs, this is because of the use of QHeaderView::ResizeToContents.

In general, we should ONLY restrict user freedom when there is good reason and motivation to do so. Tables are meant to be resized to one's liking; not everyone wants to see the User Agent string, and some may not want to see the full Address string (this is one reason why we have #276).

Users of the GUI do want to customize the sizing of various aspects of the interface and have that saved and restored. This is a precedent that has been set by the direction GUI development has taken, See #165, #194, #205, and #256.

DrahtBot commented at 10:24 PM on May 26, 2021: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>

DrahtBot added the label Needs rebase on May 26, 2021

RandyMcMillan marked this as a draft on May 26, 2021

qt: peers-tab resizeColumnsToContents f7eaa6043e

RandyMcMillan closed this on Jun 7, 2021

rebroad commented at 5:13 PM on June 26, 2021: contributor

Can we make it do this only until the user tries to manually resize a column?

bitcoin-core locked this on Aug 16, 2022