New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QT: Add 'copy full transaction details' option #7628

Merged
merged 1 commit into from Mar 8, 2016

Conversation

Projects
None yet
5 participants
@ericshawlinux
Contributor

ericshawlinux commented Mar 1, 2016

Adds feature from issue #7484

@ericshawlinux

This comment has been minimized.

Show comment
Hide comment
@ericshawlinux

ericshawlinux Mar 1, 2016

Contributor

No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

That's a bit odd. My pull request compiled fine on every other job in the Travis CI build... so I'm not sure what to make of this.

Contributor

ericshawlinux commented Mar 1, 2016

No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

That's a bit odd. My pull request compiled fine on every other job in the Travis CI build... so I'm not sure what to make of this.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Mar 1, 2016

Member

No worries, it is a know issue unrelated to your changes.

Member

MarcoFalke commented Mar 1, 2016

No worries, it is a know issue unrelated to your changes.

@ericshawlinux

This comment has been minimized.

Show comment
Hide comment
@ericshawlinux

ericshawlinux Mar 1, 2016

Contributor

ah, I see. what a relief

Contributor

ericshawlinux commented Mar 1, 2016

ah, I see. what a relief

@jonasschnelli jonasschnelli added the GUI label Mar 2, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 2, 2016

Member

Restarted travis, it should pass now.

Member

laanwj commented Mar 2, 2016

Restarted travis, it should pass now.

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Mar 3, 2016

Contributor

ACK ericshawlinux@fff0dcc

Minor NIT: empty '()' on addresses without labels. Can be tested beforehand...

For unconfirmed transaction:

3/3/16 08:05 Unconfirmed () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

4 confirms:

3/3/16 08:05 Confirming (4 of 6 recommended confirmations) () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

Confirmed:

3/3/16 08:05 Confirmed (6 confirmations) () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

With label:

