Merge bitcoin-core/gui#263: Revamp context menus

16c157d qt, refactor: Use better QMenu::addAction overloaded function (Hennadii Stepanov)
7931175 qt: Do not assign Alt+<KEY> shortcuts to context menu actions (Hennadii Stepanov)
963e120 qt: Drop menu separator that separates nothing (Hennadii Stepanov)
1398a65 qt, refactor: Make AddressBookPage::deleteAction a local variable (Hennadii Stepanov)

Pull request description:

  This PR:
  1. removes useless `Alt` + `<KEY>` shortcuts from context menu items
  2. replaces 3 lines of code with the only call of [`QMenu::addAction`](https://doc.qt.io/qt-5/qmenu.html#addAction-5) for each context menu item (it became possible since bitcoin/bitcoin#21286 was merged)
  3. makes other minor cleanups

  No behavior change.

ACKs for top commit:
  kristapsk:
    ACK 16c157d
  promag:
    Code review ACK 16c157d. Nice code cleanup that takes advantage of more recent Qt API.
  jarolrod:
    ACK 16c157d

Tree-SHA512: e5555fe957058cc67b351aaf9f09fe3635edb2d07a2223d3093913a25607ae538f0a2fde84c0b0cd43e7475b248949548eb4a5d4b21d8f7391fa2fa8541c04ff

src/qt/addressbookpage.cpp

@@ -112,29 +112,17 @@ AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode,
  break;
  }
 
- // Context menu actions
- QAction *copyAddressAction = new QAction(tr("&Copy Address"), this);
- QAction *copyLabelAction = new QAction(tr("Copy &Label"), this);
- QAction *editAction = new QAction(tr("&Edit"), this);
- deleteAction = new QAction(ui->deleteAddress->text(), this);
-
  // Build context menu
  contextMenu = new QMenu(this);
- contextMenu->addAction(copyAddressAction);
- contextMenu->addAction(copyLabelAction);
- contextMenu->addAction(editAction);
- if(tab == SendingTab)
- contextMenu->addAction(deleteAction);
- contextMenu->addSeparator();
-
- // Connect signals for context menu actions
- connect(copyAddressAction, &QAction::triggered, this, &AddressBookPage::on_copyAddress_clicked);
- connect(copyLabelAction, &QAction::triggered, this, &AddressBookPage::onCopyLabelAction);
- connect(editAction, &QAction::triggered, this, &AddressBookPage::onEditAction);
- connect(deleteAction, &QAction::triggered, this, &AddressBookPage::on_deleteAddress_clicked);
+ contextMenu->addAction(tr("Copy Address"), this, &AddressBookPage::on_copyAddress_clicked);
+ contextMenu->addAction(tr("Copy Label"), this, &AddressBookPage::onCopyLabelAction);
+ contextMenu->addAction(tr("Edit"), this, &AddressBookPage::onEditAction);
 
- connect(ui->tableView, &QWidget::customContextMenuRequested, this, &AddressBookPage::contextualMenu);
+ if (tab == SendingTab) {
+ contextMenu->addAction(tr("Delete"), this, &AddressBookPage::on_deleteAddress_clicked);
+ }
 
+ connect(ui->tableView, &QWidget::customContextMenuRequested, this, &AddressBookPage::contextualMenu);
  connect(ui->closeButton, &QPushButton::clicked, this, &QDialog::accept);
 
  GUIUtil::handleCloseWindowShortcut(this);
@@ -249,13 +237,11 @@ void AddressBookPage::selectionChanged()
  // In sending tab, allow deletion of selection
  ui->deleteAddress->setEnabled(true);
  ui->deleteAddress->setVisible(true);
- deleteAction->setEnabled(true);
  break;
  case ReceivingTab:
  // Deleting receiving addresses, however, is not allowed
  ui->deleteAddress->setEnabled(false);
  ui->deleteAddress->setVisible(false);
- deleteAction->setEnabled(false);
  break;
  }
  ui->copyAddress->setEnabled(true);

src/qt/addressbookpage.h

@@ -55,7 +55,6 @@ public Q_SLOTS:
  QString returnValue;
  AddressBookSortFilterProxyModel *proxyModel;
  QMenu *contextMenu;
- QAction *deleteAction; // to be able to explicitly disable it
  QString newAddressToSelect;
 
 private Q_SLOTS:

src/qt/bitcoingui.cpp

@@ -1468,11 +1468,8 @@ void UnitDisplayStatusBarControl::mousePressEvent(QMouseEvent *event)
 void UnitDisplayStatusBarControl::createContextMenu()
 {
  menu = new QMenu(this);
- for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits())
- {
- QAction *menuAction = new QAction(QString(BitcoinUnits::longName(u)), this);
- menuAction->setData(QVariant(u));
- menu->addAction(menuAction);
+ for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) {
+ menu->addAction(BitcoinUnits::longName(u))->setData(QVariant(u));
  }
  connect(menu, &QMenu::triggered, this, &UnitDisplayStatusBarControl::onMenuSelection);
 }

