Color preferences improvements

src/engine/controls/cuecontrol.cpp

@@ -615,12 +615,16 @@ void CueControl::hotcueSet(HotcueControl* pControl, double v) {
     pCue->setLabel();
     pCue->setType(mixxx::CueType::HotCue);
 
-    ConfigKey autoHotcueColorsKey("[Controls]", "auto_hotcue_colors");
-    if (getConfig()->getValue(autoHotcueColorsKey, false)) {
-        auto hotcueColorPalette = m_colorPaletteSettings.getHotcueColorPalette();
+    const ColorPalette hotcueColorPalette =
+            m_colorPaletteSettings.getHotcueColorPalette();
+    if (getConfig()->getValue(ConfigKey("[Controls]", "auto_hotcue_colors"), false)) {
         pCue->setColor(hotcueColorPalette.colorForHotcueIndex(hotcue));
     } else {
-        pCue->setColor(mixxx::PredefinedColorPalettes::kDefaultCueColor);
+        int hotcueDefaultColorIndex = m_pConfig->getValue(ConfigKey("[Controls]", "HotcueDefaultColorIndex"), -1);
+        if (hotcueDefaultColorIndex < 0 || hotcueDefaultColorIndex >= hotcueColorPalette.size()) {
+            hotcueDefaultColorIndex = hotcueColorPalette.size() - 1; // default to last color (orange)
+        }
+        pCue->setColor(hotcueColorPalette.at(hotcueDefaultColorIndex));
     }
 
     // TODO(XXX) deal with spurious signals

src/preferences/colorpaletteeditor.cpp

@@ -6,6 +6,7 @@
 #include <QHeaderView>
 #include <QLabel>
 #include <QMenu>
+#include <QMessageBox>
 #include <QStandardItemModel>
 #include <QTableView>
 
@@ -17,34 +18,53 @@ const QColor kDefaultPaletteColor(0, 0, 0);
 }
 
 ColorPaletteEditor::ColorPaletteEditor(QWidget* parent)
-        : QWidget(parent),
+        : QDialog(parent),
           m_bPaletteExists(false),
           m_bPaletteIsReadOnly(false),
-          m_pPaletteNameComboBox(make_parented<QComboBox>()),
-          m_pTableView(make_parented<QTableView>()),
+          m_pSaveAsEdit(make_parented<QLineEdit>(this)),
+          m_pTableView(make_parented<QTableView>(this)),
           m_pModel(make_parented<ColorPaletteEditorModel>(m_pTableView)) {
-    m_pPaletteNameComboBox->setEditable(true);
+    // Create widgets
+    QHBoxLayout* pColorButtonLayout = new QHBoxLayout();
+    m_pAddColorButton = new QPushButton(tr("Add Color"), this);
+    pColorButtonLayout->addWidget(m_pAddColorButton);
+    connect(m_pAddColorButton,
+            &QPushButton::clicked,
+            this,
+            &ColorPaletteEditor::slotAddColor);
+    m_pRemoveColorButton = new QPushButton(tr("Remove Color"), this);
+    pColorButtonLayout->addWidget(m_pRemoveColorButton);
+    connect(m_pRemoveColorButton,
+            &QPushButton::clicked,
+            this,
+            &ColorPaletteEditor::slotRemoveColor);
 
-    QDialogButtonBox* pButtonBox = new QDialogButtonBox();
-    m_pSaveButton = pButtonBox->addButton(QDialogButtonBox::Save);
-    m_pResetButton = pButtonBox->addButton(QDialogButtonBox::Reset);
-    m_pDiscardButton = pButtonBox->addButton(QDialogButtonBox::Discard);
+    QHBoxLayout* pNameLayout = new QHBoxLayout();
+    pNameLayout->addWidget(new QLabel(tr("Name")));
+    pNameLayout->addWidget(m_pSaveAsEdit, 1);
 
-    QHBoxLayout* pTopLayout = new QHBoxLayout();
-    pTopLayout->addWidget(new QLabel("Name:"));
-    pTopLayout->addWidget(m_pPaletteNameComboBox, 1);
-    pTopLayout->addWidget(pButtonBox);
+    QDialogButtonBox* pPaletteButtonBox = new QDialogButtonBox();
+    m_pRemoveButton = pPaletteButtonBox->addButton(
+            tr("Remove Palette"),
+            QDialogButtonBox::DestructiveRole);
+    m_pCloseButton = pPaletteButtonBox->addButton(QDialogButtonBox::Discard);
+    m_pResetButton = pPaletteButtonBox->addButton(QDialogButtonBox::Reset);
+    m_pSaveButton = pPaletteButtonBox->addButton(QDialogButtonBox::Save);
 
+    // Add widgets to dialog
     QVBoxLayout* pLayout = new QVBoxLayout();
-    pLayout->addLayout(pTopLayout);
+    pLayout->addLayout(pColorButtonLayout);
     pLayout->addWidget(m_pTableView, 1);
+    pLayout->addLayout(pNameLayout);
+    pLayout->addWidget(pPaletteButtonBox);
     setLayout(pLayout);
     setContentsMargins(0, 0, 0, 0);
 
     // Set up model
-    m_pModel->setColumnCount(2);
+    m_pModel->setColumnCount(3);
     m_pModel->setHeaderData(0, Qt::Horizontal, tr("Color"), Qt::DisplayRole);
-    m_pModel->setHeaderData(1, Qt::Horizontal, tr("Assign to Hotcue"), Qt::DisplayRole);
+    m_pModel->setHeaderData(1, Qt::Horizontal, tr("Assign to Hotcue Number"), Qt::DisplayRole);
+    m_pModel->setHeaderData(2, Qt::Horizontal, QString(), Qt::DisplayRole);
     connect(m_pModel,
             &ColorPaletteEditorModel::dirtyChanged,
             this,
@@ -63,85 +83,95 @@ ColorPaletteEditor::ColorPaletteEditor(QWidget* parent)
     m_pTableView->setContextMenuPolicy(Qt::CustomContextMenu);
     m_pTableView->setModel(m_pModel);
 
+    m_pTableView->horizontalHeader()->setSectionResizeMode(0, QHeaderView::ResizeToContents);
+    m_pTableView->horizontalHeader()->setSectionResizeMode(1, QHeaderView::ResizeToContents);
+    m_pTableView->horizontalHeader()->setSectionResizeMode(2, QHeaderView::Stretch);
+
     connect(m_pTableView,
             &QTableView::doubleClicked,
             this,
             &ColorPaletteEditor::slotTableViewDoubleClicked);
-    connect(m_pTableView,
-            &QTableView::customContextMenuRequested,
-            this,
-            &ColorPaletteEditor::slotTableViewContextMenuRequested);
-    connect(m_pPaletteNameComboBox,
-            &QComboBox::editTextChanged,
+    connect(m_pSaveAsEdit,
+            &QLineEdit::textChanged,
             this,
             &ColorPaletteEditor::slotPaletteNameChanged);
     connect(m_pResetButton,
             &QPushButton::clicked,
             this,
             &ColorPaletteEditor::slotResetButtonClicked);
-    connect(m_pDiscardButton,
+    connect(m_pCloseButton,
             &QPushButton::clicked,
             this,
-            &ColorPaletteEditor::slotDiscardButtonClicked);
+            &ColorPaletteEditor::slotCloseButtonClicked);
     connect(m_pSaveButton,
             &QPushButton::clicked,
             this,
             &ColorPaletteEditor::slotSaveButtonClicked);
+    connect(m_pRemoveButton,
+            &QPushButton::clicked,
+            this,
+            &ColorPaletteEditor::slotRemoveButtonClicked);
 }
 
-void ColorPaletteEditor::initialize(UserSettingsPointer pConfig) {
+void ColorPaletteEditor::initialize(
+        UserSettingsPointer pConfig,
+        const QString& paletteName) {
     DEBUG_ASSERT(!m_pConfig);
     m_pConfig = pConfig;
-    reset();
-}
+    m_resetPalette = paletteName;
+    QString saveName = paletteName;
 
-void ColorPaletteEditor::reset() {
-    m_pPaletteNameComboBox->clear();
     for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
-        m_pPaletteNameComboBox->addItem(palette.getName());
-    }
-    m_pPaletteNameComboBox->insertSeparator(mixxx::PredefinedColorPalettes::kPalettes.size());
-    ColorPaletteSettings colorPaletteSettings(m_pConfig);
-    for (const QString& paletteName : colorPaletteSettings.getColorPaletteNames()) {
-        m_pPaletteNameComboBox->addItem(paletteName);
+        if (paletteName == palette.getName()) {
+            saveName = paletteName + QStringLiteral(" (") + tr("Edited") + QChar(')');
+            ColorPaletteSettings colorPaletteSettings(m_pConfig);
+            if (colorPaletteSettings.getColorPaletteNames().contains(saveName)) {
+                m_resetPalette = saveName;
+            }
+            break;
+        }
     }
+
+    m_pSaveAsEdit->setText(saveName);
+
+    slotResetButtonClicked();
 }
 
 void ColorPaletteEditor::slotUpdateButtons() {
     bool bDirty = m_pModel->isDirty();
     bool bEmpty = m_pModel->isEmpty();
+    m_pSaveButton->setEnabled(
+            !m_pSaveAsEdit->text().trimmed().isEmpty() &&
+            (!m_bPaletteExists || (!m_bPaletteIsReadOnly && bDirty && !bEmpty)));
+    m_pRemoveButton->setEnabled(
+            m_bPaletteExists &&
+            !m_bPaletteIsReadOnly);
     m_pResetButton->setEnabled(bDirty);
-    m_pSaveButton->setEnabled(!m_bPaletteExists || (!m_bPaletteIsReadOnly && bDirty && !bEmpty));
-    m_pDiscardButton->setEnabled(m_bPaletteExists && !m_bPaletteIsReadOnly);
 }
 
 void ColorPaletteEditor::slotTableViewDoubleClicked(const QModelIndex& index) {
     if (index.isValid() && index.column() == 0) {
-        QColor color = QColorDialog::getColor();
-        if (color.isValid()) {
-            m_pModel->setColor(index.row(), color);
+        QStandardItem* pColorItem = m_pModel->item(index.row(), 0);
+        QColor oldColor = QColor(pColorItem->text());
+        QColor newColor = QColorDialog::getColor(oldColor);
+        if (newColor.isValid() && oldColor != newColor) {
+            m_pModel->setColor(index.row(), newColor);
         }
     }
 }
 
-void ColorPaletteEditor::slotTableViewContextMenuRequested(const QPoint& pos) {
-    QMenu menu(this);
-
-    QAction* pAddAction = menu.addAction("Add");
-    QAction* pRemoveAction = menu.addAction("Remove");
-    QAction* pAction = menu.exec(m_pTableView->viewport()->mapToGlobal(pos));
-    if (pAction == pAddAction) {
-        m_pModel->appendRow(kDefaultPaletteColor);
-    } else if (pAction == pRemoveAction) {
-        QModelIndexList selection = m_pTableView->selectionModel()->selectedRows();
+void ColorPaletteEditor::slotAddColor() {
+    m_pModel->appendRow(kDefaultPaletteColor);
+}
 
-        if (selection.count() > 0) {
-            QModelIndex index = selection.at(0);
+void ColorPaletteEditor::slotRemoveColor() {
+    QModelIndexList selection = m_pTableView->selectionModel()->selectedRows();
 
-            //row selected
-            int row = index.row();
-            m_pModel->removeRow(row);
-        }
+    if (selection.count() > 0) {
+        QModelIndex index = selection.at(0);
+        //row selected
+        int row = index.row();
+        m_pModel->removeRow(row);
     }
 }
 
@@ -172,49 +202,44 @@ void ColorPaletteEditor::slotPaletteNameChanged(const QString& text) {
                 }
             }
             if (!bPaletteFound) {
-                m_pModel->setColorPalette(colorPaletteSettings.getColorPalette(text, mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette));
+                m_pModel->setColorPalette(colorPaletteSettings.getColorPalette(
+                        text, mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette));
             }
         }
     }
 
     m_bPaletteExists = bPaletteExists;
     m_bPaletteIsReadOnly = bPaletteIsReadOnly;
+
     slotUpdateButtons();
 }
 
-void ColorPaletteEditor::slotDiscardButtonClicked() {
-    QString paletteName = m_pPaletteNameComboBox->currentText();
+void ColorPaletteEditor::slotCloseButtonClicked() {
+    reject();
+}
+
+void ColorPaletteEditor::slotRemoveButtonClicked() {
+    QString paletteName = m_pSaveAsEdit->text().trimmed();
     ColorPaletteSettings colorPaletteSettings(m_pConfig);
     colorPaletteSettings.removePalette(paletteName);
-    reset();
     emit paletteRemoved(paletteName);
+    accept();
 }
 
 void ColorPaletteEditor::slotSaveButtonClicked() {
-    QString paletteName = m_pPaletteNameComboBox->currentText();
+    QString paletteName = m_pSaveAsEdit->text().trimmed();
     ColorPaletteSettings colorPaletteSettings(m_pConfig);
     colorPaletteSettings.setColorPalette(paletteName, m_pModel->getColorPalette(paletteName));
     m_pModel->setDirty(false);
-    reset();
-    m_pPaletteNameComboBox->setCurrentText(paletteName);
     emit paletteChanged(paletteName);
+    accept();
 }
 
 void ColorPaletteEditor::slotResetButtonClicked() {
-    QString paletteName = m_pPaletteNameComboBox->currentText();
     ColorPaletteSettings colorPaletteSettings(m_pConfig);
-    bool bPaletteExists = colorPaletteSettings.getColorPaletteNames().contains(paletteName);
-    if (!bPaletteExists) {
-        for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
-            if (paletteName == palette.getName()) {
-                bPaletteExists = true;
-                break;
-            }
-        }
-    }
-    m_pModel->setDirty(false);
-    reset();
-    if (bPaletteExists) {
-        m_pPaletteNameComboBox->setCurrentText(paletteName);
-    }
+    ColorPalette palette = colorPaletteSettings.getColorPalette(
+            m_resetPalette,
+            mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette);
+    m_pModel->setColorPalette(palette);
+    slotUpdateButtons();
 }

src/preferences/colorpaletteeditor.h

@@ -1,45 +1,50 @@
 #pragma once
 
 #include <QComboBox>
+#include <QDialog>
+#include <QLineEdit>
 #include <QPushButton>
 #include <QTableView>
-#include <QWidget>
 
 #include "preferences/colorpaletteeditormodel.h"
 #include "preferences/usersettings.h"
 #include "util/parented_ptr.h"
 
-// Widget for viewing, adding, editing and removing color palettes that can be
-// used for track/hotcue colors.
-class ColorPaletteEditor : public QWidget {
+class ColorPaletteEditor : public QDialog {
     Q_OBJECT
   public:
     ColorPaletteEditor(QWidget* parent = nullptr);
-    void initialize(UserSettingsPointer pConfig);
-    void reset();
+    void initialize(UserSettingsPointer pConfig, const QString& paletteName);
 
   signals:
-    void paletteChanged(QString name);
-    void paletteRemoved(QString name);
+    void paletteChanged(const QString& name);
+    void paletteRemoved(const QString& name);
+    void closeButtonClicked();
 
   private slots:
     void slotUpdateButtons();
     void slotTableViewDoubleClicked(const QModelIndex& index);
-    void slotTableViewContextMenuRequested(const QPoint& pos);
+    void slotAddColor();
+    void slotRemoveColor();
     void slotPaletteNameChanged(const QString& text);
-    void slotDiscardButtonClicked();
+    void slotCloseButtonClicked();
     void slotSaveButtonClicked();
     void slotResetButtonClicked();
+    void slotRemoveButtonClicked();
 
   private:
     bool m_bPaletteExists;
     bool m_bPaletteIsReadOnly;
 
     UserSettingsPointer m_pConfig;
-    parented_ptr<QComboBox> m_pPaletteNameComboBox;
+    parented_ptr<QLineEdit> m_pSaveAsEdit;
     parented_ptr<QTableView> m_pTableView;
     parented_ptr<ColorPaletteEditorModel> m_pModel;
+    QPushButton* m_pAddColorButton;
+    QPushButton* m_pRemoveColorButton;
     QPushButton* m_pSaveButton;
-    QPushButton* m_pDiscardButton;
+    QPushButton* m_pCloseButton;
+    QPushButton* m_pRemoveButton;
     QPushButton* m_pResetButton;
+    QString m_resetPalette;
 };

src/preferences/colorpaletteeditormodel.cpp

@@ -102,9 +102,9 @@ void ColorPaletteEditorModel::setColorPalette(const ColorPalette& palette) {
 
     // Make a map of hotcue indices
     QMap<int, int> hotcueColorIndicesMap;
-    QList<unsigned int> hotcueColorIndices = palette.getHotcueIndices();
-    for (int i = 0; i < hotcueColorIndices.size(); i++) {
-        int colorIndex = hotcueColorIndices.at(i);
+    QList<int> colorIndicesByHotcue = palette.getIndicesByHotcue();
+    for (int i = 0; i < colorIndicesByHotcue.size(); i++) {
+        int colorIndex = colorIndicesByHotcue.at(i);
         hotcueColorIndicesMap.insert(colorIndex, i);
     }
 
@@ -119,7 +119,7 @@ void ColorPaletteEditorModel::setColorPalette(const ColorPalette& palette) {
 
 ColorPalette ColorPaletteEditorModel::getColorPalette(const QString& name) const {
     QList<mixxx::RgbColor> colors;
-    QMap<int, unsigned int> hotcueColorIndices;
+    QMap<int, int> hotcueColorIndices;
     for (int i = 0; i < rowCount(); i++) {
         QStandardItem* pColorItem = item(i, 0);
         QStandardItem* pHotcueIndexItem = item(i, 1);
@@ -134,5 +134,7 @@ ColorPalette ColorPaletteEditorModel::getColorPalette(const QString& name) const
             }
         }
     }
+    // If we have a non consequitive list of hotcue indexes, indexes are shifted down
+    // due to the sorting nature of QMap. This is intended, this way we have a color for every hotcue.
     return ColorPalette(name, colors, hotcueColorIndices.values());
 }