qt: Add Copy Address Action to Payment Requests #213

pull jarolrod wants to merge 1 commits into bitcoin-core:master from jarolrod:add-copyaddress-requestedpayments changing 2 files +17 −0

jarolrod commented at 1:38 AM on February 19, 2021: member

Currently, the only way to copy the address of a payment request is to double-click on the payment request and then click on the copy address button. This PR adds a convenient context menu action to copy the address of a payment request.

Master	PR
<img width="169" alt="Screen Shot 2021-02-18 at 8 33 08 PM" src="https://user-images.githubusercontent.com/23396902/108444489-b6703f80-7228-11eb-8684-945fbcd04772.png">	<img width="169" alt="Screen Shot 2021-02-18 at 8 33 50 PM" src="https://user-images.githubusercontent.com/23396902/108444505-c12ad480-7228-11eb-9eee-473fee877ad7.png">

in src/qt/receivecoinsdialog.cpp:295 in 405215f015 outdated

 288 | @@ -286,6 +289,19 @@ void ReceiveCoinsDialog::copyURI()
 289 |      GUIUtil::setClipboard(uri);
 290 |  }
 291 |  
 292 | +// context menu action: copy address
 293 | +void ReceiveCoinsDialog::copyAddress()
 294 | +{
 295 | +    QModelIndex sel = selectedRow();

Talkless commented at 5:30 PM on February 20, 2021:

nit: sel could be const

jarolrod commented at 1:24 AM on February 22, 2021:

addressed in 24fcd97

Talkless changes_requested
jarolrod force-pushed on Feb 22, 2021
jarolrod commented at 1:25 AM on February 22, 2021: member
Updated from 405215f -> 24fcd97, addressed @Talkless suggestion

Changes:
- sel is now const
hebasto commented at 4:00 PM on February 22, 2021: member

Concept ACK. I've noted the same inconvenience :)
hebasto commented at 4:11 PM on February 22, 2021: member

@jarolrod

This pr, which is submitted on 2021-02-19, is based on the branch commit from 2020-11-05. While it has no ACKs, maybe rebase it to eliminate CI glitches?
in src/qt/receivecoinsdialog.cpp:293 in 24fcd97ffb outdated
```
 288 | @@ -286,6 +289,19 @@ void ReceiveCoinsDialog::copyURI()
 289 |      GUIUtil::setClipboard(uri);
 290 |  }
 291 |  
 292 | +// context menu action: copy address
```
hebasto commented at 4:14 PM on February 22, 2021:

I understand that this comment follows the surrounding code style. But isn't it worth to add a Doxygen comment into the header instead of such an approach?

UPDATE: this slot is private, so feel free to ignore my comment.
jarolrod force-pushed on Feb 22, 2021
jarolrod commented at 4:23 PM on February 22, 2021: member

Updated from 24fcd97 -> 8871d9a

Changes: Rebased to a more recent master branch in order to avoid CI glitches on @hebasto recommendation
in src/qt/receivecoinsdialog.cpp:301 in 8871d9a6a9 outdated
```
 296 | +    const QModelIndex sel = selectedRow();
 297 | +    if (!sel.isValid()) {
 298 | +        return;
 299 | +    }
 300 | +
 301 | +    const RecentRequestsTableModel * const submodel = model->getRecentRequestsTableModel();
```
hebasto commented at 4:37 PM on February 22, 2021:

style nit (see clang-format-diff.py):

const RecentRequestsTableModel* const submodel = model->getRecentRequestsTableModel();

btw, why submodel variable is required at all? It does not improve readability, at least for me :) Maybe just

const QString address = model->getRecentRequestsTableModel()->entry(sel.row()).recipient.address;

This line could be shorter then 100 characters, if s/sel/r

Feel free to ignore the second part of this comment.
jarolrod commented at 5:04 AM on February 23, 2021:

addressed the first half of your comment in e348d7e

in src/qt/receivecoinsdialog.cpp:47 in 8871d9a6a9 outdated

  43 | @@ -44,20 +44,23 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid
  44 |  
  45 |      // context menu actions
  46 |      QAction *copyURIAction = new QAction(tr("Copy URI"), this);
  47 | +    QAction *copyAddressAction = new QAction(tr("Copy address"), this);

hebasto commented at 4:38 PM on February 22, 2021:

style nit (see clang-format-diff.py):

    QAction* copyAddressAction = new QAction(tr("Copy address"), this);

jarolrod commented at 5:03 AM on February 23, 2021:

addressed in e348d7e

hebasto approved
hebasto commented at 4:43 PM on February 22, 2021: member

ACK 8871d9a6a98d44934c634ddfaa3e4de4e45092fc, tested on Linux Mint 20.1 (Qt 5.12.8).

qt: Add Copy Address Action to Payment Requests

Currently, the only way to copy the address of a payment request is to double-click on the payment request and then click on the copy address button. This commit adds a convenient context menu action to copy the address of a payment request.

e348d7ea2c

jarolrod force-pushed on Feb 23, 2021
jarolrod commented at 5:07 AM on February 23, 2021: member
Updated from 8871d9a -> e348d7e

Changes:
- Closer adherence to clang-format
hebasto approved
hebasto commented at 7:58 AM on February 23, 2021: member

re-ACK e348d7ea2c0b5d2eb8039dab33b0b9a48655885f, only suggested changes since my previous review.
MarcoFalke merged this on Feb 23, 2021
MarcoFalke closed this on Feb 23, 2021
sidhujag referenced this in commit e5e032bc08 on Feb 23, 2021
jarolrod deleted the branch on Feb 23, 2021
MarcoFalke referenced this in commit 09bc7bfed1 on Feb 25, 2021
PastaPastaPasta referenced this in commit a0bf9a6197 on Sep 17, 2021
PastaPastaPasta referenced this in commit 44a0d7ef1b on Sep 19, 2021
thelazier referenced this in commit df7a5a8ba9 on Sep 25, 2021
gwillen referenced this in commit 6cfd41becd on Jun 28, 2022
bitcoin-core locked this on Aug 16, 2022

Contributors

jarolrod

Talkless

hebasto

Linked (view graph)

#214 qt: Disable requests context menu actions when appropriate