src/qt/coincontroldialog.cpp

@@ -50,32 +50,16 @@ CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _m
 {
  ui->setupUi(this);
 
- // context menu actions
- QAction *copyAddressAction = new QAction(tr("Copy address"), this);
- QAction *copyLabelAction = new QAction(tr("Copy label"), this);
- QAction *copyAmountAction = new QAction(tr("Copy amount"), this);
- copyTransactionHashAction = new QAction(tr("Copy transaction ID"), this); // we need to enable/disable this
- lockAction = new QAction(tr("Lock unspent"), this); // we need to enable/disable this
- unlockAction = new QAction(tr("Unlock unspent"), this); // we need to enable/disable this
-
  // context menu
  contextMenu = new QMenu(this);
- contextMenu->addAction(copyAddressAction);
- contextMenu->addAction(copyLabelAction);
- contextMenu->addAction(copyAmountAction);
- contextMenu->addAction(copyTransactionHashAction);
+ contextMenu->addAction(tr("Copy address"), this, &CoinControlDialog::copyAddress);
+ contextMenu->addAction(tr("Copy label"), this, &CoinControlDialog::copyLabel);
+ contextMenu->addAction(tr("Copy amount"), this, &CoinControlDialog::copyAmount);
+ copyTransactionHashAction = contextMenu->addAction(tr("Copy transaction ID"), this, &CoinControlDialog::copyTransactionHash);
  contextMenu->addSeparator();
- contextMenu->addAction(lockAction);
- contextMenu->addAction(unlockAction);
-
- // context menu signals
+ lockAction = contextMenu->addAction(tr("Lock unspent"), this, &CoinControlDialog::lockCoin);
+ unlockAction = contextMenu->addAction(tr("Unlock unspent"), this, &CoinControlDialog::unlockCoin);
  connect(ui->treeWidget, &QWidget::customContextMenuRequested, this, &CoinControlDialog::showMenu);
- connect(copyAddressAction, &QAction::triggered, this, &CoinControlDialog::copyAddress);
- connect(copyLabelAction, &QAction::triggered, this, &CoinControlDialog::copyLabel);
- connect(copyAmountAction, &QAction::triggered, this, &CoinControlDialog::copyAmount);
- connect(copyTransactionHashAction, &QAction::triggered, this, &CoinControlDialog::copyTransactionHash);
- connect(lockAction, &QAction::triggered, this, &CoinControlDialog::lockCoin);
- connect(unlockAction, &QAction::triggered, this, &CoinControlDialog::unlockCoin);
 
  // clipboard actions
  QAction *clipboardQuantityAction = new QAction(tr("Copy quantity"), this);

src/qt/qrimagewidget.cpp

@@ -27,12 +27,8 @@ QRImageWidget::QRImageWidget(QWidget *parent):
  QLabel(parent), contextMenu(nullptr)
 {
  contextMenu = new QMenu(this);
- QAction *saveImageAction = new QAction(tr("&Save Image..."), this);
- connect(saveImageAction, &QAction::triggered, this, &QRImageWidget::saveImage);
- contextMenu->addAction(saveImageAction);
- QAction *copyImageAction = new QAction(tr("&Copy Image"), this);
- connect(copyImageAction, &QAction::triggered, this, &QRImageWidget::copyImage);
- contextMenu->addAction(copyImageAction);
+ contextMenu->addAction(tr("Save Image..."), this, &QRImageWidget::saveImage);
+ contextMenu->addAction(tr("Copy Image"), this, &QRImageWidget::copyImage);
 }
 
 bool QRImageWidget::setQR(const QString& data, const QString& text)

src/qt/receivecoinsdialog.cpp

@@ -42,28 +42,14 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid
  ui->removeRequestButton->setIcon(_platformStyle->SingleColorIcon(":/icons/remove"));
  }
 
- // context menu actions
- QAction *copyURIAction = new QAction(tr("Copy URI"), this);
- QAction* copyAddressAction = new QAction(tr("Copy address"), this);
- copyLabelAction = new QAction(tr("Copy label"), this);
- copyMessageAction = new QAction(tr("Copy message"), this);
- copyAmountAction = new QAction(tr("Copy amount"), this);
-
  // context menu
  contextMenu = new QMenu(this);
