Multiselect in coincontrol treewidget and display selected count #11750

Elbandi commented at 10:22 AM on November 22, 2017: contributor

This patch allow multiple out selection in coincontrol dialog. Only for changing the checked state, locking need single select.

usecase: someone gets lost of payments from zpool/miningrigrentals/nicehash/etc, its easy to select inputs for sending (no need to click many times in intems or space-down-space-down-space-down... by keyboard)

fanquake added the label GUI on Nov 22, 2017

fanquake added the label Wallet on Nov 22, 2017

in src/qt/coincontroltreewidget.cpp:11 in 32846d2e8e outdated

   7 | @@ -8,17 +8,18 @@
   8 |  CoinControlTreeWidget::CoinControlTreeWidget(QWidget *parent) :
   9 |      QTreeWidget(parent)
  10 |  {
  11 | -
  12 | +    setSelectionMode( QAbstractItemView::ExtendedSelection);

promag commented at 1:56 PM on November 22, 2017:

Remove space after (.

in src/qt/coincontroldialog.cpp:425 in 32846d2e8e outdated

 417 | @@ -416,6 +418,18 @@ void CoinControlDialog::updateLabelLocked()
 418 |      else ui->labelLocked->setVisible(false);
 419 |  }
 420 |  
 421 | +// shows count of selected outputs
 422 | +void CoinControlDialog::updateLabelSelected()
 423 | +{
 424 | +    QList<QTreeWidgetItem *> selected = ui->treeWidget->selectedItems();
 425 | +    if (selected.size() > 0)

promag commented at 1:57 PM on November 22, 2017:

How about:

int count = selected.size();
ui->labelSelected->setVisible(count > 0);
ui->labelSelected->setText(tr("(%1 selected)").arg(count));

Elbandi commented at 3:40 PM on November 22, 2017:

It hides the "(X selected)" label if no selection. like locked count

promag commented at 3:46 PM on November 22, 2017:

ui->labelSelected->setVisible(count > 0); should hide when there's no selection.

in src/qt/forms/coincontroldialog.ui:381 in 32846d2e8e outdated

 374 | @@ -375,6 +375,13 @@
 375 |            </property>
 376 |           </widget>
 377 |          </item>
 378 | +        <item>
 379 | +         <widget class="QLabel" name="labelSelected">
 380 | +          <property name="text">
 381 | +           <string notr="true">(1 selected)</string>

promag commented at 2:00 PM on November 22, 2017:

Incorrect?

Elbandi commented at 3:42 PM on November 22, 2017:

???, placeholder string, as in labelLocked

promag commented at 3:51 PM on November 22, 2017:

Don't know why that is there too, and I think there is no special reason. I guess it's ok since it is consistent with others.

in src/qt/coincontroltreewidget.cpp:19 in 32846d2e8e outdated

  16 |  {
  17 |      if (event->key() == Qt::Key_Space) // press spacebar -> select checkbox
  18 |      {
  19 |          event->ignore();
  20 | -        if (this->currentItem()) {
  21 | +        for (int i = 0; i < this->selectedItems().count(); i++) {

promag commented at 2:03 PM on November 22, 2017:

for (QTreeWidgetItem* item : selectedItems()) {

in src/qt/coincontroltreewidget.cpp:16 in 32846d2e8e outdated

  12 | +    setSelectionMode( QAbstractItemView::ExtendedSelection);
  13 |  }
  14 |  
  15 |  void CoinControlTreeWidget::keyPressEvent(QKeyEvent *event)
  16 |  {
  17 |      if (event->key() == Qt::Key_Space) // press spacebar -> select checkbox

promag commented at 2:04 PM on November 22, 2017:

Comment should be "press spacebar -> toggle checkbox"?

promag commented at 2:05 PM on November 22, 2017: contributor

Care to upload a screenshot with the diff?

Multiselect in coincontrol treewidget and display selected count 7cec76f81b

Elbandi force-pushed on Nov 23, 2017

Elbandi commented at 1:29 PM on November 23, 2017: contributor

new commit pushed

jonasschnelli commented at 11:59 PM on November 29, 2017: contributor

Tested a bit. The multiselection works well.

I got confused by the selection count indicator label. It's confusing because you have the first button "(un)select all" where the new "(1) selected" has a different context for "selected".

Maybe remove the label (just add the multiselection possibility) or use a different help text for the new label... though I don't think it's necessary to have a such label (rarely saw this in multiselection use-cases).

Elbandi commented at 12:38 PM on November 30, 2017: contributor

Rename the button to "(un)check all", eventually thats the right function.

laanwj commented at 10:09 AM on February 12, 2018: member

It's confusing because you have the first button "(un)select all" where the new "(1) selected" has a different context for "selected".

I have the same opinion here; the coin selection is already inherently multi-select, as multiple outputs can be checked. So adding another layer of multi-selection on top is confusing. It's no longer clear whether selection refers to "toggled checkbox" or "selected row".

luke-jr commented at 3:29 PM on February 26, 2018: member

I agree this is confusing, yet also see the validity of the use case.

Maybe @jonasschnelli 's idea + renaming the button to "(Un)check all" would be sufficient.

jonasschnelli commented at 7:24 PM on April 10, 2018: contributor

I think we should remove the new label. Just allow the spacebar toggling. Seems much more clear to me then confusing with "selection" (multiple contexts of selection)

laanwj added the label Up for grabs on May 14, 2018

laanwj commented at 12:05 PM on May 14, 2018: member

Last post by the author was some time last year. Going to add "Up for grabs" label and closing.

laanwj closed this on May 14, 2018

Elbandi commented at 12:45 PM on May 17, 2018: contributor

Sorry, i didnt know, what is the latest consensus about the text? i can change the code, just someone say "lets be this and that"

luke-jr commented at 9:05 AM on April 4, 2019: member

@Elbandi I guess do what @jonasschnelli suggested. When you're ready, ping someone to reopen the PR before you push the new version.

bitcoin locked this on Dec 16, 2021

fanquake removed the label Up for grabs on Jul 23, 2025