Delete QTableWidget selected rows

hbatalha

I have a QTableWidget which in the first column it is filled with a QCheckBox for rows selection.

I have added a button to delete the rows selected, which is done in the following method:

void HistoryWindow::on_delete_Button_clicked()
{
    const auto ckboxlist = ui->table->findChildren<QCheckBox*>();

    for(int i = 0; i < ckboxlist.size(); ++i)
    {
        if(ckboxlist.at(i)->isChecked())
        {            
            ui->table->removeRow(i);

        }
    }
}

But a strange thing happens, only about half of the selected rows get removed. If I select two rows only one gets removed, if 3 are selected only 2 are removed. I have no idea why this happens, the function seems fine to me.

Does anyone have an idea? Thanks in advance!!

Pl45m4

@hbatalha said in Delete QTableWidget selected rows:

const auto ckboxlist = ui->table->findChildren<QCheckBox*>();

The index in your checkBox list may not represent the order of your rows in your widget.
You should check e.g. if CheckBox with index 1 is in row 1.

Christian Ehrlicher

@hbatalha said in Delete QTableWidget selected rows:

But a strange thing happens, only about half of the selected rows get removed. If I select two rows only one gets removed, if 3 are selected only 2 are removed. I have no idea why this happens, the function seems fine to me.

Properly explained in the docs: "If you want to set several items of a particular row (say, by calling setItem() in a loop), you may want to turn off sorting before doing so, and turn it back on afterwards; this will allow you to use the same row argument for all items in the same row (i.e. setItem() will not move the row)."

VRonin

@hbatalha said in Delete QTableWidget selected rows:

for(int i = 0; i < ckboxlist.size(); ++i)

You should reverse iterate.
If you want to remove rows 0 and 3 if you remove row 0 first then row 3 becomes 2 so the deletion fails. for(int i = ckboxlist.size()-1; i >=0 ; --i)

P.S.

ui->table->findChildren<QCheckBox*>();

setIndexWidget()/setCellWidget() should burn in hell!!
To have a checkbox in a QTableWidgetItem use:

tableWidgetItem->setData(Qt::CheckStateRole,Qt::Unchecked); // makes the checkbox appear
tableWidgetItem->setFlags(tableWidgetItem->flags() | Qt::ItemIsUserCheckable); // allows the user to interact with the checkbox

hbatalha

sorry for the late reply

@VRonin said in Delete QTableWidget selected rows:

You should reverse iterate.

This solved the problem.

setIndexWidget()/setCellWidget() should burn in hell!!

Why exactly? What is wrong with them?

tableWidgetItem->setData(Qt::CheckStateRole,Qt::Unchecked); // makes the checkbox appear
tableWidgetItem->setFlags(tableWidgetItem->flags() | Qt::ItemIsUserCheckable); // allows the user to interact with the checkbox

How would I connect the checkbox when clicked? how to checkif it is checked?
I will look more into it but still I'd appreciate your input.

JonB

@hbatalha said in Delete QTableWidget selected rows:

Why exactly? What is wrong with them?

They are resource hogs. If you look @VRonin's signature you will see he is on a personal mission against their usage :) I take a slightly more pragmatic approach: just how many cells (rows * columns) will your table have with these widgets in them? If it's just "a few" I personally do not think you should be crucified for using them, though he may disagree and suggest you will burn in Hell anyway.

VRonin

@hbatalha said in Delete QTableWidget selected rows:

How would I connect the checkbox when clicked?

QObject::connect(tableWidget->model(),&QAbstractItemModel::dataChanged,[](const QModelIndex& topLeft, const QModelIndex& bottomRight, const QVector<int>& roles){
    if(roles.isEmpty() || roles.contains(Qt::CheckStateRole){
        for(int i=topLeft.row();i<bottomRight.row();++i){
            for(int j=topLeft.column();j<bottomRight.column();++j){
                qDebug() << "Checkbox changed in row " << i << " column " << j;
            }
        }
    }
});

how to check if it is checked?

tableWidgetItem->data(Qt::CheckStateRole).value<Qt::CheckState>() == Qt::Checked

hbatalha

@JonB said in Delete QTableWidget selected rows:

just how many cells (rows * columns) will your table have with these widgets in them?

Right now each row has 3 widgets, 1 QCheckBox and 2 QPushButton, and the table can have as many rows as the user wishes.

JonB

@hbatalha said in Delete QTableWidget selected rows:

and the table can have as many rows as the user wishes

So in that case the table can have have as many widgets as the user wishes. If that is "large", @VRonin has warned that you will burn in hell... :)

hbatalha

@VRonin said in Delete QTableWidget selected rows:

@hbatalha said in Delete QTableWidget selected rows:

How would I connect the checkbox when clicked?

QObject::connect(tableWidget->model(),&QAbstractItemModel::dataChanged,[](const QModelIndex& topLeft, const QModelIndex& bottomRight, const QVector<int>& roles){
    if(roles.isEmpty() || roles.contains(Qt::CheckStateRole){
        for(int i=topLeft.row();i<bottomRight.row();++i){
            for(int j=topLeft.column();j<bottomRight.column();++j){
                qDebug() << "Checkbox changed in row " << i << " column " << j;
            }
        }
    }
});

I am trying to change the implementation however it is proving to be quite difficult for me to connect it. I need to connect it in a way that I will know whether I am checking or unchecking the checkbox. I tried this every time I add a row:

        QTableWidgetItem* item = ui->table->item(dest_row, 0);
        item->setData(Qt::CheckStateRole,Qt::Checked); // makes the checkbox appear
        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);

        QObject::connect(ui->table->model(),&QAbstractItemModel::dataChanged,[item](const QModelIndex& topLeft,
                         const QModelIndex& bottomRight, const QVector<int>& roles)
        {
            if(item->data(Qt::CheckStateRole).value<Qt::CheckState>() == Qt::Checked)
            {
                //do something
            }
        });