- contextMenu->addAction(copyURIAction);
- contextMenu->addAction(copyAddressAction);
- contextMenu->addAction(copyLabelAction);
- contextMenu->addAction(copyMessageAction);
- contextMenu->addAction(copyAmountAction);
-
- // context menu signals
+ contextMenu->addAction(tr("Copy URI"), this, &ReceiveCoinsDialog::copyURI);
+ contextMenu->addAction(tr("Copy address"), this, &ReceiveCoinsDialog::copyAddress);
+ copyLabelAction = contextMenu->addAction(tr("Copy label"), this, &ReceiveCoinsDialog::copyLabel);
+ copyMessageAction = contextMenu->addAction(tr("Copy message"), this, &ReceiveCoinsDialog::copyMessage);
+ copyAmountAction = contextMenu->addAction(tr("Copy amount"), this, &ReceiveCoinsDialog::copyAmount);
  connect(ui->recentRequestsView, &QWidget::customContextMenuRequested, this, &ReceiveCoinsDialog::showMenu);
- connect(copyURIAction, &QAction::triggered, this, &ReceiveCoinsDialog::copyURI);
- connect(copyAddressAction, &QAction::triggered, this, &ReceiveCoinsDialog::copyAddress);
- connect(copyLabelAction, &QAction::triggered, this, &ReceiveCoinsDialog::copyLabel);
- connect(copyMessageAction, &QAction::triggered, this, &ReceiveCoinsDialog::copyMessage);
- connect(copyAmountAction, &QAction::triggered, this, &ReceiveCoinsDialog::copyAmount);
 
  connect(ui->clearButton, &QPushButton::clicked, this, &ReceiveCoinsDialog::clear);
 

src/qt/rpcconsole.cpp

@@ -616,29 +616,14 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
  ui->peerWidget->setColumnWidth(PeerTableModel::Ping, PING_COLUMN_WIDTH);
  ui->peerWidget->horizontalHeader()->setStretchLastSection(true);
 
- // create peer table context menu actions
- QAction* disconnectAction = new QAction(tr("&Disconnect"), this);
- QAction* banAction1h = new QAction(ts.ban_for + " " + tr("1 &hour"), this);
- QAction* banAction24h = new QAction(ts.ban_for + " " + tr("1 &day"), this);
- QAction* banAction7d = new QAction(ts.ban_for + " " + tr("1 &week"), this);
- QAction* banAction365d = new QAction(ts.ban_for + " " + tr("1 &year"), this);
-
  // create peer table context menu
  peersTableContextMenu = new QMenu(this);
- peersTableContextMenu->addAction(disconnectAction);
- peersTableContextMenu->addAction(banAction1h);
- peersTableContextMenu->addAction(banAction24h);
- peersTableContextMenu->addAction(banAction7d);
- peersTableContextMenu->addAction(banAction365d);
-
- connect(banAction1h, &QAction::triggered, [this] { banSelectedNode(60 * 60); });
- connect(banAction24h, &QAction::triggered, [this] { banSelectedNode(60 * 60 * 24); });
- connect(banAction7d, &QAction::triggered, [this] { banSelectedNode(60 * 60 * 24 * 7); });
- connect(banAction365d, &QAction::triggered, [this] { banSelectedNode(60 * 60 * 24 * 365); });
-
- // peer table context menu signals
+ peersTableContextMenu->addAction(tr("Disconnect"), this, &RPCConsole::disconnectSelectedNode);
+ peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 hour"), [this] { banSelectedNode(60 * 60); });
+ peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 day"), [this] { banSelectedNode(60 * 60 * 24); });
+ peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 week"), [this] { banSelectedNode(60 * 60 * 24 * 7); });
+ peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 year"), [this] { banSelectedNode(60 * 60 * 24 * 365); });
  connect(ui->peerWidget, &QTableView::customContextMenuRequested, this, &RPCConsole::showPeersTableContextMenu);
- connect(disconnectAction, &QAction::triggered, this, &RPCConsole::disconnectSelectedNode);
 
  // peer table signal handling - update peer details when selecting new node
  connect(ui->peerWidget->selectionModel(), &QItemSelectionModel::selectionChanged, this, &RPCConsole::updateDetailWidget);
@@ -657,16 +642,10 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
  ui->banlistWidget->setColumnWidth(BanTableModel::Bantime, BANTIME_COLUMN_WIDTH);
  ui->banlistWidget->horizontalHeader()->setStretchLastSection(true);
 
- // create ban table context menu action
- QAction* unbanAction = new QAction(tr("&Unban"), this);
-
  // create ban table context menu
  banTableContextMenu = new QMenu(this);
- banTableContextMenu->addAction(unbanAction);
-
- // ban table context menu signals
+ banTableContextMenu->addAction(tr("Unban"), this, &RPCConsole::unbanSelectedNode);
  connect(ui->banlistWidget, &QTableView::customContextMenuRequested, this, &RPCConsole::showBanTableContextMenu);
