[qt] CoinControlDialog amount header display unit #13276

pull GreatSock wants to merge 2 commits into bitcoin:master from GreatSock:coincontrol_unit changing 3 files +11 −3

GreatSock commented at 12:56 PM on May 18, 2018: contributor

The coin control table currently only says "Amount". This makes it more consistent with other tables by showing the current display unit like "Amount (BTC)".

[qt] CoinControlDialog header amount display unit

The current display unit will now be appended to the amount header as in other tables.

219589fd8c

[qt] CoinControlDialog adjusted column widths

Made amount column wider to make sure the display unit fits.
Also decreased the width of the label column to get the same total width.

bced272869

fanquake added the label GUI on May 18, 2018
promag commented at 2:50 PM on May 18, 2018: member

utACK bced272. Is 20px more enough?
GreatSock commented at 5:58 PM on May 18, 2018: contributor

20px is just enough to fit the widest unit "Amount (mBTC)" on Ubuntu. But I'm not sure if there is some case where it doesn't fit, since different operating systems or window managers seem to have different font sizes.
in src/qt/coincontroldialog.cpp:168 in bced272869
```
 164 | @@ -165,6 +165,7 @@ void CoinControlDialog::setModel(WalletModel *_model)
 165 |          updateView();
 166 |          updateLabelLocked();
 167 |          CoinControlDialog::updateLabels(_model, this);
 168 | +        UpdateAmountHeader(_model);
```
jonasschnelli commented at 8:46 AM on May 25, 2018:

Why the extra roundtrip via UpdateAmountHeader() instead of just setting the header item here? Could use a comment to make it more obvious whats happening here.

GreatSock commented at 12:55 PM on May 25, 2018:

Maybe I could rename it to something like UpdateAmountDisplayUnit()? I realize that UpdateAmountHeader() is kind of vague.

hebasto commented at 7:08 AM on September 23, 2018:

Agree with @jonasschnelli: just setting the header will do.
DrahtBot closed this on Jul 22, 2018
DrahtBot reopened this on Jul 22, 2018

in src/qt/coincontroldialog.cpp:130 in bced272869

 126 | @@ -127,8 +127,8 @@ CoinControlDialog::CoinControlDialog(const PlatformStyle *_platformStyle, QWidge
 127 |      ui->treeWidget->headerItem()->setText(COLUMN_CHECKBOX, QString());
 128 |  
 129 |      ui->treeWidget->setColumnWidth(COLUMN_CHECKBOX, 84);
 130 | -    ui->treeWidget->setColumnWidth(COLUMN_AMOUNT, 110);
 131 | -    ui->treeWidget->setColumnWidth(COLUMN_LABEL, 190);
 132 | +    ui->treeWidget->setColumnWidth(COLUMN_AMOUNT, 130);

hebasto commented at 6:50 AM on September 23, 2018:

To be consistent with https://github.com/bitcoin/bitcoin/blob/920c090f63f4990bf0f3b3d1a6d3d8a8bcd14ba0/src/qt/transactionview.h#L54-L61

ui->treeWidget->setColumnWidth(COLUMN_AMOUNT, 120);
ui->treeWidget->setColumnWidth(COLUMN_LABEL, 190); // do not touch
ui->treeWidget->setColumnWidth(COLUMN_DATE, 120);

DrahtBot closed this on Apr 28, 2019
DrahtBot commented at 7:11 PM on April 28, 2019: member

The last travis run for this pull request was 279 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
DrahtBot reopened this on Apr 28, 2019
fanquake added the label Up for grabs on Jun 17, 2019
fanquake commented at 9:38 AM on June 17, 2019: member

Closing as "Up for grabs". Also looks like the author is no longer active on GitHub.
fanquake closed this on Jun 17, 2019
DrahtBot locked this on Dec 16, 2021
fanquake removed the label Up for grabs on Aug 4, 2022

Contributors

Labels