Page MenuHomePhabricator

Merge #13158: [Qt]: Improve sendcoinsdialog readability
ClosedPublic

Authored by nakihito on Sep 26 2019, 00:08.

Details

Summary

f08a385590 [qt]: changes sendcoinsdialog's box layout for improved readability. (marcoagner)

Pull request description:

I'm addressing two (probably duplicate) issues: https://github.com/bitcoin/bitcoin/issues/11606 and https://github.com/bitcoin/bitcoin/issues/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):
![screenshot from 2018-05-03 15-11-30](https://user-images.githubusercontent.com/5016303/39581859-16abec82-4edc-11e8-86d3-eb722f8a7ed6.png)

To this:
![screenshot from 2018-05-03 15-15-41](https://user-images.githubusercontent.com/5016303/39582066-96856adc-4edc-11e8-804c-468aec44cc8d.png)

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

Tree-SHA512: e94b740fab6c1babd853a97be65c3b6f86ec174c975a926fde66b147f7a47e0cf0fa10f7255ba92aaba68c76a80dde8c688008179a34705a9799bf24d3c5cd46

Backport of Core PR13158
https://github.com/bitcoin/bitcoin/pull/13158/

Test Plan
make check
bitcoin-qt -> Send -> View confirmation window

Before:

After:

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Sep 26 2019, 00:08
Owners added a reviewer: Restricted Owners Package.Sep 26 2019, 00:08
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 26 2019, 00:08
nakihito planned changes to this revision.Sep 26 2019, 00:08
nakihito requested review of this revision.Sep 26 2019, 00:10
Fabien requested changes to this revision.Sep 26 2019, 06:49

Before:

After:

Note the missing translations.

This revision now requires changes to proceed.Sep 26 2019, 06:49
nakihito planned changes to this revision.Sep 26 2019, 18:40

Will request review once translations are added.

nakihito requested review of this revision.Oct 3 2019, 20:47

Translations added in a separate diff. See D4208.

deadalnix accepted this revision.Oct 6 2019, 17:58

The test plan once again doesn't provide anything to check. Ok the window was open, then what? What did change/didn't change? Is that expected?

nakihito edited the test plan for this revision. (Show Details)Oct 6 2019, 22:29
nakihito edited the test plan for this revision. (Show Details)
Fabien accepted this revision.Oct 7 2019, 07:57
This revision is now accepted and ready to land.Oct 7 2019, 07:57