3/3/16 08:05 Confirmed (6 confirmations) (Test label for #7628) mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

The debug log contains this on OS X (the same warning as with Copy raw transaction:

2016-03-03 07:10:46 GUI: Data set on unsupported clipboard mode. QMimeData object will be deleted.
Contributor

paveljanik commented Mar 3, 2016

ACK ericshawlinux@fff0dcc

Minor NIT: empty '()' on addresses without labels. Can be tested beforehand...

For unconfirmed transaction:

3/3/16 08:05 Unconfirmed () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

4 confirms:

3/3/16 08:05 Confirming (4 of 6 recommended confirmations) () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

Confirmed:

3/3/16 08:05 Confirmed (6 confirmations) () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

With label:

3/3/16 08:05 Confirmed (6 confirmations) (Test label for #7628) mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

The debug log contains this on OS X (the same warning as with Copy raw transaction:

2016-03-03 07:10:46 GUI: Data set on unsupported clipboard mode. QMimeData object will be deleted.
@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Mar 3, 2016

Contributor

Off-topic here, but anyway: it would be nice to be able to add label to the address directly from the main window, in Recent transactions.

Contributor

paveljanik commented Mar 3, 2016

Off-topic here, but anyway: it would be nice to be able to add label to the address directly from the main window, in Recent transactions.

@jonasschnelli

View changes

Show outdated Hide outdated src/qt/transactiontablemodel.cpp
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Mar 3, 2016

Member

Tested ACK modulo nit.

Agree with @paveljanik about the () and the warning. Although the console-out warning is unrelated to this PR. If it can be fixed, it could be done in a different PR.

Binaries: https://bitcoin.jonasschnelli.ch/pulls/7628/

4 Examples

3/3/16 13:19 Unconfirmed ()  -0.00003840
3/3/16 13:24 Unconfirmed (Yes! A testlabel.) mgY7FaksgbtrNPVJKzJBrKTG3Ayfqddc8R -1.00003840
2/22/16 09:27 Immature (6 confirmations, will be available after 101) () mr5C9MyoswwkTzFzwys6LfS5TdrX6u8xRo 0.39062500
2/22/16 09:27 Confirmed (143 confirmations) () mr5C9MyoswwkTzFzwys6LfS5TdrX6u8xRo 0.78125000

Two conceptual ideas for further improvements:

  1. Would adding the transaction type make sense? In/Out/Mined? Or is the +/- in the balance enough, at least coinbase transaction are indistinguishable against "other" incoming txes?
  2. If I select a row and hit Ctrl-C, currently, it does copy the cell-content I have last clicked (only date or only type, etc.). I think it would be nice to copy the "full details" if one presses Ctrl-C while having a row selected.
Member

jonasschnelli commented Mar 3, 2016

Tested ACK modulo nit.

Agree with @paveljanik about the () and the warning. Although the console-out warning is unrelated to this PR. If it can be fixed, it could be done in a different PR.

Binaries: https://bitcoin.jonasschnelli.ch/pulls/7628/

4 Examples

3/3/16 13:19 Unconfirmed ()  -0.00003840
3/3/16 13:24 Unconfirmed (Yes! A testlabel.) mgY7FaksgbtrNPVJKzJBrKTG3Ayfqddc8R -1.00003840
2/22/16 09:27 Immature (6 confirmations, will be available after 101) () mr5C9MyoswwkTzFzwys6LfS5TdrX6u8xRo 0.39062500
2/22/16 09:27 Confirmed (143 confirmations) () mr5C9MyoswwkTzFzwys6LfS5TdrX6u8xRo 0.78125000

Two conceptual ideas for further improvements:

  1. Would adding the transaction type make sense? In/Out/Mined? Or is the +/- in the balance enough, at least coinbase transaction are indistinguishable against "other" incoming txes?
  2. If I select a row and hit Ctrl-C, currently, it does copy the cell-content I have last clicked (only date or only type, etc.). I think it would be nice to copy the "full details" if one presses Ctrl-C while having a row selected.
@jonasschnelli

View changes

Show outdated Hide outdated src/qt/transactiontablemodel.cpp
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Mar 6, 2016

Member

Can you squash your commits?

Member

jonasschnelli commented Mar 6, 2016

Can you squash your commits?

@ericshawlinux

This comment has been minimized.

Show comment
Hide comment
@ericshawlinux

ericshawlinux Mar 7, 2016

Contributor

Ok

Contributor

ericshawlinux commented Mar 7, 2016

Ok

QT: Add 'copy full transaction details' option
Adds feature from issue #7484

modifies the ctrl-c binding to copy full transaction details in transaction view.

Added translation
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Mar 8, 2016

Member

Tested ACK b51ed40.

Member

jonasschnelli commented Mar 8, 2016

Tested ACK b51ed40.

@jonasschnelli jonasschnelli merged commit b51ed40 into bitcoin:master Mar 8, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jonasschnelli added a commit that referenced this pull request Mar 8, 2016

Merge #7628: QT: Add 'copy full transaction details' option
b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)

codablock added a commit to codablock/dash that referenced this pull request Sep 5, 2017

Merge #7628: QT: Add 'copy full transaction details' option
b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)

codablock added a commit to codablock/dash that referenced this pull request Sep 5, 2017

Merge #7628: QT: Add 'copy full transaction details' option
b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)

UdjinM6 added a commit to dashpay/dash that referenced this pull request Sep 7, 2017

Backport Bitcoin Qt/Gui changes up to 0.14.x part 1 (#1614)
* Merge pull request #7154

a3c3ddb [Qt] add InMempool() info to transaction details (Jonas Schnelli)

* Merge pull request #7218

fa5769e [qt] Fix misleading translation (MarcoFalke)
fa8c8d7 torcontrol debug: Change to a blanket message that covers both cases (MarcoFalke)

* Merge pull request #7255

6fd0a07 Remove hardcoded fee from CoinControl ToolTip (fanquake)
5fdf32d Replace some instances of formatWithUnit with formatHtmlWithUnit (fanquake)

* Merge pull request #7263

a5a0831 Double semicolon cleanup. (21E14)

* Merge pull request #7334

fa989fb [qt] coincontrol workaround is still needed in qt5.4 (fixed in qt5.5) (MarcoFalke)

* Merge pull request #7329

9d263bd Typo fixes in comments (Chris Wheeler)

* Merge #7396: [Qt] Add option to increase/decrease font size in the console window

43abb02 [Qt] Add a new chevron/arrow icon for the console prompt line (Jonas Schnelli)
56c9e66 [Qt] keep scroll position in GUI console after changing font size (Jonas Schnelli)
3a3a927 [Qt] Add option to increase/decrease font size in the console window (Jonas Schnelli)

* Merge #7628: QT: Add 'copy full transaction details' option

b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)

* Merge #7668: Fix history deletion bug after font size change

21e45a0 Fix history deletion bug after font change (Andrew C)

* Copy/Move font size related icons into theme folders

* Use formatTxDate for date/time in TxPlainTextRole
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment