Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

peers-tab: bug fix right panel toggle #202

Merged
merged 1 commit into from
Feb 22, 2021
Merged

peers-tab: bug fix right panel toggle #202

merged 1 commit into from
Feb 22, 2021

Conversation

RandyMcMillan
Copy link
Contributor

Initial Presentation:

Screen Shot 2021-01-28 at 8 36 15 PM

When node row selected - panel is presented:

Screen Shot 2021-01-28 at 8 36 22 PM

When network disabled - right panel is hidden:

Screen Shot 2021-01-28 at 8 36 32 PM

@luke-jr
Copy link
Member

luke-jr commented Jan 29, 2021

What is this fixing?

@RandyMcMillan
Copy link
Contributor Author

Hiding the detailWidget is incorrect - hiding the right panel of the splitter view is the correct usage of the splitter view. This change uses the splitter view correctly.

Running the code will make what the change addresses readily apparent.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK 8353e8c

This is a welcome change as it makes more efficient use of the peers tab window. On master the peer detail widget is always shown even if no peer has been selected. This pr introduces a toggle feature where the peer detail widget will only pop up when a peer has been selected.

Checked and confirmed that when multiple peers are selected the peer widget will still hide (introduced in #13)

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK 8353e8c tested rebased on current master. Behavior is initially a bit surprising but this would allow more columns to be added to the peers tab window. Verified that selecting more than one peer, clicking on a column header, or running disconnectnode "" <currently-selected-peer-id> in the console (or on the CLI with the -server startup option) returns the window to its full size. If this is merged, it might be nice to have an obvious way to close the details area like a clickable "close this" icon in the upper left corner of the area.

@RandyMcMillan
Copy link
Contributor Author

@jonatack - thank you! yes - this is the goal! Part of a larger common initiative to make the peers tab contain more info and be more useful - a better use of GUI real estate. :) This PR is sparse - as to not break other work being done. :)

Copy link

@Talkless Talkless left a comment

Choose a reason for hiding this comment

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

tACK 8353e8c, tested on Debian Sid. Made bitcoind connect to bitcoin-qt with the PR changes, and after I quit the bitcoind instance, right panel do disappear, compared to the previous commit where it didn't.

@hebasto
Copy link
Member

hebasto commented Feb 22, 2021

A UX note: to clear selection use Ctrl + click (actually, it inverses selection).

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 22, 2021
8353e8c peers-tab: bug fix right panel toggle (randymcmillan)

Pull request description:

  Initial Presentation:

  ![Screen Shot 2021-01-28 at 8 36 15 PM](https://user-images.githubusercontent.com/152159/106220159-e2a81b80-61a8-11eb-84e9-f9b44375c9a1.png)

  When node row selected - panel is presented:

  ![Screen Shot 2021-01-28 at 8 36 22 PM](https://user-images.githubusercontent.com/152159/106220185-eb98ed00-61a8-11eb-9467-6a762941902d.png)

  When network disabled - right panel is hidden:

  ![Screen Shot 2021-01-28 at 8 36 32 PM](https://user-images.githubusercontent.com/152159/106220235-0a977f00-61a9-11eb-8a10-f31e4312ed31.png)

ACKs for top commit:
  jarolrod:
    ACK 8353e8c
  jonatack:
    ACK 8353e8c tested rebased on current master. Behavior is initially a bit surprising but this would allow more columns to be added to the peers tab window. Verified that selecting more than one peer, clicking on a column header, or running `disconnectnode "" <currently-selected-peer-id>` in the console (or on the CLI with the `-server` startup option) returns the window to its full size. If this is merged, it might be nice to have an obvious way to close the details area like a clickable "close this" icon in the upper left corner of the area.
  Talkless:
    tACK 8353e8c, tested on Debian Sid. Made `bitcoind` connect to `bitcoin-qt` with the PR changes, and after I quit the `bitcoind` instance, right panel do disappear, compared to the previous commit where it didn't.

Tree-SHA512: 8fc156f40bdd61e3ba8db333c729a2a07fd5f0fd1eed56f2fd2aa5ae5864756f8ab6fad74ae2fb0552ee7518b6d489f5800709e6c80c6f31f61fd8ce21cece5f
@hebasto
Copy link
Member

hebasto commented Feb 23, 2021

When no peer is selected, and one calls a context menu, it causes the peer selection with a layout change.

I found such a behavior distracting.

gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 28, 2022
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants