Skip to content

qt: Toggle banned peers visibility#274

Open
umop wants to merge 2 commits into
bitcoinknots:29.x-knotsfrom
umop:toggle-banned-peers-visibility
Open

qt: Toggle banned peers visibility#274
umop wants to merge 2 commits into
bitcoinknots:29.x-knotsfrom
umop:toggle-banned-peers-visibility

Conversation

@umop

@umop umop commented Mar 4, 2026

Copy link
Copy Markdown

Closes #185
Adds a Hide / Show button on the banned peer list table which shows / hides banned table if visible. Default is to show. Also persists the state in QSettings.

Also cleaned up some indentation and closing tags in the debugwindow.ui

There don't appear to be existing automated tests for individual Peers tab UI elements or visibility toggles. The existing Qt tests focus on RPC command parsing, wallet operations, and high-level integration testing rather than specific widget interactions.

Manually tested:

  • Ban a peer -> verify section appears with Hide button
  • Click Hide -> verify table collapses, button shows "Show"
  • Click Show -> verify table reappears, button shows "Hide"
  • Close and reopen Node window -> verify state persists
  • Unban all peers → verify section (heading + button + table) disappears
  • Button vertical alignment with "Banned peers" label
  • No impact on other tabs (Info, Console, Graph)
  • No extra spacing when table is hidden
  • Waiting for expiry and created Banned peer remains after expiration. #273 which remains (banned peers remain in list after expiry).

Showing:
image

Hidden:
image

Comment thread src/qt/rpcconsole.cpp Outdated
Comment thread src/qt/forms/debugwindow.ui Outdated
Comment thread src/qt/rpcconsole.cpp Outdated
@umop umop changed the title Toggle banned peers visibility qt: Toggle banned peers visibility Mar 9, 2026

@luke-jr luke-jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should be a single commit, with a subject like "GUI/Peers: Add button to hide/show ban table" and details after a blank line.

Comment thread src/qt/rpcconsole.cpp Outdated
Comment thread src/qt/rpcconsole.cpp Outdated
Comment thread src/qt/rpcconsole.cpp Outdated
Comment thread src/qt/rpcconsole.cpp Outdated
Adds a Hide/Show toggle button next to the "Banned peers" heading that allows users to collapse the ban table while keeping the heading visible. The state persists across sessions via QSettings.
@umop umop force-pushed the toggle-banned-peers-visibility branch from 9226e55 to 7c1a63c Compare March 18, 2026 17:33
Comment thread src/qt/rpcconsole.cpp Outdated
Comment thread src/qt/rpcconsole.h

@luke-jr luke-jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so close, but the Peers tab without any bans now has an extra padding at the bottom

@umop

umop commented Apr 9, 2026

Copy link
Copy Markdown
Author

I set the bottom margin to 30 to match the current ui at HEAD. There is still space there, though (there always was), so if we want to match other tab layouts, we can reduce that to a smaller value:

Current and old Peers tab:

image

Console tab:

image

@luke-jr

luke-jr commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Before:
Screenshot_20260414_155601
After:
Screenshot_20260414_155718
Before:
Screenshot_20260414_155626
After:
Screenshot_20260414_155816

Margins still very different :(

…r spacing and implemented requested whitespace review fixes.

Wrapped the ban heading and table in a container widget to control visibility and spacing as a unit. This ensures the ban section (including margins) only appears when banned peers exist, preventing excess bottom padding when the list is empty.
@umop umop force-pushed the toggle-banned-peers-visibility branch from 8b2a365 to af294fe Compare April 15, 2026 20:41
@umop

umop commented Apr 15, 2026

Copy link
Copy Markdown
Author

Very odd. My upstream baseline margin looked different than yours. Perhaps OS differences? I wasn't able to view your screenshots at full resolution (GitHub isn't letting me view them outside of the inline preview), so I estimated the target margin by measuring pixels from the inline images:

Your before (upstream): ~23px bottom margin
Your after (my PR): ~30px bottom margin
My build (macOS): ~35px bottom margin
Cross-platform scaling factor: 35/30 ≈ 1.17, so targeting 23 px on your end → 23 × 1.17 ≈ 27px on mine. That's a delta of -8px from what I was seeing (35 → 27), which translates to 30 - 8 = 22 in the .ui file.

I changed the bottom margin from 30 to 22. Should be much closer to the upstream baseline. Let me know if it's still off on your end and I can tweak further.

Befores on my end (note feature not added):

image image

Afters:

image image

@luke-jr

luke-jr commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

There are infinite OS/style combinations; trying to special-case one or two of those isn't the right approach.

@umop

umop commented Apr 16, 2026

Copy link
Copy Markdown
Author

Sorry, what I meant is that I reduced the margin by what seemed to be the correct amount per your screenshots, so the margin should be correct now. I amended the previous commit to avoid commit cruft per your previous comment about squashing the commits down to one, so maybe it wasn't clear that a change was made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show/Hide Banned Peers (gui)

2 participants