1

I have a QTableWidget, where one column is filled with custom checkboxes. Also, I have implemented an undo mechanic so that every change to the table can be undone. For the other item columns, where only text is stored, I basically achieve it in the following way:
Every time an item in the table is pressed (calling the itemPressed signal), I store the table data before the item editing starts using a function called saveOldState. After editing (and triggering an itemChanged signal), I push the actual widget content together with the old content onto a QUndoStack instance using a function called pushOnUndoStack.

Now I want to achieve a similar thing for the cell widgets. However, changing the checkbox state does not trigger itemChanged. Thus, I have to connect to the checkbox's stateChanged signal to save the new state:

QObject::connect(checkBox, &QCheckBox::stateChanged, this, [checkBox] {
    pushOnUndoStack();
});

So, getting the newest table data is not that hard. However, I am struggling to find the right moment to save the data before the checkbox is set, because there is no similar variant for an itemPressed signal in case of a cell widget.
My question is: Is there a good alternative way to store the checkbox state immediately before the state is actually set? Currently, my only idea is to implement a custom mouse move event filter for the cell widget, which calls saveOldState the moment a user moves the mouse inside the cell widget's boundaries. But is there maybe a better way?

Bakefish
  • 81
  • 6
  • Let me set aside for a moment that you are working with a checkbox and consider you are working with data of any type, with some item delegate (you may have not changed the default delegate of your table). You seem to say you want the data to be saved before the editor of the item delegate is changed, which is a "long time" before the data is committed into the model. Any reason for that? Would it work for you to save the state a little bit later (but still before the commit into the model)? – Atmo Jan 31 '23 at 14:39
  • I guess it does not really matter, when exactly the data is changed, as long as it is before the commit back into the model. So yes, you are absolutely right, a little bit later might work as well. – Bakefish Jan 31 '23 at 15:41
  • I think I have a way to answer your question but it needs a little bit of work to get the details down. Does your widget need to be a `QTableWidget`? Would it be possible to have a `QTableView` instead? Worst case scenario, showing a `QStandardItemModel`? – Atmo Feb 01 '23 at 04:49
  • I can rephrase my questions as: Do you actually need the` QTableWidgetItem` to make your view work (the methods provided by `QAbstractItemModel` should allow you to do everything already)? And if you do, do you think it is not possible to replace them by the `QStandardItem` provided by `QStandardItemModel`? – Atmo Feb 01 '23 at 04:50
  • Since I am using lots of methods provided by a `QTableWidget`, I am afraid I do need that... but shouldn't it be able to use a `QTableView` anway, since `QTableWidget` derives from that? – Bakefish Feb 01 '23 at 12:17
  • `QTableWidget` gets you started faster but at the cost of being less flexible. Consequence: what I think is the best solution does not work with `QTableWidget`... I hope my answer below is explanatory enough on why it is so (*chapter 1*) + how a workaround might do the trick, even with the added constraint (*chapter 3*). – Atmo Feb 01 '23 at 14:53

1 Answers1

0

Chapter 1: the solution you cannot use

What I think is the correct way to address your question is a proxy model in charge of maintaining the undo stack. It does so by saving the model's data right before changing it.

Header:

class MyUndoModel : public QIdentityProxyModel
{
public:
    MyUndoModel(QObject* parent = nullptr);
    bool setData(const QModelIndex& index, const QVariant& data, int role) override;
    bool restoreData(const QModelIndex& index, const QVariant& data, int role);

private:
    void pushOnUndoStack(const QPersistentModelIndex& index, int role, const QVariant& value) const;
    //QUndoStack undoStack;   
};

Source:

bool MyUndoModel::setData(const QModelIndex& index, const QVariant& data, int role)
{
    QVariant currentData = index.data(role);
    bool result = QIdentityProxyModel::setData(index, data, role);
    if (result) {
        //If the source model accepted the change, push currentData to the undo stack.
        pushOnUndoStack(QPersistentModelIndex(index), role, currentData );
    }
    return result;
}

bool MyUndoModel::restoreData(const QModelIndex& index, const QVariant& data, int role)
{
    return QIdentityProxyModel::setData(index, data, role);
}

Note that we use QPersistentModelIndexes in a modified version of pushOnUndoStack (that I let you implement yourself). Also, I did not write how the stacked undo/redo commands should be processed, apart from calling restoreData. As long as you get the idea...

Chapter 2: where it fails for you

The above solution works regarless of the actual class of the source model ... except if working with QTableWidget and QTreeWidget.

