Skip to content

Commit

Permalink
Merge pull request #13538 from ronso0/played-delegate-color-fix
Browse files Browse the repository at this point in the history
(fix) use 'played' track color also for played/play count delegate
  • Loading branch information
JoergAtGithub authored Aug 6, 2024
2 parents ba773d0 + 4771726 commit da02fcc
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 133 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/library/basetracktablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down
81 changes: 1 addition & 80 deletions src/library/tabledelegates/bpmdelegate.cpp
Original file line number Diff line number Diff line change
@@ -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"

Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
}
13 changes: 2 additions & 11 deletions src/library/tabledelegates/bpmdelegate.h
Original file line number Diff line number Diff line change
@@ -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;
};
85 changes: 85 additions & 0 deletions src/library/tabledelegates/checkboxdelegate.cpp
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
};
36 changes: 0 additions & 36 deletions src/library/tabledelegates/playcountdelegate.cpp

This file was deleted.

0 comments on commit da02fcc

Please sign in to comment.