(fix) use 'played' track color also for played/play count delegate

CMakeLists.txt

@@ -982,11 +982,11 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/library/sidebarmodel.cpp
   src/library/starrating.cpp
   src/library/tabledelegates/bpmdelegate.cpp
+  src/library/tabledelegates/checkboxdelegate.cpp
   src/library/tabledelegates/colordelegate.cpp
   src/library/tabledelegates/coverartdelegate.cpp
   src/library/tabledelegates/locationdelegate.cpp
   src/library/tabledelegates/multilineeditdelegate.cpp
-  src/library/tabledelegates/playcountdelegate.cpp
   src/library/tabledelegates/previewbuttondelegate.cpp
   src/library/tabledelegates/stardelegate.cpp
   src/library/tabledelegates/stareditor.cpp

src/library/basetracktablemodel.cpp

@@ -7,11 +7,11 @@
 #include "library/dao/trackschema.h"
 #include "library/starrating.h"
 #include "library/tabledelegates/bpmdelegate.h"
+#include "library/tabledelegates/checkboxdelegate.h"
 #include "library/tabledelegates/colordelegate.h"
 #include "library/tabledelegates/coverartdelegate.h"
 #include "library/tabledelegates/locationdelegate.h"
 #include "library/tabledelegates/multilineeditdelegate.h"
-#include "library/tabledelegates/playcountdelegate.h"
 #include "library/tabledelegates/previewbuttondelegate.h"
 #include "library/tabledelegates/stardelegate.h"
 #include "library/trackcollection.h"
