From fed250a9339fc33655fb93cd079b96a02b473f9b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 24 Nov 2023 15:21:46 +0100 Subject: [PATCH 1/4] MIDI mapping editor: allow selecting multiple Options in the table view --- .../delegates/midioptionsdelegate.cpp | 102 ++++++++++++++---- .../delegates/midioptionsdelegate.h | 3 + 2 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/controllers/delegates/midioptionsdelegate.cpp b/src/controllers/delegates/midioptionsdelegate.cpp index deab2ea10c0..99ed83ef3b7 100644 --- a/src/controllers/delegates/midioptionsdelegate.cpp +++ b/src/controllers/delegates/midioptionsdelegate.cpp @@ -1,10 +1,13 @@ #include "controllers/delegates/midioptionsdelegate.h" +#include #include +#include #include "controllers/midi/midimessage.h" #include "controllers/midi/midiutils.h" #include "moc_midioptionsdelegate.cpp" +#include "util/parented_ptr.h" namespace { @@ -35,18 +38,51 @@ MidiOptionsDelegate::MidiOptionsDelegate(QObject* pParent) MidiOptionsDelegate::~MidiOptionsDelegate() { } - QWidget* MidiOptionsDelegate::createEditor(QWidget* parent, const QStyleOptionViewItem& option, const QModelIndex& index) const { Q_UNUSED(option); Q_UNUSED(index); - QComboBox* pComboBox = new QComboBox(parent); - for (const MidiOption choice : kMidiOptions) { - pComboBox->addItem(MidiUtils::midiOptionToTranslatedString(choice), - static_cast(choice)); + // Create, populate and connect the box. + QComboBox* pComboBox = make_parented(parent); + auto* pModel = static_cast(pComboBox->model()); + DEBUG_ASSERT(pModel); + for (const MidiOption opt : kMidiOptions) { + // Don't add 'Normal' to the list because it's useless: it's exclusive, + // meaning it's the implicit result of unchecking all other options, so + // showing it checked is pointless (and it's not updated if others are + // checked). Also, clicking it does not uncheck all other options. + if (opt == MidiOption::None) { + continue; + } + QStandardItem* pItem = + new QStandardItem(MidiUtils::midiOptionToTranslatedString(opt)); + pItem->setData(static_cast(opt)); + pItem->setCheckable(true); + pModel->appendRow(pItem); } + // Unsetting the index clears the display text which is visible when closing + // the list view by clicking anywhere else. Default text is that of first + // added item. It can be set to any combination of existing item texts, e.g. + // all checked items, but it's not updated before the selection is committed + // so let's simply clear the text to avoid confusion. + pComboBox->setCurrentIndex(-1); + + // Default horizontal size policy is Preferred which results in center elide + // as soon as items are checkable, regardless the elide mode. + pComboBox->view()->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Minimum); + pComboBox->view()->setTextElideMode(Qt::ElideNone); + + // * clicking an option or pressing Enter on a selected option toggles it, + // closes the list view and commits the updated data + // * pressing Space on a selected option toggles it, list remains open + // * clicking outside the listview closes it, and another click causing the + // combobox to lose focus closes that and commits pending changes + connect(pComboBox, + QOverload::of(&QComboBox::activated), + this, + &MidiOptionsDelegate::commitAndCloseEditor); return pComboBox; } @@ -66,28 +102,58 @@ QString MidiOptionsDelegate::displayText(const QVariant& value, void MidiOptionsDelegate::setEditorData(QWidget* editor, const QModelIndex& index) const { - MidiOptions options = index.data(Qt::EditRole).value(); - - QComboBox* pComboBox = qobject_cast(editor); - if (pComboBox == nullptr) { + auto* pComboBox = qobject_cast(editor); + if (!pComboBox) { return; } - for (int i = 0; i < pComboBox->count(); ++i) { - if (MidiOptions(pComboBox->itemData(i).toInt()) & options) { - pComboBox->setCurrentIndex(i); - return; - } + + // Update checked states + const MidiOptions options = index.data(Qt::EditRole).value(); + const auto* pModel = static_cast(pComboBox->model()); + DEBUG_ASSERT(pModel); + for (int row = 0; row < pModel->rowCount(); row++) { + auto* pItem = pModel->item(row, 0); + auto opt = static_cast(pItem->data().toInt()); + pItem->setCheckState(options.testFlag(opt) ? Qt::Checked : Qt::Unchecked); } + + // Show popup immediately, as with the other editors, no 'edit' click + // required to open the list view. + pComboBox->showPopup(); } void MidiOptionsDelegate::setModelData(QWidget* editor, QAbstractItemModel* model, const QModelIndex& index) const { - MidiOptions options; - QComboBox* pComboBox = qobject_cast(editor); - if (pComboBox == nullptr) { + // Collect checked options and write them back to the model + auto* pComboBox = qobject_cast(editor); + if (!pComboBox) { return; } - options = MidiOptions(pComboBox->itemData(pComboBox->currentIndex()).toInt()); + + const auto* pModel = static_cast(pComboBox->model()); + DEBUG_ASSERT(pModel); + MidiOptions options; + for (int i = 0; i < pModel->rowCount(); i++) { + auto* pItem = pModel->item(i); + // Only check for Qt::Checked or else, ignore Qt::PartiallyChecked + options.setFlag(static_cast(pItem->data().toUInt()), + pItem->checkState() == Qt::Checked); + } model->setData(index, QVariant::fromValue(options), Qt::EditRole); } + +void MidiOptionsDelegate::commitAndCloseEditor(int index) { + QComboBox* pComboBox = qobject_cast(sender()); + if (!pComboBox) { + return; + } + const auto* pModel = static_cast(pComboBox->model()); + DEBUG_ASSERT(pModel); + auto* pItem = pModel->item(index); + DEBUG_ASSERT(pItem); + pItem->setCheckState(pItem->checkState() == Qt::Checked ? Qt::Unchecked : Qt::Checked); + + emit commitData(pComboBox); + emit closeEditor(pComboBox); +} diff --git a/src/controllers/delegates/midioptionsdelegate.h b/src/controllers/delegates/midioptionsdelegate.h index f690df38f95..8d175eb0b2e 100644 --- a/src/controllers/delegates/midioptionsdelegate.h +++ b/src/controllers/delegates/midioptionsdelegate.h @@ -17,4 +17,7 @@ class MidiOptionsDelegate : public QStyledItemDelegate { void setModelData(QWidget* editor, QAbstractItemModel* model, const QModelIndex& index) const; + + private slots: + void commitAndCloseEditor(int index); }; From a3c4cc2cf15902990c6ef397ae20192904190c86 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 9 Dec 2023 00:07:11 +0100 Subject: [PATCH 2/4] MIDI mapping table: don't show 'Normal' option --- src/controllers/delegates/midioptionsdelegate.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/controllers/delegates/midioptionsdelegate.cpp b/src/controllers/delegates/midioptionsdelegate.cpp index 99ed83ef3b7..b5218200de8 100644 --- a/src/controllers/delegates/midioptionsdelegate.cpp +++ b/src/controllers/delegates/midioptionsdelegate.cpp @@ -12,7 +12,13 @@ namespace { const QList kMidiOptions = { - MidiOption::None, + // Don't add 'Normal' to the list because it's useless: it's exclusive, + // meaning it's the implicit result of unchecking all other options, but + // clicking it does not uncheck all other options. Also, showing it + // checked is pointless (and it's not updated if others are checked). + // Furthermore, the mapping list is cleaner without it, mappings that + // have options set are much easier to spot. + // MidiOption::None, MidiOption::Invert, MidiOption::Rot64, MidiOption::Rot64Invert, @@ -49,13 +55,6 @@ QWidget* MidiOptionsDelegate::createEditor(QWidget* parent, auto* pModel = static_cast(pComboBox->model()); DEBUG_ASSERT(pModel); for (const MidiOption opt : kMidiOptions) { - // Don't add 'Normal' to the list because it's useless: it's exclusive, - // meaning it's the implicit result of unchecking all other options, so - // showing it checked is pointless (and it's not updated if others are - // checked). Also, clicking it does not uncheck all other options. - if (opt == MidiOption::None) { - continue; - } QStandardItem* pItem = new QStandardItem(MidiUtils::midiOptionToTranslatedString(opt)); pItem->setData(static_cast(opt)); From c657413c87115d13fb3a4d086b7df055293ca643 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 9 Dec 2023 00:09:49 +0100 Subject: [PATCH 3/4] MIDI option delegate: add 'Unset all' action to options list --- .../delegates/midioptionsdelegate.cpp | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/controllers/delegates/midioptionsdelegate.cpp b/src/controllers/delegates/midioptionsdelegate.cpp index b5218200de8..e9a7a29081d 100644 --- a/src/controllers/delegates/midioptionsdelegate.cpp +++ b/src/controllers/delegates/midioptionsdelegate.cpp @@ -61,6 +61,11 @@ QWidget* MidiOptionsDelegate::createEditor(QWidget* parent, pItem->setCheckable(true); pModel->appendRow(pItem); } + // Add a special item to uncheck all. See commitAndCloseEditor() + QStandardItem* pItem = new QStandardItem(tr("Unset all")); + pItem->setCheckable(false); // doesn't hurt to set this explicitly + pModel->appendRow(pItem); + // Unsetting the index clears the display text which is visible when closing // the list view by clicking anywhere else. Default text is that of first // added item. It can be set to any combination of existing item texts, e.g. @@ -112,6 +117,9 @@ void MidiOptionsDelegate::setEditorData(QWidget* editor, DEBUG_ASSERT(pModel); for (int row = 0; row < pModel->rowCount(); row++) { auto* pItem = pModel->item(row, 0); + if (!pItem->isCheckable()) { + continue; + } auto opt = static_cast(pItem->data().toInt()); pItem->setCheckState(options.testFlag(opt) ? Qt::Checked : Qt::Unchecked); } @@ -151,7 +159,18 @@ void MidiOptionsDelegate::commitAndCloseEditor(int index) { DEBUG_ASSERT(pModel); auto* pItem = pModel->item(index); DEBUG_ASSERT(pItem); - pItem->setCheckState(pItem->checkState() == Qt::Checked ? Qt::Unchecked : Qt::Checked); + if (pItem->isCheckable()) { + pItem->setCheckState(pItem->checkState() == Qt::Checked ? Qt::Unchecked : Qt::Checked); + } else { + // Clear was selected. Uncheck all other items + for (int row = 0; row < pModel->rowCount() - 1; row++) { + if (row == index) { // Actually it's the last item, but this is safer. + continue; + } + pItem = pModel->item(row, 0); + pItem->setCheckState(Qt::Unchecked); + } + } emit commitData(pComboBox); emit closeEditor(pComboBox); From 8dad925af96227380ed6d0637bbfd24a8d2c32e0 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 22 May 2024 01:15:28 +0200 Subject: [PATCH 4/4] MIDI option delegate: add TODO for concurrent options --- src/controllers/delegates/midioptionsdelegate.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/controllers/delegates/midioptionsdelegate.cpp b/src/controllers/delegates/midioptionsdelegate.cpp index e9a7a29081d..28ca99b0366 100644 --- a/src/controllers/delegates/midioptionsdelegate.cpp +++ b/src/controllers/delegates/midioptionsdelegate.cpp @@ -161,6 +161,10 @@ void MidiOptionsDelegate::commitAndCloseEditor(int index) { DEBUG_ASSERT(pItem); if (pItem->isCheckable()) { pItem->setCheckState(pItem->checkState() == Qt::Checked ? Qt::Unchecked : Qt::Checked); + // TODO Concurrent option scan be selected. Implement a compatibility + // matrix and uncheck all options that are incompatible with the last + // checked option. Store initial check state for/in each item and + // hook up to QStandardItemModel::itemChanged() } else { // Clear was selected. Uncheck all other items for (int row = 0; row < pModel->rowCount() - 1; row++) {