Revamp context menus #263

1. 
   hebasto commented at 2:44 am on March 30, 2021: member

   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 for each context menu item (it became possible since https://github.com/bitcoin/bitcoin/pull/21286 was merged)
   3. makes other minor cleanups

   No behavior change.

2. hebasto added the label Refactoring on Mar 30, 2021
3. DrahtBot added the label Needs rebase on Apr 14, 2021
4. qt, refactor: Make AddressBookPage::deleteAction a local variable 1398a6536c
5. qt: Drop menu separator that separates nothing 963e12058f
6. qt: Do not assign Alt+<KEY> shortcuts to context menu actions

   Such shortcuts are useless as pressing the Alt key closes a context menu
   widget immediately.

   79311750b5
7. qt, refactor: Use better QMenu::addAction overloaded function

   This overloaded function was introduced in Qt 5.6 and makes code more
   concise.

   16c157de3c
8. hebasto force-pushed on Apr 14, 2021
9. 
   hebasto commented at 4:03 pm on April 14, 2021: member

   Rebased b1ab2d1ff72fc92ff64b040c3df48b170e0edd28 -> 16c157de3c316517e095994fa8d526253225a672 (pr263.01 -> pr263.02) due to the conflict with #260.
10. DrahtBot removed the label Needs rebase on Apr 14, 2021
11. kristapsk approved
12. 
    kristapsk commented at 5:52 pm on April 14, 2021: contributor

    ACK 16c157de3c316517e095994fa8d526253225a672
13. in src/qt/addressbookpage.cpp:126 in 1398a6536c outdated

    123     contextMenu->addAction(copyAddressAction);
    124     contextMenu->addAction(copyLabelAction);
    125     contextMenu->addAction(editAction);
    126-    if(tab == SendingTab)
    127+    if (tab == SendingTab) {
    128+        QAction* deleteAction = new QAction(ui->deleteAddress->text(), this);
    

    ---

    ---

    

    promag commented at 11:19 am on April 17, 2021:

    1398a6536c710368d9f1d0cf6e280fe63d07c9f0

    With this change, the delete action only shows up on the sending tab where before it would be disabled in the receiving tab. I prefer this approach since the context menu is consistent for all addresses in each tab. I would prefer the other approach if the context menu actions were different on each row.

14. in src/qt/addressbookpage.cpp:138 in 963e12058f outdated

    133     connect(copyAddressAction, &QAction::triggered, this, &AddressBookPage::on_copyAddress_clicked);
    134     connect(copyLabelAction, &QAction::triggered, this, &AddressBookPage::onCopyLabelAction);
    135     connect(editAction, &QAction::triggered, this, &AddressBookPage::onEditAction);
    136 
    137     connect(ui->tableView, &QWidget::customContextMenuRequested, this, &AddressBookPage::contextualMenu);
    138-
    

    ---

    ---

    

    promag commented at 11:20 am on April 17, 2021:

    963e12058f3ca3cdaeefd9aa5a8305fa41afd1a0

    :eyes:

15. 
    promag commented at 11:24 am on April 17, 2021: contributor

    Code review ACK 16c157de3c316517e095994fa8d526253225a672. Nice code cleanup that takes advantage of more recent Qt API.
16. 
    MarcoFalke commented at 6:55 am on April 19, 2021: contributor

    @hebasto Ready for merge?
17. 
    hebasto commented at 7:29 am on April 19, 2021: member

    @jarolrod @Sjors Do you mind looking into this PR?
18. 
    jarolrod commented at 6:36 pm on April 19, 2021: member

    ACK 16c157de3c316517e095994fa8d526253225a672

    Tested that we maintain the same shortcuts between master and this PR and that the functionality is the same.

    The use of this overloaded addAction is correct and fits with our minimum supported Qt version: https://doc.qt.io/archives/qt-5.9/qmenu.html#addAction-4

    This is a nice simplification, the use of this overloaded function makes adding new context menu actions easier.

19. hebasto merged this on Apr 20, 2021
20. hebasto closed this on Apr 20, 2021

    ---

21. hebasto deleted the branch on Apr 20, 2021
22. sidhujag referenced this in commit 531653c855 on Apr 21, 2021
23. hebasto referenced this in commit 3f68f02db9 on Jun 14, 2021
24. gwillen referenced this in commit 7d4d030ccc on Jun 1, 2022
25. bitcoin-core locked this on Aug 16, 2022

Contributors
hebasto kristapsk promag MarcoFalke jarolrod

Labels
Refactoring