qt: improve "receive coins" workflow #3099

laanwj commented at 2:14 PM on October 16, 2013: member

This is a work in progress to improve the workflow for receive coins and fix #2429.

New receive tab: example1

After clicking "Request payment" a new receiving address is created (except when "Reuse ..." is checked, in which case the user can select an existing one). Then the following window is shown:

newreq

When compiling without QR codes, the dialog is the same apart from the QR code at the top.

I'm thinking of putting address/amount/label/message in a single text area to make it easier to copy/paste the entire payment request to the recipient (for example into a mail), but this is the general idea.

I've also simplified the interface by removing the "addresses" tab and instead moved command for getting a list of used receiving and sending addresses to menu menu

Any testing is welcome. Known currently broken:

Sign/verify message from address list

in src/qt/addressbookpage.cpp:None in ff1444af88 outdated

  44 |      {
  45 |      case ForSending:
  46 | +        switch(tab)
  47 | +        {
  48 | +        case SendingTab: setWindowTitle(tr("Choose the address to send coins to")); break;
  49 | +        case ReceivingTab: setWindowTitle(tr("Choose the address to receive coins to")); break;

Diapolo commented at 2:56 PM on October 16, 2013:

Perhaps replace to with with here?

Diapolo commented at 2:58 PM on October 16, 2013: none

Can you give a screen how it looks when clicking on Used sending/receiving addresses...?

in src/qt/receivecoinsdialog.h:None in ff1444af88 outdated

   8 | +    class ReceiveCoinsDialog;
   9 | +}
  10 | +class WalletModel;
  11 | +class OptionsModel;
  12 | +
  13 | +QT_BEGIN_NAMESPACE

Diapolo commented at 3:04 PM on October 16, 2013:

Is this block needed? Can't seem to find any URL stuff yet.

laanwj commented at 3:04 PM on October 16, 2013: member

Same as before really, addresses Edit: send/verify message has been removed

in src/qt/walletstack.cpp:None in ff1444af88 outdated

 154 | @@ -162,6 +155,20 @@ void WalletStack::setEncryptionStatus()
 155 |          walletView->setEncryptionStatus();
 156 |  }
 157 |  
 158 | +void WalletStack::usedSendingAddresses()
 159 | +{
 160 | +    WalletView *walletView = (WalletView*)currentWidget();

Diapolo commented at 3:05 PM on October 16, 2013:

This currently removes the ability for multiple WalletView objects, by intent?

laanwj commented at 3:27 PM on October 16, 2013:

How does it remove that? It casts the currently selected widget (in the stack) to a WalletView, and is exactly how the other functions in waletstack.cpp work.

Diapolo commented at 8:13 PM on October 16, 2013:

I was looking at WalletStack::gotoAddressBookPage(), which is different ;).

laanwj commented at 5:44 AM on October 17, 2013:

BTW we should really get rid of either WalletStack or WalletFrame some day, apart from the signal passthrough stuff the classes are essentially empty. Not in this pull though.

Diapolo commented at 2:22 PM on October 17, 2013:

Agreed, I never understood, why we integrated it that way, was hard enough to fix basic stuff after this was merged ^^.

Diapolo commented at 3:07 PM on October 16, 2013: none

Didn't test, but looks very nice :). Hope you re-enable sign/verify message links and all is fine.

mikehearn commented at 2:04 PM on October 17, 2013: contributor

If it's ready only, it's kinda weird to have widgets that look editable for the info.

Also, this dialog would be a great place to save a PaymentRequest message to a file! Although the payment protocol is probably going to be used mostly by servers, being able to save to a file and/or drag/drop into an email/IM window would be a nice step forward as well, because the payment protocol is not only for web servers and online shops but is useful for everyone.

Diapolo commented at 2:23 PM on October 17, 2013: none

At least for copy&paste reasons text from that window should be selectable, dunno if there is a way to make clear UI-wise it's read-only.

laanwj commented at 4:29 PM on October 17, 2013: member

@mikehearn Agreed on generating a payment request. That was one of my ideas behind this, but I'll leave implementing that to a later pull.

As for the fields, yes it's not ideal like this. I'd like to make it a text that can be copy/pasted in one go, not multiple fields. And as @Diapolo says there is not really a way to show readonly-ness in Qt, beyond making the field disabled but that's ugly.

laanwj commented at 6:55 AM on October 18, 2013: member

@diapolo I think I'm going to remove the sign/verify message buttons from the address lists. After this, they can be launched from the menu directly more quickly. And confusion such as in #3105 will no longer happen then.

Edit: I also want to make it possible to copy/paste or drag/drop the QR code. Hopefully that's just a matter of enabling the right flags.

Diapolo commented at 9:19 AM on October 18, 2013: none

@laanwj I'm fine with removing them.

laanwj commented at 11:13 AM on October 18, 2013: member

I've added a context menu to QRcode, and it now supports dragging

qrcode_context

As it can all be done through the context menu (which people expect from saving images in browsers) I'm not sure whether to keep the "Save Image..." button (in that case, I'll need to add a "Copy Image" button too, and having a button is probably more explicit/clear). Any opinions?

laanwj commented at 12:16 PM on October 18, 2013: member

I've made the URI look like an URI and added a button to copy it. Also I've merged everything into one HTML area to simplify the dialog and make it possible to copy/paste everything at once.

newreq

gmaxwell commented at 4:21 PM on October 18, 2013: contributor

