[Qt] small cleanups in transactiontablemodel #4667

Diapolo commented at 1:38 PM on August 9, 2014: none

No description provided.

in src/qt/transactiontablemodel.cpp:None in 8ed049e2e7 outdated

 472 | @@ -478,8 +473,9 @@ QVariant TransactionTableModel::txStatusDecoration(const TransactionRecord *wtx)
 473 |      case TransactionStatus::MaturesWarning:
 474 |      case TransactionStatus::NotAccepted:
 475 |          return QIcon(":/icons/transaction_0");
 476 | +    default:
 477 | +        return COLOR_BLACK;

Diapolo commented at 1:40 PM on August 9, 2014:

@laanwj I guess this could just return QVariant(); and I could remove COLOR_BLACK?

Diapolo commented at 10:00 AM on August 27, 2014:

@laanwj Ping, can I use the above suggestion here?

laanwj commented at 10:19 AM on August 27, 2014:

Oh, I missed this somehow! What would be the result of that? I think my original reason for putting COLOR_BLACK here is that otherwise it would look very strange if it ever ends up here (which can happen during development when statusses are added)

Diapolo commented at 9:15 PM on August 28, 2014:

I let it stay black, thanks for clarifying :).

cozz commented at 3:55 AM on August 10, 2014: contributor

Just saying, I am going to submit a pull for adding a new dynamic column to the transactions-list. That way you can filter and sort by watch-only. In the overviewpage, I just wrote watch-only before the address, that way its a little easier to grasp. The blue color may make the watch-only txs look more important than the normal ones. What do you think?

watchonly_column1

watchonly_column2

watchonly_column

Diapolo commented at 12:07 PM on August 10, 2014: none

@cozz I just chose blueish, because I thought watch-only should visually be different. By adding a new column this is also achieved :). But I think for adresses with and without label (watch-only) should be appended, not prepended (on the overviewpage as in tooltips). Otherwise I like your solution better.

laanwj commented at 8:06 AM on August 13, 2014: member

I missed this one somehow. Does #4673 include all the changes here?

laanwj added the label GUI on Aug 13, 2014

Diapolo commented at 1:16 PM on August 14, 2014: none

@laanwj I'm going to remove the watch-only stuff from this, so it can be merged.

Updated!

Diapolo renamed this:
~~[Qt] some cleanup and watch-only changes to transactiontablemodel~~
[Qt] small cleanups in transactiontablemodel
on Aug 14, 2014

in src/qt/guiconstants.h:None in 9d9808a61f outdated

  16 | @@ -17,13 +17,6 @@ static const int STATUSBAR_ICONSIZE = 16;
  17 |  /* Invalid field background style */
  18 |  #define STYLE_INVALID "background:#FF8080"
  19 |  
  20 | -/* Transaction list -- unconfirmed transaction */

laanwj commented at 10:40 AM on August 18, 2014:

Please keep these here. The reason is to keep all compile-time configurability of the GUI in one place. At some point these may be run-time configurable, but NACK on just moving them to another header.

[Qt] add all used colors in txtablemodel to guiconstants

- add colors used in TX status decoration

21f1516468

[Qt] simplify return code and return values in txtablemodel

- also move an added space in a string where it belongs

bbad683224

Diapolo commented at 9:15 PM on August 28, 2014: none

@laanwj Should be ready now, can you take a look?

BitcoinPullTester commented at 9:53 PM on August 28, 2014: none

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

Diapolo closed this on Sep 5, 2014

Diapolo deleted the branch on Sep 5, 2014

laanwj commented at 10:23 AM on September 5, 2014: member

@diapolo eh, closed, what about the other changes?

Diapolo restored the branch on Sep 5, 2014

Diapolo commented at 11:00 AM on September 5, 2014: none

Uuups, that was not intentionally... re-opened.

Diapolo reopened this on Sep 5, 2014

Diapolo commented at 11:01 AM on September 5, 2014: none

@laanwj If you want, you can merge this now.

Diapolo commented at 10:17 AM on September 14, 2014: none

@laanwj Ping, can we please get this merged after your ACK :)?

laanwj merged this on Sep 18, 2014

laanwj closed this on Sep 18, 2014

laanwj referenced this in commit 7fd8813675 on Sep 18, 2014

Diapolo deleted the branch on Sep 18, 2014

MarcoFalke locked this on Sep 8, 2021