Code review: multiple checkboxes as QStyledItemDelegate

Mark81

I kindly ask to check my code below. It is a QStyledItemDelegate I wrote to achieve a specific goal.
My table has an integer column that represents the binary sum of weekdays. I want to display 7 checkboxes to let the user to select the desired combination of weekdays.

I thought it was a simple task, but I ended with a quite articulated code and I'm not sure if it's the best approach or there is another simpler. The good news is it works.

I studied the stardelegate example code and the qcheckbox source code.

#ifndef DELEGATEWEEKDAYS_H
#define DELEGATEWEEKDAYS_H

#include <QStyledItemDelegate>
#include <QPainter>
#include <QBoxLayout>
#include <QCheckBox>
#include <QDebug>
#include <QApplication>
#include <QMouseEvent>

class WeekdaySelector
{
public:
    enum class EditMode { Editable, ReadOnly };

    explicit WeekdaySelector(int value = 0) : _wd(value)
    {
        _wdPress = -1;
    }

    void paint(QPainter *painter, const QRect &rect, const QPalette &palette, EditMode mode) const
    {
        QCheckBox dummy;
        int w = dummy.sizeHint().width();

        painter->save();
        for (int i = 0; i < 7; i++)
        {
            QStyleOptionButton cbOpt;
            cbOpt.rect = rect;
            bool isChecked = _wd & (1 << i);

            if (isChecked) cbOpt.state = QStyle::State_On;
            else cbOpt.state = QStyle::State_Off;

            if (_wdPress == i) cbOpt.state.setFlag(QStyle::State_Sunken);

            painter->translate(6, 0); // gap
            QApplication::style()->drawControl(QStyle::CE_CheckBox, &cbOpt, painter);
            painter->translate(w, 0); // width
        }
        painter->restore();
    }

    QSize sizeHint() const
    {
        QCheckBox dummy;
        QSize size = dummy.sizeHint();
        size.setWidth(size.width() * 7 + 5 * 6); // 7 checkbox + 6 gaps
        return size;
    }

    int weekdays() const { return _wd; }
    void setWeekdays(int wd) { _wd = wd; }
    void toggleWeekday(int pos) { _wd ^= (1 << pos); }
    int pressWeekday() { return _wdPress; }
    void setPressWeekday(int pos) { _wdPress = pos; }

private:
    int _wd;
    int _wdPress;
};
Q_DECLARE_METATYPE(WeekdaySelector)

class EditorWeekdays : public QWidget
{
    Q_OBJECT

public:
    explicit EditorWeekdays(QWidget *parent = nullptr) : QWidget(parent)
    {
        setAutoFillBackground(true);
    }

    WeekdaySelector selector() { return _selector; }
    void setSelector(WeekdaySelector selector) { _selector = selector; }

    QSize sizeHint() const override
    {
        return _selector.sizeHint();
    }

protected:
    void paintEvent(QPaintEvent *event) override
    {
        QPainter painter(this);
        _selector.paint(&painter, rect(), palette(), WeekdaySelector::EditMode::Editable);
    }

    void mousePressEvent(QMouseEvent *e) override
    {
        const int day = checkboxAtPosition(e->position().x());
        if (day != - 1)
        {
            _selector.setPressWeekday(day);
            update();
        }
        QWidget::mousePressEvent(e);
    }

    void mouseReleaseEvent(QMouseEvent *e) override
    {
        const int day = checkboxAtPosition(e->position().x());
        if (day != -1)
        {
            if (day == _selector.pressWeekday()) _selector.toggleWeekday(day);
            _selector.setPressWeekday(-1);
            update();
        }
        QWidget::mouseReleaseEvent(e);
    }

private:
    WeekdaySelector _selector;

    int checkboxAtPosition(int x) const
    {
        QCheckBox dummy;
        int w = dummy.sizeHint().width();

        int x0 = 6;
        if (x < x0) return -1;

        for (int i = 0; i < 7; i++)
        {
            x0 += w;
            if (x < x0) return i;

            x0 += 6;
            if (x < x0) return -1;
        }

        return -1;
    }
};

class DelegateWeekdays : public QStyledItemDelegate
{
    Q_OBJECT

public:
    DelegateWeekdays(QObject *parent = nullptr) : QStyledItemDelegate(parent)
    {
    }

    QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const override
    {
        EditorWeekdays *editor = new EditorWeekdays(parent);
        return editor;
    }

    void setEditorData(QWidget *editor, const QModelIndex &index) const override
    {
        WeekdaySelector selector(index.data().toInt());
        EditorWeekdays *e = qobject_cast<EditorWeekdays *>(editor);
        e->setSelector(selector);

        int value = index.model()->data(index, Qt::EditRole).toInt();
        selector.setWeekdays(value);
    }

    void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const override
    {
        EditorWeekdays *e = qobject_cast<EditorWeekdays *>(editor);
        model->setData(index, QVariant::fromValue(e->selector().weekdays()));
    }

    QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override
    {
        WeekdaySelector selector(index.data().toInt());
        return selector.sizeHint();
    }

    void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override
    {
        WeekdaySelector selector(index.data().toInt());
        QStyleOptionViewItem opt = option;
        initStyleOption(&opt, index);
        selector.paint(painter, opt.rect, opt.palette, WeekdaySelector::EditMode::ReadOnly);
    }
};

#endif // DELEGATEWEEKDAYS_H

As I said it works: it displays the value from the model and the editor allows me to change the weekdays and store the new value into the database.

Questions:

Was all that code necessary? I mean, instead of mimic the behavior of the checkboxes could I have used QCheckbox directly?
Is it correct to create a dummy QCheckBox to retrieve its size?

QCheckBox dummy;
int w = dummy.sizeHint().width();

I'm also trying to add the code to handle the keyboard (i.e. cursor or tab to change the focus and space to toggle the flag), but I'm not able to add the focus border, like this:

according to the documentation when drawing a CE_CheckBox the state flag State_HasFocus should be available, but if I add in my paint() event:

cbOpt.state.setFlag(QStyle::State_HasFocus);
QApplication::style()->drawControl(QStyle::CE_CheckBox, &cbOpt, painter);

nothing happens, the checkboxes are painted without focus:

I read the wrong documentation?

Christian Ehrlicher

@Mark81 said in Code review: multiple checkboxes as QStyledItemDelegate:

cbOpt.state.setFlag(QStyle::State_HasFocus);

Don't you think you should set the flag before calling the painting through the style?
What style do you use? Fusion and Windows are honoring the focus flag through it's base class: https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/styles/qcommonstyle.cpp#n1421

Mark81

@Christian-Ehrlicher yep, that was a typo when I copied 'n pasted - corrected thanks.
I'm using the default style under X11 (Ubuntu).

QApplication a(argc, argv);
qDebug() << a.style()->name();

tells me it's fusion, indeed.

The focus works on the QCheckBox control as from my screenshot, but setting the flag (in the proper order!) does not draw the focus border.

Christian Ehrlicher

Then debug to see why it is not painted. As you can see the style honors the flag.

Mark81

@Christian-Ehrlicher said in Code review: multiple checkboxes as QStyledItemDelegate:

Then debug to see why it is not painted. As you can see the style honors the flag.

How can I debug this kind of issue? This is beyond my knowledge.
With the debugger I checked the flag is actually set, just before call the drawControl() function:

Since the error is of course on my side, what else can I verify?

SGaist

Hi,

This stack overflow thread covers a similar issue.

Basically, you have not initialized the button options with all the required values.

Christian Ehrlicher

Install the Qt debug libs and source code and step into the Qt source code

Mark81

@SGaist said in Code review: multiple checkboxes as QStyledItemDelegate:

This stack overflow thread covers a similar issue.
Basically, you have not initialized the button options with all the required values.

Where can I find in the docs what are all the required values to draw the focus rect?

I tried the following:

QStyleOptionButton opt;
opt.rect = rect;
bool isChecked = _wd & (1 << i);

if (isChecked) opt.state.setFlag(QStyle::State_On);
else opt.state.setFlag(QStyle::State_Off);
if (_wdPress == i) opt.state.setFlag(QStyle::State_Sunken);
 
opt.state.setFlag(QStyle::State_Enabled);
opt.state.setFlag(QStyle::State_HasFocus);

painter->translate(6, 0); // gap
QApplication::style()->drawControl(QStyle::CE_CheckBox, &opt, painter);
painter->translate(w, 0); // width

and also other combinations (with State_FocusAtBorder, State_MouseOver, State_Active, State_Selected, State_Editing, State_KeyboardFocusChange) but I was not able to draw the focus rect.

I'm downloading the source code as @Christian-Ehrlicher kindly suggested, but it seems odd I need to debug the Qt source code if the error is on my side.

Mark81

@Christian-Ehrlicher I did. From what I understand it seems it run the code you highlighted before:

but still no focus rect:

To further digging the issue I placed a QCheckBox on a form and I managed to make the focus rect appeared:

then I placed a breakpoint to inspect the properties of the state variable:

QStyle::State_Enabled | QStyle::State_On | QStyle::State_HasFocus | QStyle::State_Active | QStyle::State_KeyboardFocusChange

and as I've done before, I added them to my delegate:

if (isChecked) opt.state.setFlag(QStyle::State_On);
else opt.state.setFlag(QStyle::State_Off);
if (_wdPress == i) opt.state.setFlag(QStyle::State_Sunken);

opt.state.setFlag(QStyle::State_Enabled);
opt.state.setFlag(QStyle::State_HasFocus);
opt.state.setFlag(QStyle::State_Active);
opt.state.setFlag(QStyle::State_KeyboardFocusChange);

and now it works. But it works even removing the Enabled and Active state.
And I clearly tried these flags before, as I wrote above.

I have no time to reinstall Qt 6.8.1 to test again, but it seems something has changed in 6.8.2.