Skip to content

feat(qt): add shift-click range selection to coin control dialog #7260

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.

Sign up for GitHub

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

Open
PastaPastaPasta wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

feat(qt): add shift-click range selection to coin control dialog #7260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10a9774
feat(qt): add shift-click range selection to coin control dialog
PastaPastaPasta Mar 30, 2026
099bf3b
fix(qt): restrict coin control shift-select anchor to left-clicks on …
PastaPastaPasta Mar 31, 2026
7086905
fix(qt): gate coin control shift-select to checkbox clicks (CodeRabbit)
PastaPastaPasta Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/qt/coincontroldialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ CoinControlDialog::~CoinControlDialog()
delete ui;
}

void CoinControlDialog::refreshLabels()
{
CoinControlDialog::updateLabels(m_coin_control, model, this);
}

// ok button
void CoinControlDialog::buttonBoxClicked(QAbstractButton* button)
{
Expand Down Expand Up @@ -648,6 +653,7 @@ void CoinControlDialog::updateView()

bool treeMode = ui->radioTreeMode->isChecked();
ui->treeWidget->clear();
ui->treeWidget->resetAnchor();
ui->treeWidget->setEnabled(false); // performance, otherwise updateLabels would be called for every checked checkbox
ui->treeWidget->setAlternatingRowColors(!treeMode);
QFlags<Qt::ItemFlag> flgCheckbox = Qt::ItemIsSelectable | Qt::ItemIsEnabled | Qt::ItemIsUserCheckable;
Expand Down
2 changes: 2 additions & 0 deletions src/qt/coincontroldialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class CoinControlDialog : public QDialog
// static because also called from sendcoinsdialog
static void updateLabels(wallet::CCoinControl& m_coin_control, WalletModel*, QDialog*);

void refreshLabels();

static QList<CAmount> payAmounts;
static bool fSubtractFeeFromAmount;

Expand Down
115 changes: 115 additions & 0 deletions src/qt/coincontroltreewidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,21 @@
#include <qt/coincontroltreewidget.h>
#include <qt/coincontroldialog.h>

#include <QStyle>
#include <QStyleOptionViewItem>
#include <QTreeWidgetItemIterator>

CoinControlTreeWidget::CoinControlTreeWidget(QWidget *parent) :
QTreeWidget(parent)
{

}

void CoinControlTreeWidget::resetAnchor()
{
m_lastClickedItem = nullptr;
}

void CoinControlTreeWidget::keyPressEvent(QKeyEvent *event)
{
if (event->key() == Qt::Key_Space) // press spacebar -> select checkbox
Expand All @@ -32,3 +41,109 @@ void CoinControlTreeWidget::keyPressEvent(QKeyEvent *event)
this->QTreeWidget::keyPressEvent(event);
}
}

// Helper: check if an item is a leaf node (UTXO) by its 64-char tx hash
static bool isLeafItem(QTreeWidgetItem* item)
{
int COLUMN_ADDRESS = 3;
return item && item->data(COLUMN_ADDRESS, Qt::UserRole).toString().length() == 64;
}

bool CoinControlTreeWidget::isCheckboxClick(QTreeWidgetItem* item, const QPoint& pos) const
{
int COLUMN_CHECKBOX = 0;

if (!item || columnAt(pos.x()) != COLUMN_CHECKBOX) {
return false;
}

const QModelIndex index = indexFromItem(item, COLUMN_CHECKBOX);
if (!index.isValid()) {
return false;
}

QStyleOptionViewItem option;
option.initFrom(this);
option.rect = visualRect(index);
option.features = QStyleOptionViewItem::HasCheckIndicator;
option.checkState = item->checkState(COLUMN_CHECKBOX);

return style()->subElementRect(QStyle::SE_ItemViewItemCheckIndicator, &option, this).contains(pos);
}