Whats the rational for even having the reuse box? The only thing I could come up with was trying to avoid adding state to a wallet and breaking backups, but change is going to do that silently in any case (and if we're going to worry about that we should probably just do a fast determinstization of the wallet)

laanwj commented at 4:41 PM on October 18, 2013: member

I can't really imagine either what it's useful for. But I'm sure people will complain if we actually make it impossible. We could go even further to obscure the checkbox, but removing the feature completely is a step too far (maybe in 0.11 :-).

gmaxwell commented at 4:49 PM on October 18, 2013: contributor

@laanwj Really? Hm.

I think the thing people will miss is not that, but they'll want a reasonably easy way to get a list of previously used addresses. E.g. "What address did I just give bob?" So why not add a request history button/page/tab and then something to click on there to repeat/edit/etc one of the prior ones?

From the UI screenshots its not clear to me what I should do when I want an address to receive an unspecified amount, which is a common thing to do.

laanwj commented at 4:58 PM on October 18, 2013: member

Yes, a request history would be a good idea too. It could simply store the last N generated bitcoin URIs. I'm not going to add that in this pull, though.

If you want an address to receive an unspecified amount you can just leave the amount and other fields empty (except for possibly the label). It mentions on the receive page that all fields are optional, that's not a lie. I could add a "copy address" button to the dialog to make it easier to copy only the address.

Or alternatively go to the "Used received addresses..." and add a new receiving address directly. That's still possible like before.

gavinandresen commented at 3:04 AM on October 23, 2013: contributor

Nice! Compiles and runs on OSX, and I like the functionality.

qt: rework "receive coins" workflow 74fb765e29

qt: remove verify/send message and send coins from address book

These no longer make sense in the new workflow. It's less
clicks to reach sign/verify message from the menu. And sending
from the address book is one kind of automatic address reuse
we're trying to avoid.

864a6f790e

qt: allow dragging, copying and saving QR code

Add context menu and drag handling to QR code widget.

82095923bb

qt: add message field to SendCoinsRecipient

Also update URI parsing to fill in this field.
Note that the message is not currently used in any way with the client.
It should be stored with the transaction.

03535acd05

qt: use SendCoinsRecipient for payment request information

This brings some symmetry into the design by using the same object
both for incoming URIs that are parsed as for outgoing URIs that
are formatted.

8a7f37c797

qt: move bitcoin URI formatting to guiutil

Follow the same pattern as the parseBitcoinURI function.

786b066f03

qt: show payment information in one text area

Simplifies the dialog (makes it look less crowded) as well
as the code and makes it possible to copy multiple fields at once.
Also format bitcoin URI as URI, add copy button for URI.

33a2febf5d

in src/qt/forms/sendcoinsentry.ui:None in 9258399ffb outdated

 671 | @@ -672,7 +672,7 @@
 672 |   <customwidgets>
 673 |    <customwidget>
 674 |     <class>BitcoinAmountField</class>
 675 | -   <extends>QLineEdit</extends>
 676 | +   <extends>QWidget</extends>

Diapolo commented at 10:03 AM on October 23, 2013:

Did you or your Qt Creator / Qt Designer do this? Perhaps we should update all occurances of BitcoinAmountField be based on QWidget?

in src/qt/forms/sendcoinsentry.ui:None in 9258399ffb outdated

 638 | @@ -639,7 +639,7 @@
 639 |       </widget>
 640 |      </item>
 641 |      <item row="5" column="2">
 642 | -     <widget class="BitcoinAmountField" name="payAmount_s">
 643 | +     <widget class="BitcoinAmountField" name="payAmount_s" native="true">

Diapolo commented at 10:04 AM on October 23, 2013:

Wondering what native="true" means here, can you help?

laanwj commented at 10:27 AM on October 23, 2013:

I was actually trying to get it to substitute a qlineedit because that it more sensible in the editor (a qwidget is invisible, and qlineedit has appropriate properties like readonly). But it didn't stick somehow maybe I didn't try enough.

It only affects how it's shown in the editor. I don't know what the native attribute does.

BitcoinPullTester commented at 11:30 AM on October 23, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c0e322aea7872c3e4eecde227dfe7ccef852a78c for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

qt: general polish after adding new receive flow

- We no longer have an address book, but "address lists", update message
  accordingly
- Add tooltips here and there
- Clarify text on buttons
- add Copy Address button to receive request dialog

70b14636cd

laanwj referenced this in commit 081c0cdeb2 on Oct 25, 2013

laanwj merged this on Oct 25, 2013

laanwj closed this on Oct 25, 2013

laanwj referenced this in commit 7cd8623883 on Oct 25, 2013

laanwj referenced this in commit f04191eb55 on Oct 25, 2013

laanwj referenced this in commit 9a45a823f8 on Oct 27, 2013

laanwj deleted the branch on Apr 9, 2014

MathyV referenced this in commit 01f49879ce on Jul 31, 2014

MathyV referenced this in commit 479d6aa970 on Jul 31, 2014

MathyV referenced this in commit fd327a2105 on Jul 31, 2014

MathyV referenced this in commit ec4fd245d8 on Jul 31, 2014

MathyV referenced this in commit e3e83e13dc on Jul 31, 2014

MathyV referenced this in commit 1d3aca0146 on Aug 1, 2014

MathyV referenced this in commit ec4b6adbf1 on Aug 3, 2014

MathyV referenced this in commit bf823038ce on Aug 3, 2014

MathyV referenced this in commit 4c6d3a2d24 on Aug 3, 2014

MathyV referenced this in commit 159b6633e7 on Aug 5, 2014

MathyV referenced this in commit 1cca7148db on Aug 5, 2014

MathyV referenced this in commit ccbca09f78 on Aug 5, 2014

Bushstar referenced this in commit e2d651f601 on Apr 8, 2020

DrahtBot locked this on Sep 8, 2021