Skip to content

qt: Make it possible to show details for multiple transactions#7939

Merged
laanwj merged 2 commits intobitcoin:masterfrom
laanwj:2016_04_qt_multiple_transaction_details
Apr 28, 2016
Merged

qt: Make it possible to show details for multiple transactions#7939
laanwj merged 2 commits intobitcoin:masterfrom
laanwj:2016_04_qt_multiple_transaction_details

Conversation

@laanwj
Copy link
Copy Markdown
Member

@laanwj laanwj commented Apr 25, 2016

A small GUI annoyance for me has always been that it's impossible to have multiple transaction detail windows open, for example to compare transactions.

This patch makes the window non-modal so that it is possible to open transaction details at will.

@laanwj laanwj added the GUI label Apr 25, 2016
A small GUI annoyance for me has always been that it's impossible to
have multiple transaction detail windows open, for example to compare
transactions.

This patch makes the window non-modal so that it is possible to open
transaction details at will.
@laanwj laanwj force-pushed the 2016_04_qt_multiple_transaction_details branch from 1f3b3fd to 17a6a21 Compare April 25, 2016 14:06
@jonasschnelli
Copy link
Copy Markdown
Contributor

Nice! Also though about this. Thanks.

utACK 1f3b3fd48bb6c5787893529862430bd77b2c7fb3

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Apr 25, 2016

Another thing to consider here now may be to add some information to the window title. After all if there are multiple open it may be useful to distinguish them in taskbars etc.

I'm not sure on what to add though; transaction id seems to be of limited usefulness. Maybe date/recipient/amount.

@paveljanik
Copy link
Copy Markdown
Contributor

Numbering the windows?

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Apr 25, 2016

Numbering doesn't seem more useful to me than just keeping them with the same window title. It needs to actually identify the transaction.
(also this opens up issues with reusing or not reusing numbers, I'd rather just add (part of) the transaction id in that case)

@jonasschnelli
Copy link
Copy Markdown
Contributor

What about the shortened transaction id (middle truncated). Something like 4d8fad...8dd9f9 for 4d8fad408da55d4b54e3be97960db06d78489f5984e0a6e8ada082295e8dd9f9?

@paveljanik
Copy link
Copy Markdown
Contributor

Full tx ID. window manager/... will shorten it if needed (ie. Transaction details: tx_id).

@jonasschnelli
Copy link
Copy Markdown
Contributor

jonasschnelli commented Apr 25, 2016

To make this work on OSX (not sure about Windows) you need to make the dialog non-modal.

diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
index eb6111e..bfdd7a4 100644
--- a/src/qt/transactionview.cpp
+++ b/src/qt/transactionview.cpp
@@ -480,6 +480,7 @@ void TransactionView::showDetails()
     {
         TransactionDescDialog *dlg = new TransactionDescDialog(selection.at(0));
         dlg->setAttribute(Qt::WA_DeleteOnClose);
+        dlg->setModal(false);
         dlg->show();
     }
 }

@jonasschnelli
Copy link
Copy Markdown
Contributor

jonasschnelli commented Apr 25, 2016

I tested with the full transaction-ID as window title. LGTM.
bildschirmfoto 2016-04-25 um 21 27 50

diff --git a/src/qt/transactiondescdialog.cpp b/src/qt/transactiondescdialog.cpp
index f7b6995..5c10d84 100644
--- a/src/qt/transactiondescdialog.cpp
+++ b/src/qt/transactiondescdialog.cpp
@@ -15,6 +15,7 @@ TransactionDescDialog::TransactionDescDialog(const QModelIndex &idx, QWidget *pa
 {
     ui->setupUi(this);
     QString desc = idx.data(TransactionTableModel::LongDescriptionRole).toString();
+    setWindowTitle(tr("Details for %1").arg(idx.data(TransactionTableModel::TxHashRole).toString()));
     ui->detailText->setHtml(desc);
 }

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Apr 26, 2016

@jonasschnelli: Interesting, do you know why is this necessary specifically here, how about the other places where we use setAttribute(Qt::WA_DeleteOnClose)?

  • src/qt/receivecoinsdialog.cpp: on_receiveButton_clicked, on_recentRequestsView_doubleClicked
  • src/qt/walletview.cpp: WalletView::gotoSignMessageTab, WalletView::gotoVerifyMessageTab

(excluding the splash window and the shutdown window as they are special cases anyway)

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Apr 26, 2016

From the QDialog documentation:
http://doc.qt.io/qt-5/qdialog.html#modal-prop "By default, this property is false"

It's only dlg->exec() which sets it to true, which we don't use anymore (here). I don't think we need to set it.

Edit: screenshot looks good, I do think we should prefix the title Transaction details: though so we don't get an all-hex title :)

@paveljanik
Copy link
Copy Markdown
Contributor

Works OK here, even without setModal(false).

@jonasschnelli
Copy link
Copy Markdown
Contributor

Right. setModal() is not required. Was a local issue here.

@jonasschnelli
Copy link
Copy Markdown
Contributor

Using "Details for" instead of "Transaction Details" would not truncate the window title (at least on OSX).

bildschirmfoto 2016-04-26 um 08 13 21

Tested ACK 17a6a21.

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Apr 26, 2016

Yes, Abbreviating to "Details for %s" makes sense, will do that, it was just that having the whole title hex was a bit too cypherpunk even for me :)

@laanwj laanwj force-pushed the 2016_04_qt_multiple_transaction_details branch from b0aad40 to f135e3c Compare April 26, 2016 14:05
@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Apr 26, 2016

Added a new commit that puts the tx id in the window title (as well as removes the old "Transaction details" translation string).

@laanwj laanwj merged commit f135e3c into bitcoin:master Apr 28, 2016
laanwj added a commit that referenced this pull request Apr 28, 2016
…ctions

f135e3c qt: Add transaction hash to details window title (Wladimir J. van der Laan)
17a6a21 qt: Make it possible to show details for multiple transactions (Wladimir J. van der Laan)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 20, 2017
… transactions

f135e3c qt: Add transaction hash to details window title (Wladimir J. van der Laan)
17a6a21 qt: Make it possible to show details for multiple transactions (Wladimir J. van der Laan)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants