[Qt] polish new ban ui from #6315 #6710

Diapolo commented at 12:11 PM on September 22, 2015: none

No description provided.

in src/net.cpp:None in cc8f5d5d03 outdated

2491 | @@ -2492,6 +2492,9 @@ void DumpBanlist()
2492 |      CNode::GetBanned(banmap);
2493 |      bandb.Write(banmap);
2494 |  
2495 | +    // Notify UI about changed banlist
2496 | +    uiInterface.BannedListChanged();

jonasschnelli commented at 12:20 PM on September 22, 2015:

Hmm... emit the new ui signal from DumpBanlist() would result in multiple calling of banTableModel->refresh();. I'd prefer keeping this outside of the core classes (keep the call in the rpcnet.cpp methods). Or if you like to move it, you need to take care RPCConsole::banSelectedNode(int bantime), etc. to avoid multiple calls to the same method.

Diapolo commented at 12:34 PM on September 22, 2015:

DumpBanlist() -> BannedListChanged() -> ClientModel::updateBanlist() -> banTableModel->refresh()... where are the multiple calls?

jonasschnelli commented at 12:36 PM on September 22, 2015:

Right. You have updated the PR during my writing. Now it looks good.

in src/qt/rpcconsole.cpp:None in 698a04d2bb outdated

 355 | @@ -357,10 +356,10 @@ void RPCConsole::setClientModel(ClientModel *model)
 356 |  
 357 |          // create peer table context menu actions
 358 |          QAction* disconnectAction = new QAction(tr("&Disconnect Node"), this);
 359 | -        QAction* banAction1h      = new QAction(tr("Ban Node for") + " " + tr("1 &hour"), this);
 360 | -        QAction* banAction24h     = new QAction(tr("Ban Node for") + " " + tr("1 &day"), this);
 361 | -        QAction* banAction7d      = new QAction(tr("Ban Node for") + " " + tr("1 &week"), this);
 362 | -        QAction* banAction365d    = new QAction(tr("Ban Node for") + " " + tr("1 &year"), this);
 363 | +        QAction* banAction1h      = new QAction(tr("Ban Node for") + " " + tr("1 hour"), this);

jonasschnelli commented at 12:38 PM on September 22, 2015:

Wouldn't this remove the shortcut capabilities? Or where they never working in the context menu?

Diapolo commented at 12:42 PM on September 22, 2015:

If I remember correctly we had that discussion during the creation of the pull a few weeks ago, but you didn't want to add these changes :). AFAIK this isn't working in context menus.

MarcoFalke commented at 7:49 AM on September 23, 2015: member

Please edit the OP to say in one or two lines what will be improved by this PR. No need to copy paste the commits.

Diapolo commented at 10:57 AM on September 23, 2015: none

IMHO quicker to add the descriptive commit headers as to write something new ;).

laanwj added the label GUI on Sep 23, 2015

MarcoFalke commented at 8:50 AM on September 24, 2015: member

@Diapolo No need for duplicate content, imo. Just leave it empty?

Also, needs rebase.

[Qt] rework signal handling when banning/unbanning nodes

- remove the need for manually calling BanTableModel::refresh() in the GUI
  code by using BannedListChanged signal in DumpBanlist()

e93c2eb78f

[Qt] remove dup include of bantablemodel.h from rpcconsole.cpp 38d92ea790

[Qt] remove shortcuts for context menu while banning/unbanning nodes

- this is not used anywhere else in our code and also doesn't work at all

09165d3b78

[Qt, Trivial] remove unneeded/unused tags from rpcconsole.ui 575016dbd2

[Qt] make use of Q_FOREACH in bantablemodel.cpp

- allows an easy replacement when switching to C++11 and also matches code
  in peertablemodel.cpp

3b6d8edb5f

Diapolo commented at 7:16 AM on October 9, 2015: none

@MarcoFalke Rebased, redundant infos in pull-req removed. @laanwj Ping

jonasschnelli commented at 7:18 AM on October 9, 2015: contributor

utACK

laanwj commented at 10:05 AM on October 10, 2015: member

utACK apart from[Qt] rework signal handling when banning/unbanning nodes, which IMO does not make things better conceptually, see comment above.

Diapolo closed this on Oct 31, 2015

Diapolo deleted the branch on Oct 31, 2015

DrahtBot locked this on Sep 8, 2021