What blocks this solution in the case of e.g. QTableWidget is its internal model (QTableModel).

  • You cannot substitute model of your QTableWidget to use MyUndoModel instead.
    If you try, you will very quickly see your application crash.
  • You could in theory subclass QTableModel to perform the above substitution but I advise against it.
    Sample: myTableWidget->QTableView::setModel(new MyQTableModel);
    QTableModel is a private class in Qt and should not be used directly. I wish I knew why it was done this way.

Chapter 3:The alternative solution

Alternatively, subclassing QStyledItemDelegate could work for you. The design is not as clean, there are more ways to make a mistake when using it in your window but it essentially follows the same logic as the above proxy model.

class UndoItemDelegate : protected QStyledItemDelegate
{
public:
    UndoItemDelegate(QUndoStack* undoStack, QObject* parent = nullptr);
    //Importnt: we set setModelData as final.
    void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const override final;

protected:
    virtual QVariant valueFromEditor(QWidget *editor) const noexcept = 0;
    virtual int roleFromEditor(QWidget *editor) const noexcept = 0;
    
private:
    void pushOnUndoStack(const QPersistentModelIndex& index, int role, const QVariant& value) const; 
    //undoStack as a pointer makes it possible to share it across several delegates of the same view (or of multiple view)
    mutable QUndoStack* undoStack;
};

The magic is in setModelData.

void UndoItemDelegate::setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const
{
    auto role = roleFromEditor(editor);
    QVariant currentData = index.data(role);
    
    bool dataChanged = model->setData(index, valueFromEditor(editor), role);
    if (dataChanged && undoStack) {
        pushOnUndoStack(QPersistentModelIndex(index), role, currentData);
    }
}

I kept the version with index (my habit) but you could use the pointers to QTableItem of course.

To be used (most likely in the constructor of your window):

ui->setupUi(this);
auto myDelegate = new MyUndoItemDelegateSubclass(&windowUndoStack, ui->myTableWidget);
ui->myTableWidget->setItemDelegate(myDelegate);

You will have to implement:

  • pushOnUndoStack (once).
  • roleFromEditor and valueFromEditor (for every subclass).
  • the processing of undo/redo commands.

Edit to address your comment.
I am going to assume you know how QAbstractIdemModel and subclasses work in a generic manner. To manipulate a checkState in the model of a QTableWidget, I recommend you create a UndoCheckboxDelegate subclass to implement/override the additional methods this way:

Header:

class UndoCheckboxDelegate : public UndoItemDelegate
{
public:
    UndoCheckboxDelegate(QUndoStack* undoStack, QObject* parent = nullptr);
    QWidget* createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
protected:
    virtual QVariant valueFromEditor(QWidget *editor) const noexcept override;
    virtual int roleFromEditor(QWidget *editor) const noexcept override;
};

Source:

UndoCheckboxDelegate::UndoCheckboxDelegate(QUndoStack* undoStack, QObject* parent)
    : UndoItemDelegate(undoStack, parent)
{}

QWidget* UndoCheckboxDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const
{
    if (index.isValid()) {
        QCheckBox* control = new QCheckBox(parent);
        control->setText(index.data(Qt::DisplayRole).toString());
        control->setCheckState(index.data(Qt::CheckStateRole).value<Qt::CheckState>());
        return control;
    }
    else
        return nullptr;
}
QVariant UndoCheckboxDelegate::valueFromEditor(QWidget *editor) const noexcept
{
    if (editor)
        return static_cast<QCheckBox*>(editor)->checkState();
    else
        return QVariant();
}
int UndoCheckboxDelegate::roleFromEditor(QWidget * /* unused */) const noexcept
{
    return Qt::CheckStateRole;
}

It may be only a starting point for you. Make sure it correctly fills the undo stack first; after that, you can tweak the behavior a bit.

Atmo
  • 2,281
  • 1
  • 2
  • 21
  • Okay, I understand the basic idea of your solution. Thanks so far! But I have some questions regarding the third answer: First, is the `currentData` variable the data _before_ the change? And what exactly do the `roleFromEditor` and `valueFromEditor` stand for? And would my checkbox be a subclass item? – Bakefish Feb 02 '23 at 09:11
  • Indeed `currentData` is the data before the change and `valueFromEditor` returns the value you want to set instead of it. I will answer the rest of your comment in my previous answer – Atmo Feb 02 '23 at 12:00
  • Okay, now with your edits things make much more sense to me. I will try that out in a few days. Many thanks for the extra explanations! – Bakefish Feb 02 '23 at 19:11