Multiple Selection for peer and ban tables #8874

Merged
merged 2 commits into from Nov 9, 2016

Projects

None yet

7 participants

@achow101
Contributor
achow101 commented Oct 4, 2016

Allows multiple selection and action for the nodes in the peer and ban tables in the Debug Window.

Closes #8864

@fanquake fanquake added the GUI label Oct 4, 2016
@fanquake
Contributor
fanquake commented Oct 4, 2016

OS X screenshots:
Current
before
8874
after

When multiple nodes are selected the dropdown menu should be "Nodes".

@paveljanik
Contributor
paveljanik commented Oct 4, 2016 edited

concept ACK

... or you can change the text to "Disconnect" and "Ban for 1 hour". I think we should use consistent terminology here. The bookmark is called "Peers" and we use "node" here. RPCs are getpeerinfo, addnode. Oops. But at least GUI should be consistent.

@luke-jr
Member
luke-jr commented Oct 4, 2016

Our peers are nodes. Different terms for different things. Peer is the relationship, node is the thing.

Truncating the menu items does seem like the simplest approach here, though.

@achow101
Contributor
achow101 commented Oct 4, 2016

I've just removed Node so that it says Disconnect, Ban for .., and Unban

@laanwj
Member
laanwj commented Oct 5, 2016

Probably need #8885 in first to be able to test this re:banning.

@laanwj
Member
laanwj commented Oct 5, 2016

Have tested d651056
I've been unable to ban/kick multiple peers. Either with Ctrl-click or Shift-click. Is there some other trick?
Ubuntu 16.04, Qt reports as 5.5.1.
It seems like it allows multiple selection (e.g. two nodes can be selected for a fraction of a second) but always ends up selecting the last clicked node. Maybe some rogue event handler?

@achow101
Contributor
achow101 commented Oct 5, 2016

Click, hold, and move your mouse across multiple rows. That's what works. I haven't figured out Ctrl+click or Shift+click yet.

@achow101
Contributor
achow101 commented Oct 5, 2016

@laanwj 763cc02 should allow for multiple selection with Ctrl-click, shift-click, and click n' drag.

@jonasschnelli
Member

Needs rebase.

@achow101
Contributor
achow101 commented Oct 8, 2016

rebased

@jonasschnelli

Tested a bit. Selecting multiple peers (with shift key or with the mouse) results in some of them getting deselected short after the selection.

src/qt/rpcconsole.cpp
+ if(!LookupHost(addr.c_str(), resolved, false))
+ continue;
+ g_connman->Ban(resolved, BanReasonManuallyAdded, bantime);
+ clearSelectedNode();
@jonasschnelli
jonasschnelli Oct 9, 2016 Member

I think the clearSelectedNode(); and L1008 can be moved out of the for loop

@achow101
achow101 Oct 9, 2016 Contributor

Done

@achow101
Contributor
achow101 commented Oct 9, 2016

I'm not sure why some of them get deselected. I noticed that too, but I can't figure out a reason for why that happens.

@rebroad
Contributor
rebroad commented Oct 19, 2016

Can we use keyboard shortcuts for selecting as per GUI standards? I.e. hold down shift while using the a cursor up/down to select multiple, or using spacebar and cursor up/down to toggle whether a node is selected or not (once at least one node is selected)?

@achow101
Contributor

@rebroad It's whatever QAbstractItemView::ExtendedSelection specifies

@luke-jr luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Oct 20, 2016
@achow101 @luke-jr achow101 + luke-jr Multiple Selection for peer and ban tables
Allows multiple selection and action for the nodes in the peer and ban tables in the Debug Window.

Github-Pull: #8874
Rebased-From: db74962
7ec15f7
@jonasschnelli
Member

@achow101: Are you planing to finalizing this? IMO there are still the issue with auto-deselecting rows.

@achow101
Contributor
achow101 commented Nov 8, 2016

@jonasschnelli Yes, I do plan on finishing this. However, I am having trouble with figuring out why there is auto-deselecting.

@achow101 achow101 Multiple Selection for peer and ban tables
Allows multiple selection and action for the nodes in the peer and ban tables in the Debug Window.
addfdeb
@achow101
Contributor
achow101 commented Nov 8, 2016

rebased.

I think I figured out why it was auto-deselecting. It loses the selection after the model is refresehed.

@achow101
Contributor
achow101 commented Nov 8, 2016

I think I fixed the problem.

@achow101
Contributor
achow101 commented Nov 8, 2016

Actually fixed it this time.

@paveljanik
Contributor

ACK 1077577

@laanwj
Member
laanwj commented Nov 9, 2016

Seems to work well enough now. If there are still minor issues with selection they can be fixed later.
ACK 1077577

@laanwj laanwj merged commit 1077577 into bitcoin:master Nov 9, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laanwj laanwj added a commit that referenced this pull request Nov 9, 2016
@laanwj laanwj Merge #8874: Multiple Selection for peer and ban tables
1077577 Fix auto-deselection of peers (Andrew Chow)
addfdeb Multiple Selection for peer and ban tables (Andrew Chow)
e984730
@jonasschnelli
Member

Post merge ACK.

@luke-jr luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 21, 2016
@achow101 @luke-jr achow101 + luke-jr Multiple Selection for peer and ban tables
Allows multiple selection and action for the nodes in the peer and ban tables in the Debug Window.

Github-Pull: #8874
Rebased-From: addfdeb
b109232
@luke-jr luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 21, 2016
@achow101 @luke-jr achow101 + luke-jr Fix auto-deselection of peers
Github-Pull: #8874
Rebased-From: 1077577
4b8ec55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment