From a6f567590b73986fde83d8face6827e0801a2e8c Mon Sep 17 00:00:00 2001 From: John Moffett Date: Fri, 2 Dec 2022 11:06:43 -0500 Subject: [PATCH] Fix Requested Payments History Multiselect The recent payment requests list has somewhat broken functionality. You can only select contiguous rows. Sorting the list loses the selection data. The context menu appears when multiple rows are selected despite the actions only affecting the first in the list. These issues are all corrected. First, a QSortFilterProxyModel is inserted to take care of sorting instead of sorting the model manually. This also preserves selection data when sorting. Second, the selection model is changed to ExtendedSelection to allow for non-contiguous multi- select. Third, the context menu now operates on multiple selections properly. It will copy newline-separated rows of data if appropriate. --- src/qt/receivecoinsdialog.cpp | 206 +++++++++++++++++++++------- src/qt/receivecoinsdialog.h | 8 +- src/qt/recentrequeststablemodel.cpp | 48 +++---- src/qt/recentrequeststablemodel.h | 13 -- 4 files changed, 179 insertions(+), 96 deletions(-) diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index 22eb642ecd1..dff32074937 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include @@ -22,6 +23,18 @@ #include #include +static const char* const COPY_URI_SINGLE_TEXT = "Copy &URI"; +static const char* const COPY_ADDRESS_SINGLE_TEXT = "&Copy address"; +static const char* const COPY_LABEL_SINGLE_TEXT = "Copy &label"; +static const char* const COPY_MESSAGE_SINGLE_TEXT = "Copy &message"; +static const char* const COPY_AMOUNT_SINGLE_TEXT = "Copy &amount"; + +static const char* const COPY_URI_MULTIPLE_TEXT = "Copy &URIs"; +static const char* const COPY_ADDRESS_MULTIPLE_TEXT = "&Copy addresses"; +static const char* const COPY_LABEL_MULTIPLE_TEXT = "Copy &labels"; +static const char* const COPY_MESSAGE_MULTIPLE_TEXT = "Copy &messages"; +static const char* const COPY_AMOUNT_MULTIPLE_TEXT = "Copy &amounts"; + ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWidget *parent) : QDialog(parent, GUIUtil::dialog_flags), ui(new Ui::ReceiveCoinsDialog), @@ -29,6 +42,9 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid { ui->setupUi(this); + m_sort_proxy = new QSortFilterProxyModel(this); + m_sort_proxy->setSortRole(Qt::UserRole); + if (!_platformStyle->getImagesOnButtons()) { ui->clearButton->setIcon(QIcon()); ui->receiveButton->setIcon(QIcon()); @@ -43,11 +59,11 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid // context menu contextMenu = new QMenu(this); - contextMenu->addAction(tr("Copy &URI"), this, &ReceiveCoinsDialog::copyURI); - contextMenu->addAction(tr("&Copy address"), this, &ReceiveCoinsDialog::copyAddress); - copyLabelAction = contextMenu->addAction(tr("Copy &label"), this, &ReceiveCoinsDialog::copyLabel); - copyMessageAction = contextMenu->addAction(tr("Copy &message"), this, &ReceiveCoinsDialog::copyMessage); - copyAmountAction = contextMenu->addAction(tr("Copy &amount"), this, &ReceiveCoinsDialog::copyAmount); + copyURIAction = contextMenu->addAction(tr(COPY_URI_SINGLE_TEXT), this, &ReceiveCoinsDialog::copyURI); + copyAddressAction = contextMenu->addAction(tr(COPY_ADDRESS_SINGLE_TEXT), this, &ReceiveCoinsDialog::copyAddress); + copyLabelAction = contextMenu->addAction(tr(COPY_LABEL_SINGLE_TEXT), this, &ReceiveCoinsDialog::copyLabel); + copyMessageAction = contextMenu->addAction(tr(COPY_MESSAGE_SINGLE_TEXT), this, &ReceiveCoinsDialog::copyMessage); + copyAmountAction = contextMenu->addAction(tr(COPY_AMOUNT_SINGLE_TEXT), this, &ReceiveCoinsDialog::copyAmount); connect(ui->recentRequestsView, &QWidget::customContextMenuRequested, this, &ReceiveCoinsDialog::showMenu); connect(ui->clearButton, &QPushButton::clicked, this, &ReceiveCoinsDialog::clear); @@ -56,7 +72,7 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid tableView->verticalHeader()->hide(); tableView->setAlternatingRowColors(true); tableView->setSelectionBehavior(QAbstractItemView::SelectRows); - tableView->setSelectionMode(QAbstractItemView::ContiguousSelection); + tableView->setSelectionMode(QAbstractItemView::ExtendedSelection); QSettings settings; if (!tableView->horizontalHeader()->restoreState(settings.value("RecentRequestsViewHeaderState").toByteArray())) { @@ -74,12 +90,12 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) if(_model && _model->getOptionsModel()) { - _model->getRecentRequestsTableModel()->sort(RecentRequestsTableModel::Date, Qt::DescendingOrder); connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &ReceiveCoinsDialog::updateDisplayUnit); updateDisplayUnit(); QTableView* tableView = ui->recentRequestsView; - tableView->setModel(_model->getRecentRequestsTableModel()); + tableView->setModel(m_sort_proxy); + m_sort_proxy->setSourceModel(_model->getRecentRequestsTableModel()); tableView->sortByColumn(RecentRequestsTableModel::Date, Qt::DescendingOrder); connect(tableView->selectionModel(), @@ -191,12 +207,10 @@ void ReceiveCoinsDialog::on_receiveButton_clicked() void ReceiveCoinsDialog::on_recentRequestsView_doubleClicked(const QModelIndex &index) { - const RecentRequestsTableModel *submodel = model->getRecentRequestsTableModel(); - ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this); - dialog->setModel(model); - dialog->setInfo(submodel->entry(index.row()).recipient); - dialog->setAttribute(Qt::WA_DeleteOnClose); - dialog->show(); + QModelIndexList selection = SelectedRows(); + if (!selection.isEmpty() && selection.at(0).isValid()) { + ShowReceiveRequestDialogForItem(selection.at(0)); + } } void ReceiveCoinsDialog::recentRequestsView_selectionChanged(const QItemSelection &selected, const QItemSelection &deselected) @@ -209,90 +223,182 @@ void ReceiveCoinsDialog::recentRequestsView_selectionChanged(const QItemSelectio void ReceiveCoinsDialog::on_showRequestButton_clicked() { - if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel()) - return; - QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows(); + QModelIndexList selection = SelectedRows(); for (const QModelIndex& index : selection) { - on_recentRequestsView_doubleClicked(index); + ShowReceiveRequestDialogForItem(index); } } void ReceiveCoinsDialog::on_removeRequestButton_clicked() { - if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel()) - return; - QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows(); + QModelIndexList selection = SelectedRows(); if(selection.empty()) return; - // correct for selection mode ContiguousSelection - QModelIndex firstIndex = selection.at(0); - model->getRecentRequestsTableModel()->removeRows(firstIndex.row(), selection.length(), firstIndex.parent()); + + // Collect row indices in a set (sorted) and pass in reverse order to removeRows + // to avoid having to keep track of changed source indices after each removal + std::set row_indices; + for (const QModelIndex& ind : selection) { + row_indices.insert(ind.row()); + } + + for (auto row_ind : reverse_iterate(row_indices)) { + model->getRecentRequestsTableModel()->removeRows(row_ind, 1); + } } -QModelIndex ReceiveCoinsDialog::selectedRow() +QModelIndexList ReceiveCoinsDialog::SelectedRows() { if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel()) - return QModelIndex(); + return QModelIndexList(); QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows(); - if(selection.empty()) - return QModelIndex(); - // correct for selection mode ContiguousSelection - QModelIndex firstIndex = selection.at(0); - return firstIndex; + QModelIndexList source_mapped; + for (auto row : selection) { + source_mapped.append(m_sort_proxy->mapToSource(row)); + } + + return source_mapped; } // copy column of selected row to clipboard void ReceiveCoinsDialog::copyColumnToClipboard(int column) { - QModelIndex firstIndex = selectedRow(); - if (!firstIndex.isValid()) { + const QModelIndexList sel = SelectedRows(); + if (sel.isEmpty()) { + return; + } + + const RecentRequestsTableModel* const submodel = model->getRecentRequestsTableModel(); + QString column_value; + for (int sel_ind = 0; sel_ind < sel.size(); ++sel_ind) { + if (!sel.at(sel_ind).isValid()) { + continue; + } + column_value += submodel->index(sel.at(sel_ind).row(), column).data(Qt::EditRole).toString(); + if (sel_ind < sel.size() - 1) { + column_value += QString("\n"); + } + } + GUIUtil::setClipboard(column_value); +} + +void ReceiveCoinsDialog::ShowReceiveRequestDialogForItem(const QModelIndex& index) +{ + if (!index.isValid()) { return; } - GUIUtil::setClipboard(model->getRecentRequestsTableModel()->index(firstIndex.row(), column).data(Qt::EditRole).toString()); + const RecentRequestsTableModel* submodel = model->getRecentRequestsTableModel(); + ReceiveRequestDialog* dialog = new ReceiveRequestDialog(this); + dialog->setModel(model); + dialog->setInfo(submodel->entry(index.row()).recipient); + dialog->setAttribute(Qt::WA_DeleteOnClose); + dialog->show(); } // context menu void ReceiveCoinsDialog::showMenu(const QPoint &point) { - const QModelIndex sel = selectedRow(); - if (!sel.isValid()) { + const QModelIndexList sel = SelectedRows(); + if (sel.isEmpty()) { return; } - // disable context menu actions when appropriate - const RecentRequestsTableModel* const submodel = model->getRecentRequestsTableModel(); - const RecentRequestEntry& req = submodel->entry(sel.row()); - copyLabelAction->setDisabled(req.recipient.label.isEmpty()); - copyMessageAction->setDisabled(req.recipient.message.isEmpty()); - copyAmountAction->setDisabled(req.recipient.amount == 0); - - contextMenu->exec(QCursor::pos()); + if (sel.size() == 1 && sel.at(0).isValid()) { + // Make sure we have single labels + copyAddressAction->setText(tr(COPY_ADDRESS_SINGLE_TEXT)); + copyURIAction->setText(tr(COPY_URI_SINGLE_TEXT)); + copyLabelAction->setText(tr(COPY_LABEL_SINGLE_TEXT)); + copyMessageAction->setText(tr(COPY_MESSAGE_SINGLE_TEXT)); + copyAmountAction->setText(tr(COPY_AMOUNT_SINGLE_TEXT)); + + // disable context menu actions when appropriate + const RecentRequestsTableModel* const submodel = model->getRecentRequestsTableModel(); + const RecentRequestEntry& req = submodel->entry(sel.at(0).row()); + copyLabelAction->setDisabled(req.recipient.label.isEmpty()); + copyMessageAction->setDisabled(req.recipient.message.isEmpty()); + copyAmountAction->setDisabled(req.recipient.amount == 0); + + contextMenu->exec(QCursor::pos()); + } else if (sel.size() > 1) { + // multiple selection + + // Make sure we have multiple labels + copyAddressAction->setText(tr(COPY_ADDRESS_MULTIPLE_TEXT)); + copyURIAction->setText(tr(COPY_URI_MULTIPLE_TEXT)); + copyLabelAction->setText(tr(COPY_LABEL_MULTIPLE_TEXT)); + copyMessageAction->setText(tr(COPY_MESSAGE_MULTIPLE_TEXT)); + copyAmountAction->setText(tr(COPY_AMOUNT_MULTIPLE_TEXT)); + + copyLabelAction->setDisabled(false); + copyMessageAction->setDisabled(false); + copyAmountAction->setDisabled(false); + + // disable context menu actions when appropriate + const RecentRequestsTableModel* const submodel = model->getRecentRequestsTableModel(); + + for (auto selection : sel) { + if (!selection.isValid()) { + continue; + } + const RecentRequestEntry& req = submodel->entry(selection.row()); + if (req.recipient.label.isEmpty()) { + copyLabelAction->setDisabled(true); + } + if (req.recipient.message.isEmpty()) { + copyMessageAction->setDisabled(true); + } + if (req.recipient.amount == 0) { + copyAmountAction->setDisabled(true); + } + } + contextMenu->exec(QCursor::pos()); + } } // context menu action: copy URI void ReceiveCoinsDialog::copyURI() { - QModelIndex sel = selectedRow(); - if (!sel.isValid()) { + const QModelIndexList sel = SelectedRows(); + if (sel.isEmpty()) { return; } const RecentRequestsTableModel * const submodel = model->getRecentRequestsTableModel(); - const QString uri = GUIUtil::formatBitcoinURI(submodel->entry(sel.row()).recipient); + QString uri; + for (int sel_ind = 0; sel_ind < sel.size(); ++sel_ind) { + if (!sel.at(sel_ind).isValid()) { + continue; + } + const RecentRequestEntry& req = submodel->entry(sel.at(sel_ind).row()); + uri += GUIUtil::formatBitcoinURI(req.recipient); + if (sel_ind < sel.size() - 1) { + uri += QString("\n"); + } + } GUIUtil::setClipboard(uri); } // context menu action: copy address void ReceiveCoinsDialog::copyAddress() { - const QModelIndex sel = selectedRow(); - if (!sel.isValid()) { + const QModelIndexList sel = SelectedRows(); + if (sel.isEmpty()) { return; } const RecentRequestsTableModel* const submodel = model->getRecentRequestsTableModel(); - const QString address = submodel->entry(sel.row()).recipient.address; + QString address; + for (int sel_ind = 0; sel_ind < sel.size(); ++sel_ind) { + if (!sel.at(sel_ind).isValid()) { + continue; + } + const RecentRequestEntry& req = submodel->entry(sel.at(sel_ind).row()); + address += req.recipient.address; + if (sel_ind < sel.size() - 1) { + address += QString("\n"); + } + } GUIUtil::setClipboard(address); } diff --git a/src/qt/receivecoinsdialog.h b/src/qt/receivecoinsdialog.h index 0bb02ebcf23..036dab16179 100644 --- a/src/qt/receivecoinsdialog.h +++ b/src/qt/receivecoinsdialog.h @@ -13,6 +13,7 @@ #include #include #include +#include #include class PlatformStyle; @@ -51,15 +52,20 @@ public Q_SLOTS: private: Ui::ReceiveCoinsDialog *ui; + QSortFilterProxyModel* m_sort_proxy; WalletModel* model{nullptr}; QMenu *contextMenu; + QAction* copyURIAction; + QAction* copyAddressAction; QAction* copyLabelAction; QAction* copyMessageAction; QAction* copyAmountAction; const PlatformStyle *platformStyle; - QModelIndex selectedRow(); + // Returns QModelIndex in source model + QModelIndexList SelectedRows(); void copyColumnToClipboard(int column); + void ShowReceiveRequestDialogForItem(const QModelIndex& index); private Q_SLOTS: void on_receiveButton_clicked(); diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp index 85ade624cf3..787542d7915 100644 --- a/src/qt/recentrequeststablemodel.cpp +++ b/src/qt/recentrequeststablemodel.cpp @@ -90,11 +90,23 @@ QVariant RecentRequestsTableModel::data(const QModelIndex &index, int role) cons else return BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), rec->recipient.amount); } - } - else if (role == Qt::TextAlignmentRole) - { + } else if (role == Qt::TextAlignmentRole) { if (index.column() == Amount) - return (int)(Qt::AlignRight|Qt::AlignVCenter); + return (int)(Qt::AlignRight | Qt::AlignVCenter); + } else if (role == Qt::UserRole) { + const RecentRequestEntry* rec = &list[index.row()]; + switch (index.column()) { + case Date: + return rec->date; + case Label: + return rec->recipient.label; + case Message: + return rec->recipient.message; + case Amount: + return QVariant(static_cast(rec->recipient.amount)); + default: + return QVariant(); + } } return QVariant(); } @@ -210,35 +222,7 @@ void RecentRequestsTableModel::addNewRequest(RecentRequestEntry &recipient) endInsertRows(); } -void RecentRequestsTableModel::sort(int column, Qt::SortOrder order) -{ - std::sort(list.begin(), list.end(), RecentRequestEntryLessThan(column, order)); - Q_EMIT dataChanged(index(0, 0, QModelIndex()), index(list.size() - 1, NUMBER_OF_COLUMNS - 1, QModelIndex())); -} - void RecentRequestsTableModel::updateDisplayUnit() { updateAmountColumnTitle(); } - -bool RecentRequestEntryLessThan::operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const -{ - const RecentRequestEntry* pLeft = &left; - const RecentRequestEntry* pRight = &right; - if (order == Qt::DescendingOrder) - std::swap(pLeft, pRight); - - switch(column) - { - case RecentRequestsTableModel::Date: - return pLeft->date.toSecsSinceEpoch() < pRight->date.toSecsSinceEpoch(); - case RecentRequestsTableModel::Label: - return pLeft->recipient.label < pRight->recipient.label; - case RecentRequestsTableModel::Message: - return pLeft->recipient.message < pRight->recipient.message; - case RecentRequestsTableModel::Amount: - return pLeft->recipient.amount < pRight->recipient.amount; - default: - return pLeft->id < pRight->id; - } -} diff --git a/src/qt/recentrequeststablemodel.h b/src/qt/recentrequeststablemodel.h index 151f8322a8a..e11be1388f4 100644 --- a/src/qt/recentrequeststablemodel.h +++ b/src/qt/recentrequeststablemodel.h @@ -34,18 +34,6 @@ class RecentRequestEntry } }; -class RecentRequestEntryLessThan -{ -public: - RecentRequestEntryLessThan(int nColumn, Qt::SortOrder fOrder): - column(nColumn), order(fOrder) {} - bool operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const; - -private: - int column; - Qt::SortOrder order; -}; - /** Model for list of recently generated payment requests / bitcoin: URIs. * Part of wallet model. */ @@ -75,7 +63,6 @@ class RecentRequestsTableModel: public QAbstractTableModel QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const override; bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex()) override; Qt::ItemFlags flags(const QModelIndex &index) const override; - void sort(int column, Qt::SortOrder order = Qt::AscendingOrder) override; /*@}*/ const RecentRequestEntry &entry(int row) const { return list[row]; }