[Qt] Display txfee in first sendCoinsDialog message box

laanwj commented at 6:09 pm on August 30, 2013: member

I’ve rebased pull request #2651 to payment request. This was a bit tricky as they affect the same code.

I’ve tried it a bit and it appears to work, but haven’t tested the interaction with payment request yet. @jonasschnelli, @diapolo can you please review/test this?

laanwj commented at 6:47 am on August 31, 2013: member

I’m following the payment request testplan: https://github.com/gavinandresen/QA/blob/master/PaymentRequestTest.md

“Unsigned payment request”

OK

“Signed payment request”

OK

“Signed payment request, multiple addresses”

Overview is correct, sent transactions are correct
Strange: no label is added for the second address when doing this. Not sure this is a regression though.

“PaymentACK message”

OK (paymentack received message displays)

“Expired payment request”

OK (payment request expired dialog pops up)

in src/qt/sendcoinsdialog.cpp: in fecfa1dffe outdated

 96@@ -97,9 +97,18 @@ void SendCoinsDialog::on_sendButton_clicked()
 97         QString amount = BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), rcp.amount);
 98         if (rcp.authenticatedMerchant.isEmpty())
 99         {
100-            QString address = rcp.address;
101-            QString to = GUIUtil::HtmlEscape(rcp.label);
102-            formatted.append(tr("<b>%1</b> to %2 (%3)").arg(amount, to, address));
103+            QString recipientElement = QString("<b>%1</b> ").arg(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, rcp.amount));

Diapolo commented at 9:12 am on August 31, 2013:

This should be changed to dynamic units via model->getOptionsModel()->getDisplayUnit(), remember my recent pull :).

jonasschnelli commented at 9:57 am on August 31, 2013:

Yes. We should use that. @laanwj should i takeover again and fix these things?

laanwj commented at 10:25 am on August 31, 2013:

@jonasschnelli I’d rather just get this merged ASAP and leave improvements to later. So if there’s nothing breaking or obviously incorrect I’d like to get ACKs.

in src/qt/sendcoinsdialog.cpp: in fecfa1dffe outdated

182+
183+    if(txFee > 0)
184+    {
185+        // append fee string if a fee is required
186+        questionString.append("<hr /><span style='color:#aa0000;'>");
187+        questionString.append(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, txFee));

Diapolo commented at 9:12 am on August 31, 2013:

Same here model->getOptionsModel()->getDisplayUnit().

laanwj commented at 10:27 am on August 31, 2013:

Will fix these before merge.

in src/qt/sendcoinsdialog.cpp: in fecfa1dffe outdated

190+    }
191+    if(txFee > 0 || recipients.count() > 1)
192+    {
193+        // add total amount string if there are more then one recipients or a fee is required
194+        questionString.append("<hr />");
195+        questionString.append(tr("Total Amount %1").arg(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, currentTransaction.getTotalTransactionAmount()+txFee)));

Diapolo commented at 9:13 am on August 31, 2013:

Same here model->getOptionsModel()->getDisplayUnit().

in src/qt/sendcoinsdialog.cpp: in fecfa1dffe outdated

103+            QString recipientElement = QString("<b>%1</b> ").arg(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, rcp.amount));
104+            recipientElement.append(tr("to"));
105+
106+            if(rcp.label.length() > 0)
107+            {
108+                recipientElement.append(QString(" %1 <span style='font-size:8px;'>%2</span><br />").arg(GUIUtil::HtmlEscape(rcp.label), rcp.address)); // add address with label

Diapolo commented at 9:15 am on August 31, 2013:

I’m not sure if we should use fixed font-sizes.

jonasschnelli commented at 9:58 am on August 31, 2013:

Hmm… i would keep the current font. Even with fixed font’s line breaks are possible. I also think fixed fonts can look really bad.

laanwj commented at 10:24 am on August 31, 2013:

This was also in @jonasschnelli’s pull, which I just rebased. We can keep font improvements for a future pull.

in src/qt/walletmodel.cpp: in fecfa1dffe outdated

247-                wtx.vOrderForm.push_back(make_pair(key, value));
248+                newTx->vOrderForm.push_back(make_pair(key, value));
249             }
250-        }        
251-
252-        if(!uiInterface.ThreadSafeAskFee(nFeeRequired))

Diapolo commented at 9:18 am on August 31, 2013:

Is this still used anywhere after this pull?

laanwj commented at 10:23 am on August 31, 2013:

I think it’s still used when you submit transactions over RPC.

qt: Display txfee in first sendCoinsDialog message box

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

9e8904f6ae

Diapolo commented at 11:33 am on August 31, 2013: none

I didn’t try to compile, but you can get a looked-at-the-code ACK. We should merge and improve the rest afterwards.

BitcoinPullTester commented at 1:37 pm on September 1, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/9e8904f6aeb6e98b4781fcf408b0a2bee550051d 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.

laanwj referenced this in commit f4f048ee58 on Sep 2, 2013

laanwj merged this on Sep 2, 2013

laanwj closed this on Sep 2, 2013

laanwj deleted the branch on Apr 9, 2014

Bushstar referenced this in commit 05adda99fe on Apr 8, 2020

DrahtBot locked this on Sep 8, 2021

[Qt] Display txfee in first sendCoinsDialog message box #2958