But it is not practical since it becomes too slow when adding a lot of rows in a loop for example.

VRonin

Are you calling QObject::connect every time you add a row? No, that should only be done once when you create the model (in the constructor)

hbatalha

@VRonin yeah, I just did that to be able to grab the item. What I really want to know is just whether I am checking or unchecking.

VRonin

I see I confused you a bit. See this minimal example on how this works:

#include <QApplication>
#include <QTableWidget>
#include <QPushButton>
#include <QVBoxLayout>
#include <QDebug>

class ExampleWidget : public QWidget{
    Q_DISABLE_COPY(ExampleWidget)
public:
    explicit ExampleWidget(QWidget* parent = nullptr)
        :QWidget(parent)
    {
        tableWidget= new QTableWidget(0,1,this);
        connect(tableWidget->model(),&QAbstractItemModel::dataChanged,this,&ExampleWidget::checkboxChanged);

        addRowButton= new QPushButton(tr("Add Row"),this);
        connect(addRowButton,&QPushButton::clicked,this,&ExampleWidget::addRow);

        QVBoxLayout* mainLay = new QVBoxLayout(this);
        mainLay->addWidget(tableWidget);
        mainLay->addWidget(addRowButton);
    }
private slots:
    void addRow(){
        QTableWidgetItem* item = new QTableWidgetItem;
        const int numRows = tableWidget->rowCount();
        item->setData(Qt::EditRole, numRows+1);
        item->setData(Qt::CheckStateRole,Qt::Checked); // makes the checkbox appear
        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
        tableWidget->insertRow(numRows);
        tableWidget->setItem(numRows,0,item);
    }
    void checkboxChanged(const QModelIndex& topLeft, const QModelIndex& bottomRight, const QVector<int>& roles){
        if(roles.contains(Qt::CheckStateRole)|| roles.isEmpty()){
            for(int i=topLeft.row();i<=bottomRight.row();++i){
                for(int j=topLeft.column();j<=bottomRight.column();++j){
                    qDebug() << "checkbox changed in row " << i << " column " << j;
                    QTableWidgetItem* item = tableWidget->item(i,j);
                    if(item->data(Qt::CheckStateRole).value<Qt::CheckState>() == Qt::Checked)
                        qDebug() << "checkbox is checked";
                    else
                        qDebug() << "checkbox is uncecked";
                }
            }
        }
    }
private:
    QTableWidget* tableWidget;
    QPushButton* addRowButton;
};

int main(int argc, char *argv[])
{
    QApplication app(argc,argv);
    ExampleWidget wid;
    wid.show();
    return app.exec();
}

As you can see the connect is done only once and it's overall really fast. If you don't want your slot to be triggered when a new item is inserted then you just need to remove || roles.isEmpty()
Forgot to mention this requires Qt >= 5.12

hbatalha

@VRonin A good solution but I do have a question:

void checkboxChanged(const QModelIndex& topLeft, const QModelIndex& bottomRight, const QVector<int>& roles){
        if(roles.contains(Qt::CheckStateRole)|| roles.isEmpty()){
            for(int i=topLeft.row();i<=bottomRight.row();++i){
                for(int j=topLeft.column();j<=bottomRight.column();++j){
                    qDebug() << "checkbox changed in row " << i << " column " << j;
                    QTableWidgetItem* item = tableWidget->item(i,j);
                    if(item->data(Qt::CheckStateRole).value<Qt::CheckState>() == Qt::Checked)
                        qDebug() << "checkbox is checked";
                    else
                        qDebug() << "checkbox is uncecked";
                }
            }
        }
    }

From what I could see, isn't that a little expensive to check all the checkboxes every time one is changed, suppose the user has a table with 2 hundreds rows?

Edit: the topLeft.row() is the exact row where the checkbox is modified so it can be used instead of iterating through all the columns and rows.

VRonin

@hbatalha said in Delete QTableWidget selected rows:

to check all the checkboxes

I'm not checking all the checkboxes. The signal tells you that something has changed in the rectangle with corners topLeft bottomRight so i check only that rectangle. 99% of the cases that rectangle is just a single item (topLeft==bottomRight) so you end up checking only 1 item

hbatalha

@VRonin I think I misspoke, when I say "check all the checkboxes" I mean to check all the checkboxes if they are checked or unchecked.

Anyway, I understand how it works now. Thank you very much.

hbatalha

@VRonin Is there a way to change the checkbox color? maybe inside it.

VRonin

See https://doc.qt.io/qt-5/stylesheet-examples.html#customizing-qtableview

The QTableView's checkbox indicator can also be customized. In the following snippet the indicator background-color in unchecked state is customized:
QTableView::indicator:unchecked {
    background-color: #d7d6d5
}

hbatalha

@VRonin It looked kinda weird. So I tried to change the background-color for both checked and unchecked but it only works for one.
This is the result:

I want a similar result I achieved with a QCheckBox by setting the palette:

The code for QCheckBox:

       QCheckBox *title_ckbox = new QCheckBox(title);
        title_ckbox->setToolTip(title);

        QPalette p = title_ckbox->palette( );
        QColor blue( 0, 0, 255 );
        p.setColor( QPalette::Active, QPalette::Base, blue );
        title_ckbox->setPalette(p);

Is it possible to have something like that?

hbatalha

@VRonin I was able to smooth the problem by doing this:

QTableView::indicator:unchecked {
  border: 1px solid blue
 }

It is not exactly what I wanted but it does it for now.