@@ -482,7 +482,7 @@ QAbstractItemDelegate* BaseTrackTableModel::delegateForColumn(
     } else if (index == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_BPM)) {
         return new BPMDelegate(pTableView);
     } else if (index == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_TIMESPLAYED)) {
-        return new PlayCountDelegate(pTableView);
+        return new CheckboxDelegate(pTableView, QStringLiteral("LibraryPlayedCheckbox"));
     } else if (PlayerManager::numPreviewDecks() > 0 &&
             index == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_PREVIEW)) {
         return new PreviewButtonDelegate(pTableView, index);

src/library/tabledelegates/bpmdelegate.cpp

@@ -1,11 +1,8 @@
 #include "library/tabledelegates/bpmdelegate.h"
 
-#include <QCheckBox>
 #include <QDoubleSpinBox>
 #include <QItemEditorCreatorBase>
 #include <QItemEditorFactory>
-#include <QPainter>
-#include <QTableView>
 
 #include "moc_bpmdelegate.cpp"
 
@@ -34,14 +31,7 @@ class BpmEditorCreator : public QItemEditorCreatorBase {
 };
 
 BPMDelegate::BPMDelegate(QTableView* pTableView)
-        : TableItemDelegate(pTableView),
-          m_pCheckBox(new QCheckBox(m_pTableView)) {
-    m_pCheckBox->setObjectName("LibraryBPMButton");
-    // NOTE(rryan): Without ensurePolished the first render of the QTableView
-    // shows the checkbox unstyled. Not sure why -- but this fixes it.
-    m_pCheckBox->ensurePolished();
-    m_pCheckBox->hide();
-
+        : CheckboxDelegate(pTableView, QStringLiteral("LibraryBPMButton")) {
     // Register a custom QItemEditorFactory to override the default
     // QDoubleSpinBox editor.
     m_pFactory = new QItemEditorFactory();
@@ -52,72 +42,3 @@ BPMDelegate::BPMDelegate(QTableView* pTableView)
 #endif
     setItemEditorFactory(m_pFactory);
 }
-
-BPMDelegate::~BPMDelegate() {
-    delete m_pFactory;
-}
-
-void BPMDelegate::paintItem(QPainter* painter,const QStyleOptionViewItem &option,
-                        const QModelIndex& index) const {
-    // NOTE(rryan): Qt has a built-in limitation that we cannot style multiple
-    // CheckState indicators in the same QAbstractItemView. The CSS rule
-    // QTableView::indicator:checked applies to all columns with a
-    // CheckState. This is a big pain if we want to use CheckState roles on two
-    // columns (i.e. the played column and the BPM column) with different
-    // styling. We typically want a lock icon for the BPM check-state and a
-    // check-box for the times-played column and may want more in the future.
-    //
-    // This workaround creates a hidden QComboBox named LibraryBPMButton. We use
-    // the parent QTableView's QStyle with the hidden QComboBox as the source of
-    // style rules to draw a CE_ItemViewItem.
-    //
-    // Here's how you would typically style the LibraryBPMButton:
-    // #LibraryBPMButton::indicator:checked {
-    //   image: url(:/images/library/ic_library_locked.svg);
-    // }
-    // #LibraryBPMButton::indicator:unchecked {
-    //  image: url(:/images/library/ic_library_unlocked.svg);
-    // }
-
-    // Actually QAbstractTableModel::data(index, BackgroundRole) provides the
-    // correct custom background color (track color).
-    // Though, since Qt6 the above style rules would not apply for some reason,
-    // (see bug #11630) which can be fixed by also setting
-    // #LibraryBPMButton::item { border: 0px;}
-    // This however enables some default styles and clears the custom background
-    // color (track color), see bug #12355 ¯\_(ツ)_/¯ Qt is fun!
-    // Fix that by setting the bg color explicitly here.
-    paintItemBackground(painter, option, index);
-
-    QStyleOptionViewItem opt = option;
-    initStyleOption(&opt, index);
-
-    // The checkbox uses the QTableView's qss style, therefore it's not picking
-    // up the 'missing' or 'played' text color via ForegroundRole from
-    // BaseTrackTableModel::data().
-    // Enforce it with an explicit stylesheet. Note: the stylesheet persists so
-    // we need to reset it to normal/highlighted.
-    QColor textColor;
-    if (option.state & QStyle::State_Selected) {
-        textColor = option.palette.color(QPalette::Normal, QPalette::HighlightedText);
-    } else {
-        auto colorData = index.data(Qt::ForegroundRole);
-        if (colorData.canConvert<QColor>()) {
-            textColor = colorData.value<QColor>();
-        } else {
-            textColor = option.palette.color(QPalette::Normal, QPalette::Text);
-        }
-    }
-
-    if (textColor.isValid() && textColor != m_cachedTextColor) {
-        m_cachedTextColor = textColor;
-        m_pCheckBox->setStyleSheet(QStringLiteral(
-                "#LibraryBPMButton::item { color: %1; }")
-                                           .arg(textColor.name(QColor::HexRgb)));
-    }
-
-    QStyle* style = m_pTableView->style();
-    if (style != nullptr) {
-        style->drawControl(QStyle::CE_ItemViewItem, &opt, painter, m_pCheckBox);
-    }
-}

src/library/tabledelegates/bpmdelegate.h

@@ -1,21 +1,12 @@
 #pragma once
 
-#include "library/tabledelegates/tableitemdelegate.h"
+#include "library/tabledelegates/checkboxdelegate.h"
 
-class QCheckBox;
-
-class BPMDelegate : public TableItemDelegate {
+class BPMDelegate : public CheckboxDelegate {
     Q_OBJECT
   public:
     explicit BPMDelegate(QTableView* pTableView);
-    virtual ~BPMDelegate();
-
-    void paintItem(QPainter* painter,
-            const QStyleOptionViewItem& option,
-            const QModelIndex& index) const override;
 
   private:
-    QCheckBox* m_pCheckBox;
     QItemEditorFactory* m_pFactory;
-    mutable QColor m_cachedTextColor;
 };

src/library/tabledelegates/checkboxdelegate.cpp

@@ -0,0 +1,85 @@
+#include "library/tabledelegates/checkboxdelegate.h"
+
+#include <QCheckBox>
+#include <QPainter>
+#include <QTableView>
+
+#include "moc_checkboxdelegate.cpp"
+
+CheckboxDelegate::CheckboxDelegate(QTableView* pTableView, const QString& checkboxName)
+        : TableItemDelegate(pTableView),
+          m_pCheckBox(new QCheckBox(m_pTableView)),
+          m_checkboxName(checkboxName) {
+    m_pCheckBox->setObjectName(checkboxName);
+    // NOTE(rryan): Without ensurePolished the first render of the QTableView
+    // shows the checkbox unstyled. Not sure why -- but this fixes it.
+    m_pCheckBox->ensurePolished();
+    m_pCheckBox->hide();
+}
+
+void CheckboxDelegate::paintItem(QPainter* painter,
+        const QStyleOptionViewItem& option,
+        const QModelIndex& index) const {
+    // NOTE(rryan): Qt has a built-in limitation that we cannot style multiple
+    // CheckState indicators in the same QAbstractItemView. The CSS rule
+    // QTableView::indicator:checked applies to all columns with a
+    // CheckState. This is a big pain if we want to use CheckState roles on two
+    // columns (i.e. the played column and the BPM column) with different
+    // styling. We typically want a lock icon for the BPM check-state and a
+    // check-box for the times-played column and may want more in the future.
+    //
+    // This workaround creates a hidden QComboBox named LibraryBPMButton. We use
+    // the parent QTableView's QStyle with the hidden QComboBox as the source of
+    // style rules to draw a CE_ItemViewItem.
+    //
+    // Here's how you would typically style the LibraryBPMButton:
+    // #LibraryBPMButton::indicator:checked {
+    //   image: url(:/images/library/ic_library_locked.svg);
+    // }
+    // #LibraryBPMButton::indicator:unchecked {
+    //  image: url(:/images/library/ic_library_unlocked.svg);
+    // }
+
+    // Actually QAbstractTableModel::data(index, BackgroundRole) provides the
+    // correct custom background color (track color).
+    // Though, since Qt6 the above style rules would not apply for some reason,
+    // (see bug #11630) which can be fixed by also setting
+    // #LibraryBPMButton::item { border: 0px;}
+    // This however enables some default styles and clears the custom background
+    // color (track color), see bug #12355 ¯\_(ツ)_/¯ Qt is fun!
+    // Fix that by setting the bg color explicitly here.
+    paintItemBackground(painter, option, index);
+
+    QStyleOptionViewItem opt = option;
+    initStyleOption(&opt, index);
+
+    // The checkbox uses the QTableView's qss style, therefore it's not picking
+    // up the 'missing' or 'played' text color via ForegroundRole from
+    // BaseTrackTableModel::data().
+    // Enforce it with an explicit stylesheet. Note: the stylesheet persists so
+    // we need to reset it to normal/highlighted.
+    QColor textColor;
+    if (option.state & QStyle::State_Selected) {
+        textColor = option.palette.color(QPalette::Normal, QPalette::HighlightedText);
+    } else {
+        auto colorData = index.data(Qt::ForegroundRole);
+        if (colorData.canConvert<QColor>()) {
+            textColor = colorData.value<QColor>();
+        } else {
+            textColor = option.palette.color(QPalette::Normal, QPalette::Text);
+        }
+    }
+
+    if (textColor.isValid() && textColor != m_cachedTextColor) {
+        m_cachedTextColor = textColor;
+        m_pCheckBox->setStyleSheet(QStringLiteral(
+                "#%1::item { color: %2; }")
+                                           .arg(m_checkboxName,
+                                                   textColor.name(QColor::HexRgb)));
+    }
+
+    QStyle* style = m_pTableView->style();
+    if (style != nullptr) {
+        style->drawControl(QStyle::CE_ItemViewItem, &opt, painter, m_pCheckBox);
+    }
+}

src/library/tabledelegates/playcountdelegate.h → src/library/tabledelegates/checkboxdelegate.h

@@ -1,19 +1,21 @@
 #pragma once
 
 #include "library/tabledelegates/tableitemdelegate.h"
+#include "util/parented_ptr.h"
 
 class QCheckBox;
 
-class PlayCountDelegate : public TableItemDelegate {
+class CheckboxDelegate : public TableItemDelegate {
     Q_OBJECT
   public:
-    explicit PlayCountDelegate(QTableView* pTableView);
+    explicit CheckboxDelegate(QTableView* pTableView, const QString& checkboxName);
 
     void paintItem(QPainter* painter,
             const QStyleOptionViewItem& option,
             const QModelIndex& index) const override;
 
   private:
-    QTableView* m_pTableView;
     QCheckBox* m_pCheckBox;
+    const QString m_checkboxName;
+    mutable QColor m_cachedTextColor;
 };