[Qt]: Improve sendcoinsdialog readability #13158

pull marcoagner wants to merge 1 commits into bitcoin:master from marcoagner:improve_sendcoinsdialog_readability changing 1 files +29 −22

marcoagner commented at 2:19 PM on May 3, 2018: contributor
I'm addressing two (probably duplicate) issues: #11606 and #10613.

Some points worth noting:
- I've tried to balance the proposed changes on both issues without going too far and remaining a bit conservative. It will be easier to improve based on suggestions where necessary.
- I preferred to maintain a layout that doesn't ask for an address truncation because, in my view, this wallet should be conservative on this.
- I didn't follow the idea of aligning the amounts to the right for finding it more natural (and minimalist) to read the information without having to map alignments. Additionally, that approach seems to need more <hr />'s (or similar) in order to help the user to map information, which ended up cluttering the box too much (specially with multiple recipients). Thus, I preferred to just give some more space between recipients. Let me know if there are better ideas on this.
Visually, I went from this (current):

To this:

As a side note, while doing this, I thought about a better way to show fees and found there's already a PR on this (https://github.com/bitcoin/bitcoin/pull/12189) and thought it is
fanquake added the label GUI on May 3, 2018
marcoagner renamed this:
~~[qt]: Improve sendcoinsdialog readability~~
[Qt]: Improve sendcoinsdialog readability
on May 3, 2018
MarcoFalke commented at 2:51 PM on May 3, 2018: member

Thanks for working on this. I haven't looked at the code, but the screenshots look good.
in src/qt/sendcoinsdialog.cpp:352 in e419cb0a2f outdated
```
 348 | @@ -337,19 +349,10 @@ void SendCoinsDialog::on_sendButton_clicked()
 349 | if(u != model->getOptionsModel()->getDisplayUnit())
 350 | alternativeUnits.append(BitcoinUnits::formatHtmlWithUnit(u, totalAmount));
 351 | }
 352 | - questionString.append(tr("Total Amount %1")
 353 | + questionString.append(tr("Total Amount: %1")
```
marcoagner commented at 3:04 PM on May 3, 2018:

This line may seem weird. But I made this line's bold this way to match the bold on "Transaction fee" line where the colon is not inside bold tag since I didn't think it was good to bold the entire "Transaction fee" line and there is more text between "Transaction fee" and the colon. Maybe someone has a better alternative.
laanwj commented at 12:15 PM on May 7, 2018:

We should try, if possible, to keep the html/structure out of the translation strings, to make it easier for translators. For example here it would be better to do this:

questionString.append("%1: %2".arg("Total Amount") .arg(...))

(this arises in a few other places as well)
laanwj commented at 12:18 PM on May 7, 2018: member

Concept ACK

in src/qt/sendcoinsdialog.cpp:318 in e419cb0a2f outdated

 318 |          formatted.append(recipientElement);
 319 |      }
 320 |  
 321 |      QString questionString = tr("Are you sure you want to send?");
 322 | -    questionString.append("<br /><br />%1");
 323 | +    questionString.append(tr("<br /><span style='font-size:10pt;'>Please, review your transaction.</span><br />%1"));

jonasschnelli commented at 12:18 PM on May 7, 2018:

Please extract the translation part so translator do not have to deal with html tags, etc.

in src/qt/sendcoinsdialog.cpp:323 in e419cb0a2f outdated

 327 |          // append fee string if a fee is required
 328 | -        questionString.append("<hr /><span style='color:#aa0000;'>");
 329 | -        questionString.append(BitcoinUnits::formatHtmlWithUnit(model->getOptionsModel()->getDisplayUnit(), txFee));
 330 | -        questionString.append("</span> ");
 331 | -        questionString.append(tr("added as transaction fee"));
 332 | +        questionString.append(tr("<hr /><b>Transaction fee</b>"));

jonasschnelli commented at 12:18 PM on May 7, 2018:

same here

in src/qt/sendcoinsdialog.cpp:355 in e419cb0a2f outdated

 362 | -        questionString.append(tr("Not signalling Replace-By-Fee, BIP-125."));
 363 | -    }
 364 | -    questionString.append("</span>");
 365 | -
 366 | +    questionString.append(QString("<br /><span style='font-size:10pt; font-weight:normal;'>(=%1)</span>")
 367 | +        .arg(alternativeUnits.join(" " + tr("or "))));

jonasschnelli commented at 12:19 PM on May 7, 2018:

use tr("or")+" "?

jonasschnelli changes_requested
jonasschnelli commented at 12:19 PM on May 7, 2018: contributor

Concept ACK Build is here: https://bitcoin.jonasschnelli.ch/build/597
marcoagner commented at 6:41 PM on May 7, 2018: contributor

Thank you all for taking the time to review this. As pointed, I extracted the html tags from translator. The branch is rebased with master; should I squash the commits? Let me know if you see something else I should address, thanks.
MarcoFalke commented at 4:09 PM on May 8, 2018: member

utACK 617dfa98ce16d461009cb5e9eb207c0e0eae30fe Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
[qt]: changes sendcoinsdialog's box layout for improved readability.
[qt]: extracts html tags from translator. [qt]: removes missed tr() call.
f08a385590
marcoagner commented at 8:31 PM on May 8, 2018: contributor

Done; commits squashed.
MarcoFalke commented at 8:49 PM on May 8, 2018: member

utACK f08a38559007143017d172cedb2592ce96355bc1
MarcoFalke commented at 9:10 PM on May 10, 2018: member

@jonasschnelli Mind to do a build here?
jonasschnelli commented at 6:57 AM on May 11, 2018: contributor

build: https://bitcoin.jonasschnelli.ch/build/605
MarcoFalke merged this on May 14, 2018
MarcoFalke closed this on May 14, 2018
MarcoFalke referenced this in commit 81c533c6f4 on May 14, 2018
jonasschnelli commented at 7:12 AM on May 16, 2018: contributor

Post merge tested ACK f08a38559007143017d172cedb2592ce96355bc1
marcoagner deleted the branch on Jul 19, 2018
jasonbcox referenced this in commit 6b6d0ee541 on Oct 11, 2019
UdjinM6 referenced this in commit ee8beba049 on Jun 18, 2021
TheArbitrator referenced this in commit 52ad4ff0ab on Jun 21, 2021
UdjinM6 referenced this in commit 9316725eb2 on Jun 24, 2021
UdjinM6 referenced this in commit d8051ec52f on Jun 29, 2021
UdjinM6 referenced this in commit 47b2895954 on Jun 29, 2021
UdjinM6 referenced this in commit 9621d4181b on Jul 2, 2021
UdjinM6 referenced this in commit 984f6bbd18 on Jul 4, 2021
UdjinM6 referenced this in commit a650372830 on Jul 6, 2021
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#10613 Display payment confirmation information in a way that is easier to read #11606 Improve layout of transaction confirmation dialog #12189 [Qt] Display transaction fee with sat/vbyte value in SendCoinsDialog