Before this change QT just remained on the Send tab, which I found confusing. Now it switches to the Transactions tab. This makes it more clear to the user that the send actually succeeded, and here they can monitor progress.
Ideally I would like to highlight the transaction, e.g. by refactoring TransactionView::focusTransaction(const QModelIndex &idx) to accept a transaction hash, but I’m not sure how to do that.
just remained on the Send tab, which I found confusing
IMHO this is more confusing, having such sudden UI change. What if he wants to do multiple sends?
How about adding a checkbox like [x] View transaction after sending and remembering the checkbox state in settings?
53@@ -54,6 +54,9 @@ public Q_SLOTS:
54 void setBalance(const CAmount& balance, const CAmount& unconfirmedBalance, const CAmount& immatureBalance,
55 const CAmount& watchOnlyBalance, const CAmount& watchUnconfBalance, const CAmount& watchImmatureBalance);
56
57+Q_SIGNALS:
58+ void transactionSent();
focusTransaction(const QModelIndex &idx) needs.
Nit, rename to coinsSent().
Regarding row focus, see https://stackoverflow.com/a/35154534.
@promag I don’t think I’ve ever seen an online banking application that doesn’t take the user to the transaction overview page after sending money.
In most financial applications Send is a modal experience, where the Send screen is pushed on top of the transaction screen and popped when the user is done. In a desktop browser that’s usually a modal window, on an iPhone it’s a window that animates in from the right, with an arrow on the top left to dismiss it. Doing another send is then simply a matter of clicking on Send again.
Making the experience modal might be too much of a change, but e.g. we could get rid of all tabs: merge the Overview and Transactions tab, Send and Receive open a new window.
I do think highlighting the new transaction would give a better visual clue to the user as to why they moved to this other tab.
368@@ -369,6 +369,8 @@ void SendCoinsDialog::on_sendButton_clicked()
369 accept();
370 CoinControlDialog::coinControl()->UnSelectAll();
371 coinControlUpdateLabels();
372+ // Navigate to transaction history page:
600+ const QModelIndexList results = this->model->getTransactionTableModel()->match(
601+ this->model->getTransactionTableModel()->index(0,0),
602+ TransactionTableModel::TxHashRole,
603+ hash);
604+
605+ if (!results.length())
0if (results.isEmpty()) return;
591@@ -592,6 +592,22 @@ void TransactionView::focusTransaction(const QModelIndex &idx)
592 transactionView->setFocus();
593 }
594
595+void TransactionView::focusTransaction(const QString &hash)
596+{
597+ if(!transactionProxyModel)
if, one line.
53@@ -54,6 +54,9 @@ public Q_SLOTS:
54 void setBalance(const CAmount& balance, const CAmount& unconfirmedBalance, const CAmount& immatureBalance,
55 const CAmount& watchOnlyBalance, const CAmount& watchUnconfBalance, const CAmount& watchImmatureBalance);
56
57+Q_SIGNALS:
58+ void coinsSent(QString);
const QString& txid so that it’s clear what the argument is?
591@@ -592,6 +592,22 @@ void TransactionView::focusTransaction(const QModelIndex &idx)
592 transactionView->setFocus();
593 }
594
595+void TransactionView::focusTransaction(const QString &hash)
596+{
597+ if(!transactionProxyModel)
598+ return;
599+
600+ const QModelIndexList results = this->model->getTransactionTableModel()->match(
generate 10000. I didn’t notice any delay when sending transactions, although my machine is relatively fast. I also tried reversing transaction order in the view.
369@@ -369,6 +370,8 @@ void SendCoinsDialog::on_sendButton_clicked()
370 accept();
371 CoinControlDialog::coinControl()->UnSelectAll();
372 coinControlUpdateLabels();
373+ const uint256 txid = currentTransaction.getTransaction()->GetHash();
374+ Q_EMIT coinsSent(QString::fromStdString(txid.ToString()));
Q_DECLARE_METATYPE, see https://github.com/bitcoin/bitcoin/blob/dcfe218626b05204e9fbc95ba5d95ca0eb72ec9b/src/qt/bitcoin.cpp#L80-L82
uint256.
12@@ -13,6 +13,7 @@
13 #include <qt/optionsmodel.h>
14 #include <qt/platformstyle.h>
15 #include <qt/sendcoinsentry.h>
16+#include <qt/transactiontablemodel.h>
53@@ -54,6 +54,9 @@ public Q_SLOTS:
54 void setBalance(const CAmount& balance, const CAmount& unconfirmedBalance, const CAmount& immatureBalance,
55 const CAmount& watchOnlyBalance, const CAmount& watchUnconfBalance, const CAmount& watchImmatureBalance);
56
57+Q_SIGNALS:
58+ void coinsSent(const uint256 txid);
const uint256& txid.
590@@ -592,6 +591,21 @@ void TransactionView::focusTransaction(const QModelIndex &idx)
591 transactionView->setFocus();
592 }
593
594+void TransactionView::focusTransaction(const uint256 txid)
const uint256& txid.
601+ TransactionTableModel::TxHashRole,
602+ QString::fromStdString(txid.ToString()));
603+
604+ if (results.isEmpty()) return;
605+
606+ focusTransaction(results[0]);
isEmpty() with a check that length is 1.
I’d like to select both entries on the transaction screen when a user sends to two addresses. Unfortunately:
this->model->getTransactionTableModel()->match(... only returns one entry.transactionView->selectRow(...) deselects whichever row was previously selected (even though the user can manually select multiple transactions)
https://doc.qt.io/qt-5/qtableview.html#setSelection with QItemSelectionModel::Rows (and probably QItemSelectionModel::ClearAndSelect).
Why would match only return one entry??
Why would match only return one entry??
I don’t know. Here’s a WIP commit. The fprintf(stderr, "row: line is only called once.
I’m confused about the relation between this->model->getTransactionTableModel(), transactionView and transactionProxyModel. E.g. what does QModelIndex targetIdx = transactionProxyModel->mapFromSource(idx); do? My intuitive grasp is that there’s somehow multiple representations of the same data. Maybe @laanwj can explain the big picture here?
601+ TransactionTableModel::TxHashRole,
602+ QString::fromStdString(txid.ToString()), -1);
603+
604+ transactionView->setFocus();
605+ transactionView->selectionModel()->clearSelection();
606+ for (const QModelIndex& index : results) {
We could ensure first row is visible:
0for (...) {
1 if (index == results[0]) transactionView->scrollTo(index);
transactionProxyModel->mapFromSource) when transactions are ordered by date. Unfortunately when they’re ordered in reverse it doesn’t scroll all the way to the bottom. Maybe we need to wait a little bit for transactionView to become aware of its new size?
590@@ -592,6 +591,25 @@ void TransactionView::focusTransaction(const QModelIndex &idx)
591 transactionView->setFocus();
592 }
593
594+void TransactionView::focusTransaction(const uint256& txid)
595+{
596+ if(!transactionProxyModel)
if.
368@@ -369,6 +369,8 @@ void SendCoinsDialog::on_sendButton_clicked()
369 accept();
370 CoinControlDialog::coinControl()->UnSelectAll();
371 coinControlUpdateLabels();
372+ const uint256 txid = currentTransaction.getTransaction()->GetHash();
Avoid copy here:
0const uint256& txid = ...
Or roll it into the Q_EMIT, as txid is not used after this. YMMV. For performance I’d be surprised it makes any difference.
0Q_EMIT coinsSent(currentTransaction.getTransaction()->GetHash());
Q_EMIT.
The transaction will be selected. When sending to multiple
destinations, all will be selected (thanks @promag).
606+ for (const QModelIndex& index : results) {
607+ const QModelIndex targetIndex = transactionProxyModel->mapFromSource(index);
608+ transactionView->selectionModel()->select(
609+ targetIndex,
610+ QItemSelectionModel::Rows | QItemSelectionModel::Select);
611+ if (index == results[0]) transactionView->scrollTo(targetIndex);
On my system calling transactionView->scrollTo(targetIndex) for every model index fixes it (not sure why). It also works in the default order because scroll hint is QAbstractItemView::EnsureVisible, so focusing the 2nd row will not remove the 1st from the viewport (the 2nd row is already visible).
But it’s also possible to order by other column (for instance address), and in that case it can be impossible to show the whole selection in the table viewport.
I see 3 possible solutions for that:
When I call scrollTo for every model index and transactions are sort by ascending date, things get weird: if I send to two destinations it scrolls correctly, but if I send to one destination it won’t scroll far enough.
Calling scollTo() twice “fixes” it. Seems suspiciously similar to this I’ll poke around a bit.
I prefer your first solution for the case where transactions are not ordered by date. At least one of them will be in view.