- connect(unbanAction, &QAction::triggered, this, &RPCConsole::unbanSelectedNode);
 
  // ban table signal handling - clear peer details when clicking a peer in the ban table
  connect(ui->banlistWidget, &QTableView::clicked, this, &RPCConsole::clearSelectedNode);

src/qt/transactionview.cpp

@@ -161,32 +161,21 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa
  transactionView->horizontalHeader()->setStretchLastSection(true);
  }
 
- // Actions
- abandonAction = new QAction(tr("Abandon transaction"), this);
- bumpFeeAction = new QAction(tr("Increase transaction fee"), this);
- bumpFeeAction->setObjectName("bumpFeeAction");
- copyAddressAction = new QAction(tr("Copy address"), this);
- copyLabelAction = new QAction(tr("Copy label"), this);
- QAction *copyAmountAction = new QAction(tr("Copy amount"), this);
- QAction *copyTxIDAction = new QAction(tr("Copy transaction ID"), this);
- QAction *copyTxHexAction = new QAction(tr("Copy raw transaction"), this);
- QAction *copyTxPlainText = new QAction(tr("Copy full transaction details"), this);
- QAction *editLabelAction = new QAction(tr("Edit address label"), this);
- QAction *showDetailsAction = new QAction(tr("Show transaction details"), this);
-
  contextMenu = new QMenu(this);
  contextMenu->setObjectName("contextMenu");
- contextMenu->addAction(copyAddressAction);
- contextMenu->addAction(copyLabelAction);
- contextMenu->addAction(copyAmountAction);
- contextMenu->addAction(copyTxIDAction);
- contextMenu->addAction(copyTxHexAction);
- contextMenu->addAction(copyTxPlainText);
- contextMenu->addAction(showDetailsAction);
+ copyAddressAction = contextMenu->addAction(tr("Copy address"), this, &TransactionView::copyAddress);
+ copyLabelAction = contextMenu->addAction(tr("Copy label"), this, &TransactionView::copyLabel);
+ contextMenu->addAction(tr("Copy amount"), this, &TransactionView::copyAmount);
+ contextMenu->addAction(tr("Copy transaction ID"), this, &TransactionView::copyTxID);
+ contextMenu->addAction(tr("Copy raw transaction"), this, &TransactionView::copyTxHex);
+ contextMenu->addAction(tr("Copy full transaction details"), this, &TransactionView::copyTxPlainText);
+ contextMenu->addAction(tr("Show transaction details"), this, &TransactionView::showDetails);
  contextMenu->addSeparator();
- contextMenu->addAction(bumpFeeAction);
- contextMenu->addAction(abandonAction);
- contextMenu->addAction(editLabelAction);
+ bumpFeeAction = contextMenu->addAction(tr("Increase transaction fee"));
+ GUIUtil::ExceptionSafeConnect(bumpFeeAction, &QAction::triggered, this, &TransactionView::bumpFee);
+ bumpFeeAction->setObjectName("bumpFeeAction");
+ abandonAction = contextMenu->addAction(tr("Abandon transaction"), this, &TransactionView::abandonTx);
+ contextMenu->addAction(tr("Edit address label"), this, &TransactionView::editLabel);
 
  connect(dateWidget, static_cast<void (QComboBox::*)(int)>(&QComboBox::activated), this, &TransactionView::chooseDate);
  connect(typeWidget, static_cast<void (QComboBox::*)(int)>(&QComboBox::activated), this, &TransactionView::chooseType);
@@ -199,16 +188,6 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa
  connect(transactionView, &QTableView::doubleClicked, this, &TransactionView::doubleClicked);
  connect(transactionView, &QTableView::customContextMenuRequested, this, &TransactionView::contextualMenu);
 
- GUIUtil::ExceptionSafeConnect(bumpFeeAction, &QAction::triggered, this, &TransactionView::bumpFee);
- connect(abandonAction, &QAction::triggered, this, &TransactionView::abandonTx);
- connect(copyAddressAction, &QAction::triggered, this, &TransactionView::copyAddress);
- connect(copyLabelAction, &QAction::triggered, this, &TransactionView::copyLabel);
- connect(copyAmountAction, &QAction::triggered, this, &TransactionView::copyAmount);
- connect(copyTxIDAction, &QAction::triggered, this, &TransactionView::copyTxID);
- connect(copyTxHexAction, &QAction::triggered, this, &TransactionView::copyTxHex);
- connect(copyTxPlainText, &QAction::triggered, this, &TransactionView::copyTxPlainText);
- connect(editLabelAction, &QAction::triggered, this, &TransactionView::editLabel);
- connect(showDetailsAction, &QAction::triggered, this, &TransactionView::showDetails);
  // Double-clicking on a transaction on the transaction history page shows details
  connect(this, &TransactionView::doubleClicked, this, &TransactionView::showDetails);
  // Highlight transaction after fee bump