QFileSystemModel set item color issue

Christian Ehrlicher

QAbstractItemView::setData() returns a boolean to indicate if the value was really set. Since you're using a plain QFileSystemModel I doubt it somehow implemented something in the setData() function for the BackgroundRole.
Therefore you have to derive from QFileSystemModel and implement it on your own.

Cobra91151

@Christian-Ehrlicher

Ok. I tried it and it returns false.

bool res = ui->listView->model()->setData(ui->listView->currentIndex(), QBrush(Qt::gray), Qt::BackgroundRole);
qDebug() << res;

So, no chance to set the item color by QFileSystemModel default behavior/methods? Should I really reimplement QFileSystemModel model?

Christian Ehrlicher

@Cobra91151 said in QFileSystemModel set item color issue:

I tried it and it returns false.

The code I posted the link to shows that QFileSystemModel does not handle Qt::BackgroundRole so anything other than false would be a surprise here.

Should I really reimplement QFileSystemModel model?

No, you should derive from it

Cobra91151

@Christian-Ehrlicher

Ok, by derive you mean to inherit QFileSystemModel class and reimplement the setData method to accept other roles? Because from this code it only accepts EditRole.

...
if (!idx.isValid()
       || idx.column() != 0
       || role != Qt::EditRole
       || (flags(idx) & Qt::ItemIsEditable) == 0) {
       return false;
   }

JonB

@Cobra91151
Yes that is exactly what @Christian-Ehrlicher means! Sub-class, override setData(), implement your own logic for Qt::BackgroundRole role, else call the base implementation.

Cobra91151

@JonB

Ok. I will try it. Thanks.

Cobra91151

@JonB

Ok. I have tried it, but still it does not change the item background color.

Code:

bool TestModel::setData(const QModelIndex &index, const QVariant &varData, int role)
{
    QStandardItem item(index.row(), index.column());

    if (role == Qt::BackgroundRole) {
        QBrush brush = varData.value<QBrush>();
        item.setBackground(brush);
        qDebug() << "setBackground";
    }

    return QFileSystemModel::setData(index, varData, role);
}

Christian Ehrlicher

What should work here? You're creating a local QStandardItem, setting the background color and then it gets destroyed. How should work this at all? How should data() magically return the correct Background role with your code?

Cobra91151

@Christian-Ehrlicher

So, I changed code to:

bool TestModel::setData(const QModelIndex &index, const QVariant &varData, int role)
{
    QStandardItem *item = new QStandardItem(index.row(), index.column());

    if (role == Qt::BackgroundRole) {
        QBrush brush = varData.value<QBrush>();
        item->setBackground(brush);
        qDebug() << "setBackground";
        return item->data(Qt::BackgroundRole).toBool();
    }

    return QFileSystemModel::setData(index, varData, role);
}

But the issue still exists. What logic should I implement then? Thanks.

Christian Ehrlicher

You really should take a look at how the ItemModels work... https://doc.qt.io/qt-5/model-view-programming.html
You set data with setData() and you return data with data() - so where did you implement your data() function? What should the (now leaking) item do at all?

Cobra91151

@Christian-Ehrlicher

Good. I will check it. Thanks.

JonB

@Cobra91151
So I understand: the user clicks "cut" on your QListView, and you want to change item color displayed while it's cut? And you propose to do that via the QFileSystemModel's setData(Qt::BackgroundRole)? So you're going to store (at least conceptually) an "is this item presently cut" against each item?

@Christian-Ehrlicher
Given the code shown for QFileSystemModel::setData() rejects attempts to set Qt::BackgroundRole, are you going to/can you use QAbstractItemModel::setItemData(const QModelIndex &index, const QMap<int, QVariant> &roles) to specify a value for role Qt::BackgroundRole to "force" the Qt::BackgroundRole into the data, ready to picked up by data(Qt::BackgroundRole)? Or does that actually call setData()?! In which case, where will you store the "cut" data so that data() retrieves it?

A possible alternative approach, avoiding any setData() overriding: don't store "cut" status against each item. Just keep a variable for what item(s) is cut. (You could possibly just use the selection model for that, indicating whether it's a "cut" vs a "copy"; then when the selection goes away it's automatically no longer in "cut" state?) Override only data(): when role is Qt::BackgroundRole look up the QModelIndex in your variable/list of "cut" ones and return QBrush(Qt::gray) if there. Comment?

Cobra91151

@JonB

I have cut option via context menu. It works using QClipboard and works well. The reason I want to change color of the item is to notify user that such item has been cut. So, basically it is only the style, hence I can do a QMessageBox or QLabel to notify. I am still figuring out what should I reimplement to just set the item color.

JonB

@Cobra91151

Override data() to unconditionally return QBrush(Qt::gray) when role is Qt::BackgroundRole. Check the principle works, and visuals look OK.
Override setData() to store the index (QModelindex) in some container when you want to mark the item as being "cut".
Alter the data() override code to check whether the index is marked in the "cut" container, and only return QBrush(Qt::gray) for Qt::BackgroundRole when it is.

Hint: container should be directly accessible by QModelIndex for it (#3) to be fast.

Cobra91151

@Christian-Ehrlicher @JonB

Ok. I figured it out. My code:

TestModel.h

#ifndef TESTMODEL_H
#define TESTMODEL_H

#include <QObject>
#include <QFileSystemModel>
#include <QSet>
#include <QBrush>

class TestModel : public QFileSystemModel
{

    Q_OBJECT
public:
    using QFileSystemModel::QFileSystemModel;
    QVariant data(const QModelIndex &index, int role) const override;
    bool setData(const QModelIndex &index, const QVariant &varData, int role) override;

private:
    QSet<QPersistentModelIndex> indexList;
};

#endif // TESTMODEL_H

TestModel.cpp

#include "testmodel.h"

QVariant TestModel::data(const QModelIndex &index, int role) const {
    if (role == Qt::BackgroundRole) {
        return indexList.contains(index) ? QBrush(Qt::gray) : QBrush(Qt::transparent);
    }

    return QFileSystemModel::data(index, role);
}

bool TestModel::setData(const QModelIndex &index, const QVariant &value, int role) {
    if (role == Qt::BackgroundRole) {
        if (value == QBrush(Qt::gray)) {
            indexList.insert(index);
        } else {
            indexList.remove(index);
        }

        emit dataChanged(index, index, {Qt::BackgroundRole});
        return true;
    }

    return QFileSystemModel::setData(index, value, role);
}

Now it works well. The issue is resolved!

Christian Ehrlicher

dataChanged() should pass the modified role -> dataChanged(idx, idx, {Qt::BackgroundRole}) and the two ctors and flags() function is not needed in your class.
In the class definition data() and setData(9 should be marked with 'override'

Cobra91151

@Christian-Ehrlicher

Ok. I improved dataChanged method and marked data() and setData() with override. Thanks.

When I removed constructors I got compiling issues. It is required to have at least one constructor. About flags, I think it is also required because I am implementing drag and drop with models and views.

Christian Ehrlicher

You don't do anything in your flags() so no need for it

wrt the ctors:

class TestModel : public QFileSystemModel
{
    Q_OBJECT
public:
    using QFileSystemModel::QFileSystemModel;
...

Cobra91151

@Christian-Ehrlicher

Good. I changed it. Thanks.