void CoinControlTreeWidget::mouseReleaseEvent(QMouseEvent *event)
{
int COLUMN_CHECKBOX = 0;

QTreeWidgetItem* clickedItem = itemAt(event->pos());
const bool isCheckboxInteraction = isCheckboxClick(clickedItem, event->pos());

bool isShiftClick = (event->button() == Qt::LeftButton)
&& (event->modifiers() & Qt::ShiftModifier)
&& m_lastClickedItem
&& clickedItem
&& clickedItem != m_lastClickedItem
&& isCheckboxInteraction
&& isLeafItem(clickedItem)
&& !clickedItem->isDisabled()
&& !m_lastClickedItem->isDisabled();

if (!isShiftClick) {
// Normal click — let Qt handle the checkbox toggle on release
const Qt::CheckState previousState = clickedItem ? clickedItem->checkState(COLUMN_CHECKBOX) : Qt::Unchecked;
QTreeWidget::mouseReleaseEvent(event);
// Record anchor after toggle so we capture the post-toggle state
if (event->button() == Qt::LeftButton && clickedItem
&& isCheckboxInteraction
&& isLeafItem(clickedItem)
&& !clickedItem->isDisabled()
&& clickedItem->checkState(COLUMN_CHECKBOX) != previousState) {
m_lastClickedItem = clickedItem;
Comment on lines +96 to +101

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update range anchor only on checkbox interactions

The non-shift path records m_lastClickedItem for any left-clicked leaf row, even when that click did not change the checkbox state. In the coin control tree, users can left-click rows for navigation/context without toggling selection, which silently moves the shift-range anchor; a later shift-click then applies an unexpected state across a range of UTXOs. Restrict anchor updates to actual checkbox toggles (or checkbox-indicator clicks) so range selection follows intentional selection actions.

Useful? React with 👍 / 👎.

}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return;
}

Comment on lines +78 to +105

@thepastaclaw thepastaclaw Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Range selection and anchor tracking fire from any column click, not just checkbox interactions

Both the shift-range path (line 63) and the anchor-update path (line 71-74) trigger on any left-click on a leaf row, regardless of which column was clicked. In the existing dialog, clicking a leaf's text columns (amount, address, date, etc.) only changes focus/selection — the checkbox state is only toggled when clicking column 0's checkbox indicator.

After this PR:

  1. Clicking the "Amount" column on row A silently sets A as the anchor
  2. Shift-clicking the "Address" column on row D batch-toggles all coins A through D

Neither click was on a checkbox, but coins are now selected/deselected. This is a behavioral regression from Qt's normal checkbox handling where state changes are tied to actual checkbox activation.

The fix should gate both anchor updates and range application on clicks in the checkbox column. At minimum, check visualItemRect(item) against the checkbox indicator area, or verify that event->pos().x() falls within column 0's geometry via header()->sectionPosition(0) + header()->sectionSize(0).

source: ['codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/qt/coincontroltreewidget.cpp`:
- [SUGGESTION] lines 54-75: Range selection and anchor tracking fire from any column click, not just checkbox interactions
  Both the shift-range path (line 63) and the anchor-update path (line 71-74) trigger on any left-click on a leaf row, regardless of which column was clicked. In the existing dialog, clicking a leaf's text columns (amount, address, date, etc.) only changes focus/selection — the checkbox state is only toggled when clicking column 0's checkbox indicator.

After this PR:
1. Clicking the "Amount" column on row A silently sets A as the anchor
2. Shift-clicking the "Address" column on row D batch-toggles all coins A through D

Neither click was on a checkbox, but coins are now selected/deselected. This is a behavioral regression from Qt's normal checkbox handling where state changes are tied to actual checkbox activation.

The fix should gate both anchor updates and range application on clicks in the checkbox column. At minimum, check `visualItemRect(item)` against the checkbox indicator area, or verify that `event->pos().x()` falls within column 0's geometry via `header()->sectionPosition(0)` + `header()->sectionSize(0)`.

// Shift+click: select/deselect the range between anchor and target
// Read the anchor's current check state live (not cached) so it stays
// correct after bulk operations like Select All or parent tristate changes
Qt::CheckState stateToApply = m_lastClickedItem->checkState(COLUMN_CHECKBOX);

// Collect visible leaf items in display order, skipping children of
// collapsed parent nodes so we don't toggle coins the user can't see
std::vector<QTreeWidgetItem*> leafItems;
int anchorIdx = -1;
int targetIdx = -1;
QTreeWidgetItemIterator it(this);

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restrict shift-range updates to visible rows

The range-building logic starts from QTreeWidgetItemIterator it(this), which walks the full tree structure in pre-order rather than the currently visible rows, so in tree mode it will include children of collapsed address groups. A shift-click from one visible leaf to another can therefore silently toggle hidden UTXOs in collapsed groups, causing unintended coin selection and potentially spending coins the user never saw selected.

Useful? React with 👍 / 👎.

while (*it) {
if (isLeafItem(*it)) {
QTreeWidgetItem* parent = (*it)->parent();
if (!parent || parent->isExpanded()) {
if (*it == m_lastClickedItem) anchorIdx = leafItems.size();
if (*it == clickedItem) targetIdx = leafItems.size();
leafItems.push_back(*it);
}
}
++it;
}

if (anchorIdx < 0 || targetIdx < 0) {
QTreeWidget::mouseReleaseEvent(event);
Comment on lines +129 to +130

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-anchor when the previous item is no longer visible

If a user checks a leaf, collapses its parent, and then Shift-clicks another visible checkbox, the saved anchor is omitted from leafItems, so this fallback performs only the ordinary target toggle. Because it also bypasses the normal path that records m_lastClickedItem, the hidden anchor remains active and subsequent Shift-clicks continue failing until the user performs another normal checkbox click; reset or replace the anchor in this fallback.

Useful? React with 👍 / 👎.

return;
Comment on lines +129 to +131

@thepastaclaw thepastaclaw Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Replace an anchor that is no longer visible

When the anchor's parent is collapsed, the anchor is deliberately omitted from leafItems, leaving anchorIdx negative. The fallback delegates the Shift-click to Qt, which toggles the visible target normally, but it neither clears the hidden anchor nor records the newly toggled target. Every subsequent Shift-click therefore repeats this fallback instead of applying a range until the user performs an unmodified checkbox click. After normal handling, replace the unusable anchor with the target if its checkbox changed; otherwise clear the anchor.

Suggested change
if (anchorIdx < 0 || targetIdx < 0) {
QTreeWidget::mouseReleaseEvent(event);
return;
if (anchorIdx < 0 || targetIdx < 0) {
const Qt::CheckState previousState = clickedItem->checkState(COLUMN_CHECKBOX);
QTreeWidget::mouseReleaseEvent(event);
if (clickedItem->checkState(COLUMN_CHECKBOX) != previousState) {
m_lastClickedItem = clickedItem;
} else {
resetAnchor();
}
return;
}

source: ['codex']

}

if (anchorIdx > targetIdx) std::swap(anchorIdx, targetIdx);

// Batch update: disable widget to suppress per-item updateLabels calls
setEnabled(false);
for (int i = anchorIdx; i <= targetIdx; ++i) {
QTreeWidgetItem* item = leafItems[i];
if (!item->isDisabled() && item->checkState(COLUMN_CHECKBOX) != stateToApply) {
item->setCheckState(COLUMN_CHECKBOX, stateToApply);
}
}
setEnabled(true);

// Single label update for the whole batch
CoinControlDialog* coinControlDialog = qobject_cast<CoinControlDialog*>(this->parentWidget());
if (coinControlDialog) coinControlDialog->refreshLabels();
}
Comment on lines +74 to +149

@thepastaclaw thepastaclaw Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: No automated coverage for new shift-click selection logic

No Qt test covers the new mouse-release handler. Deterministic cases include recording an anchor only after a real checkbox toggle, applying ranges in list and tree order, skipping disabled coins, excluding collapsed groups, recovering from a hidden anchor, and resetting the anchor when updateView() rebuilds the tree. These behaviors depend on Qt event sequencing and the range path bypasses the default release handler, so manual testing alone leaves the feature vulnerable to silent interaction regressions.

source: ['codex']

10 changes: 9 additions & 1 deletion src/qt/coincontroltreewidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define BITCOIN_QT_COINCONTROLTREEWIDGET_H

#include <QKeyEvent>
#include <QMouseEvent>
#include <QTreeWidget>

class CoinControlTreeWidget : public QTreeWidget
Expand All @@ -14,9 +15,16 @@ class CoinControlTreeWidget : public QTreeWidget

public:
explicit CoinControlTreeWidget(QWidget *parent = nullptr);
void resetAnchor();

protected:
virtual void keyPressEvent(QKeyEvent *event) override;
void keyPressEvent(QKeyEvent *event) override;
void mouseReleaseEvent(QMouseEvent *event) override;

private:
bool isCheckboxClick(QTreeWidgetItem* item, const QPoint& pos) const;

QTreeWidgetItem* m_lastClickedItem{nullptr};
};

#endif // BITCOIN_QT_COINCONTROLTREEWIDGET_H
Loading