Till now, peerTable and banTable under the peerTab were fixed in height and cannot be resized. This creates unnecessary inconvenience, especially when you have many items on one table and not many on another.
This PR proposes to add a splitter between these two tables, allowing them to be resized freely.
| Master | PR |
|---|---|
 |
 |
concept ACK
I like the resize. Are they connected or active traffic?
Are they connected or active traffic?
I think they both mean the same thing in this context. Connected peers, implies the peer that we are currently connected to, indicating the active traffic.
977+ <widget class="QWidget" name="peerTable" native="true">
978+ <layout class="QVBoxLayout" name="verticalLayout_9">
979+ <item>
980+ <widget class="QLabel" name="peerHeading_2">
981+ <property name="text">
982+ <string>Connected peers</string>
I’m not sure about this.
In any case, I don’t think this should be a part of this PR considering the title suggests you want to just add functionality to resize tabled
This shouldn’t display until there is a Banned Peers table Additionally, I shouldn’t be able to hide any of the tables
These are valid suggestions. Let me take a look into implementing these.
I don’t think this should be a part of this PR considering the title suggests you want to just add functionality to resize tabled
Ok then, I shall remove the name change from this PR and may open it as another pull request in the future (if need be)
This shouldn’t display until there is a Banned Peers table
Additionally, I shouldn’t be able to hide any of the tables
The new label is redundant: the tab name itself indicates what the user is viewing.
Concept ACK to adding a splitter.
Updated 5e54cfe to c09e6ad (pr457.01 -> pr457.02) Addressed @jarolrod comments
Updates:
3px to 4px.This shouldn’t display until there is a Banned Peers table @jarolrod. As you rightly mentioned, showing splitter when no BanTable was looking kind of off. But in the new iteration of this PR, the splitter seems like an element of peerTable and looks aesthetically pleasing even when there is no banTable. I have updated the PR description with the new Screenshots. You can look at them to better get the feel of what I am trying to say.
Getting there, but the splitter should behave like the splitter that currently exists between the Peers Table and the Peer Information Pane.
When I select a Peer, I get the Peer Information Pane, as well as a splitter where I can resize as needed. When the peer is de-selected, the splitter disappears; because there is nothing to resize.
Similarly, until I have a banned peer and a banned peer table, the splitter being introduced here should not be displayed at all because there is nothing to resize. Additionally, if i unban a peer and subsequently no longer have a banned peers table, the splitter should disappear as well.
Updated from c09e6ad39da24a7a969533f6ce5c9cea14e8c9ce to aa6229e5d2bebe20d3e8e3eded8ff3ad7841bfcb (pr457.02 -> pr457.03, diff) Addressed @jarolrod comments
Changes:
5@@ -6,13 +6,16 @@
6 <rect>
7 <x>0</x>
8 <y>0</y>
9- <width>740</width>
10- <height>430</height>
11+ <width>744</width>
12+ <height>531</height>
953- </property>
954- <property name="textInteractionFlags">
955- <set>Qt::NoTextInteraction</set>
956+ <widget class="QSplitter" name="tableSplitter">
957+ <property name="styleSheet">
958+ <string notr="true">QSplitter::handle{background: #a6a6a6;}</string>
Do you recommend keeping the splitter invisible?
I don’t think the hardcoded color will work for any platform and appearance (dark/light), no?
I don’t think the hardcoded color will work for any platform and appearance (dark/light), no?
The spiller looks fine for the default light mode (in Linux) and dark mode in macOS, which can be seen in the screenshots of the following comments: light; dark.
However, this doesn’t guarantee that this will work for any custom appearance. Do you recommend removing the hardcoded color and making the splitter invisible again?
Why not keeping the same look as for the horizontal splitter?
Do you recommend keeping the splitter invisible?
What does make you think so?
Do you recommend keeping the splitter invisible?
By this, I meant to say, keeping it the same as the horizontal splitter.
Why not keep the same look as for horizontal splitter?
I think this will be a more optimal approach to go with.
But the horizontal splitter is not invisible :)
And having different looks for similar GUI elements does not as an optimal design for me.
753@@ -750,6 +754,9 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
754 ui->startupTime->setText(model->formatClientStartupTime());
755 ui->networkName->setText(QString::fromStdString(Params().NetworkIDString()));
756
757+ //Setup Peer and Ban Table Splitter
758+ ui->tableSplitter->setChildrenCollapsible(false);
759+
This code could be a part of the debugwindow.ui file as it is for the horizontal splitter.
Qt Designer tool could very helpful for such tasks.
38@@ -36,7 +39,7 @@
39 <item>
40 <widget class="QTabWidget" name="tabWidget">
41 <property name="currentIndex">
42- <number>0</number>
43+ <number>3</number>
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Updated from aa6229e5d2bebe20d3e8e3eded8ff3ad7841bfcb to 0c364201a2e16355cd6424c6137761c7967db4e0(pr457.04 -> pr457.05) Addressed @hebasto and @randymacmilan comments
childrenCollapsible=False to debugwindow.ui file.886@@ -887,85 +887,112 @@
887 <height>0</height>
888 </size>
889 </property>
890- <layout class="QVBoxLayout" name="verticalLayout_7">
891+ <layout class="QVBoxLayout" name="verticalLayout_9">
Since this PR introduced a new sub-Layout, the introduced sub-Layout was named in the chronological order of its introduction. And since verticalLayout_7 already exists (on Line 935), giving this the same name leads to a warning message while compiling:
0qt/forms/debugwindow.ui: Warning: The name 'verticalLayout_7' (QVBoxLayout) is already in use, defaulting to 'verticalLayout_71'.
So, I am letting this change be as it is for now, and I welcome any suggestions you might have.
Approach ACK 0c364201a2e16355cd6424c6137761c7967db4e0.
Could you please separate a refactoring commit which introduces the banTable widget?
- This is a pure refactoring change and is done so that the table
splitter can be added between peerTable and banTable in the following
commit.
This commit adds a splitter between the peerWidget and the
banTable widget. This adds the ability to resize these windows
to the user’s needs.
Updated from 0c364201a2e16355cd6424c6137761c7967db4e0 to 5a9bdce0ad753919522b8bc627c1c12d92d7cae8 (pr457.05 -> pr457.06, diff)
Addressed @hebasto comment
Changes:
width/height changes.
While the resizing works, the UI here does not. The UI element representing the splitter is too small and is pinned to the bottom of peerTable.
| linux | macOS |
|---|---|
 |