Clear up Highlight sound settings (#4194)

Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>
Closes #1540

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unversioned
 
+- Minor: Cleared up highlight sound settings (#4194)
+- Bugfix: Fixed highlight sounds not reloading on change properly. (#4194)
 - Bugfix: Fixed CTRL + C not working in reply thread popups. (#4209)
 - Bugfix: Fixed message input showing as red after removing a message that was more than 500 characters. (#4204)
 - Bugfix: Fixed unnecessary saving of windows layout. (#4201)

src/controllers/highlights/HighlightController.cpp

@@ -50,8 +50,7 @@ void rebuildSubscriptionHighlights(Settings &settings,
     {
         auto highlightSound = settings.enableSubHighlightSound.getValue();
         auto highlightAlert = settings.enableSubHighlightTaskbar.getValue();
-        auto highlightSoundUrlValue =
-            settings.whisperHighlightSoundUrl.getValue();
+        auto highlightSoundUrlValue = settings.subHighlightSoundUrl.getValue();
         boost::optional<QUrl> highlightSoundUrl;
         if (!highlightSoundUrlValue.isEmpty())
         {
@@ -284,15 +283,22 @@ namespace chatterino {
 
 void HighlightController::initialize(Settings &settings, Paths & /*paths*/)
 {
+    this->rebuildListener_.addSetting(settings.enableSelfHighlight);
+    this->rebuildListener_.addSetting(settings.enableSelfHighlightSound);
+    this->rebuildListener_.addSetting(settings.enableSelfHighlightTaskbar);
+    this->rebuildListener_.addSetting(settings.selfHighlightSoundUrl);
+    this->rebuildListener_.addSetting(settings.showSelfHighlightInMentions);
+
     this->rebuildListener_.addSetting(settings.enableWhisperHighlight);
     this->rebuildListener_.addSetting(settings.enableWhisperHighlightSound);
     this->rebuildListener_.addSetting(settings.enableWhisperHighlightTaskbar);
     this->rebuildListener_.addSetting(settings.whisperHighlightSoundUrl);
-    this->rebuildListener_.addSetting(settings.whisperHighlightColor);
-    this->rebuildListener_.addSetting(settings.enableSelfHighlight);
+
     this->rebuildListener_.addSetting(settings.enableSubHighlight);
     this->rebuildListener_.addSetting(settings.enableSubHighlightSound);
     this->rebuildListener_.addSetting(settings.enableSubHighlightTaskbar);
+    this->rebuildListener_.addSetting(settings.subHighlightSoundUrl);
+
     this->rebuildListener_.addSetting(settings.enableThreadHighlight);
     this->rebuildListener_.addSetting(settings.enableThreadHighlightSound);
     this->rebuildListener_.addSetting(settings.enableThreadHighlightTaskbar);

src/controllers/highlights/HighlightModel.cpp

@@ -140,10 +140,7 @@ void HighlightModel::afterInit()
     redeemedRow[Column::PlaySound]->setFlags({});
     redeemedRow[Column::UseRegex]->setFlags({});
     redeemedRow[Column::CaseSensitive]->setFlags({});
-
-    QUrl RedeemedSound =
-        QUrl(getSettings()->redeemedHighlightSoundUrl.getValue());
-    setFilePathItem(redeemedRow[Column::SoundPath], RedeemedSound, false);
+    redeemedRow[Column::SoundPath]->setFlags(Qt::NoItemFlags);
 
     auto RedeemedColor =
         ColorProvider::instance().color(ColorType::RedeemedHighlight);
@@ -169,11 +166,7 @@ void HighlightModel::afterInit()
     firstMessageRow[Column::PlaySound]->setFlags({});
     firstMessageRow[Column::UseRegex]->setFlags({});
     firstMessageRow[Column::CaseSensitive]->setFlags({});
-
-    QUrl FirstMessageSound =
-        QUrl(getSettings()->firstMessageHighlightSoundUrl.getValue());
-    setFilePathItem(firstMessageRow[Column::SoundPath], FirstMessageSound,
-                    false);
+    firstMessageRow[Column::SoundPath]->setFlags(Qt::NoItemFlags);
 
     auto FirstMessageColor =
         ColorProvider::instance().color(ColorType::FirstMessageHighlight);
@@ -200,11 +193,7 @@ void HighlightModel::afterInit()
     elevatedMessageRow[Column::PlaySound]->setFlags({});
     elevatedMessageRow[Column::UseRegex]->setFlags({});
     elevatedMessageRow[Column::CaseSensitive]->setFlags({});
-
-    QUrl elevatedMessageSound =
-        QUrl(getSettings()->elevatedMessageHighlightSoundUrl.getValue());
-    setFilePathItem(elevatedMessageRow[Column::SoundPath], elevatedMessageSound,
-                    false);
+    elevatedMessageRow[Column::SoundPath]->setFlags(Qt::NoItemFlags);
 
     auto elevatedMessageColor =
         ColorProvider::instance().color(ColorType::ElevatedMessageHighlight);
@@ -416,21 +405,6 @@ void HighlightModel::customRowSetData(const std::vector<QStandardItem *> &row,
                     getSettings()->subHighlightSoundUrl.setValue(
                         value.toString());
                 }
-                else if (rowIndex == HighlightRowIndexes::RedeemedRow)
-                {
-                    getSettings()->redeemedHighlightSoundUrl.setValue(
-                        value.toString());
-                }
-                else if (rowIndex == HighlightRowIndexes::FirstMessageRow)
-                {
-                    getSettings()->firstMessageHighlightSoundUrl.setValue(
-                        value.toString());
-                }
-                if (rowIndex == HighlightRowIndexes::ElevatedMessageRow)
-                {
-                    getSettings()->elevatedMessageHighlightSoundUrl.setValue(
-                        value.toString());
-                }
                 else if (rowIndex == HighlightRowIndexes::ThreadMessageRow)
                 {
                     getSettings()->threadHighlightSoundUrl.setValue(

src/controllers/highlights/HighlightModel.hpp

@@ -17,9 +17,9 @@ class HighlightModel : public SignalVectorModel<HighlightPhrase>
         Pattern = 0,
         ShowInMentions = 1,
         FlashTaskbar = 2,
-        PlaySound = 3,
-        UseRegex = 4,
-        CaseSensitive = 5,
+        UseRegex = 3,
+        CaseSensitive = 4,
+        PlaySound = 5,
         SoundPath = 6,
         Color = 7,
         COUNT  // keep this as last member of enum

src/messages/SharedMessageBuilder.cpp

@@ -19,14 +19,17 @@ namespace chatterino {
 
 namespace {
 
+    /**
+     * Gets the default sound url if the user set one,
+     * or the chatterino default ping sound if no url is set.
+     */
     QUrl getFallbackHighlightSound()
     {
         QString path = getSettings()->pathHighlightSound;
-        bool fileExists = QFileInfo::exists(path) && QFileInfo(path).isFile();
+        bool fileExists = !path.isEmpty() && QFileInfo::exists(path) &&
+                          QFileInfo(path).isFile();
 
-        // Use fallback sound when checkbox is not checked
-        // or custom file doesn't exist
-        if (getSettings()->customHighlightSound && fileExists)
+        if (fileExists)
         {
             return QUrl::fromLocalFile(path);
         }

src/singletons/Settings.hpp

@@ -271,7 +271,6 @@ class Settings : public ABSettings, public ConcurrentSettings
 
     /// Highlighting
     //    BoolSetting enableHighlights = {"/highlighting/enabled", true};
-    BoolSetting customHighlightSound = {"/highlighting/useCustomSound", false};
 
     BoolSetting enableSelfHighlight = {
         "/highlighting/selfHighlight/nameIsHighlightKeyword", true};
@@ -303,8 +302,8 @@ class Settings : public ABSettings, public ConcurrentSettings
     //        "/highlighting/redeemedHighlight/enableSound", false};
     //    BoolSetting enableRedeemedHighlightTaskbar = {
     //        "/highlighting/redeemedHighlight/enableTaskbarFlashing", false};
-    QStringSetting redeemedHighlightSoundUrl = {
-        "/highlighting/redeemedHighlightSoundUrl", ""};
+    //    QStringSetting redeemedHighlightSoundUrl = {
+    //        "/highlighting/redeemedHighlightSoundUrl", ""};
     QStringSetting redeemedHighlightColor = {
         "/highlighting/redeemedHighlightColor", ""};
 
@@ -314,8 +313,8 @@ class Settings : public ABSettings, public ConcurrentSettings
     //        "/highlighting/firstMessageHighlight/enableSound", false};
     //    BoolSetting enableFirstMessageHighlightTaskbar = {
     //        "/highlighting/firstMessageHighlight/enableTaskbarFlashing", false};
-    QStringSetting firstMessageHighlightSoundUrl = {
-        "/highlighting/firstMessageHighlightSoundUrl", ""};
+    //    QStringSetting firstMessageHighlightSoundUrl = {
+    //        "/highlighting/firstMessageHighlightSoundUrl", ""};
     QStringSetting firstMessageHighlightColor = {
         "/highlighting/firstMessageHighlightColor", ""};
 
@@ -325,8 +324,8 @@ class Settings : public ABSettings, public ConcurrentSettings
     //        "/highlighting/elevatedMessageHighlight/enableSound", false};
     //    BoolSetting enableElevatedMessageHighlightTaskbar = {
     //        "/highlighting/elevatedMessageHighlight/enableTaskbarFlashing", false};
-    QStringSetting elevatedMessageHighlightSoundUrl = {
-        "/highlighting/elevatedMessageHighlight/soundUrl", ""};
+    //    QStringSetting elevatedMessageHighlightSoundUrl = {
+    //        "/highlighting/elevatedMessageHighlight/soundUrl", ""};
     QStringSetting elevatedMessageHighlightColor = {
         "/highlighting/elevatedMessageHighlight/color", ""};
 

src/widgets/settingspages/HighlightingPage.cpp

@@ -7,24 +7,19 @@
 #include "controllers/highlights/UserHighlightModel.hpp"
 #include "singletons/Settings.hpp"
 #include "singletons/Theme.hpp"
+#include "util/Helpers.hpp"
 #include "util/LayoutCreator.hpp"
 #include "util/StandardItemHelper.hpp"
 #include "widgets/dialogs/BadgePickerDialog.hpp"
 #include "widgets/dialogs/ColorPickerDialog.hpp"
 
 #include <QFileDialog>
 #include <QHeaderView>
-#include <QListWidget>
 #include <QPushButton>
 #include <QStandardItemModel>
 #include <QTableView>
 #include <QTabWidget>
-#include <QTextEdit>
 
-#define ENABLE_HIGHLIGHTS "Enable Highlighting"
-#define HIGHLIGHT_MSG "Highlight messages containing your name"
-#define PLAY_SOUND "Play sound when your name is mentioned"
-#define FLASH_TASKBAR "Flash taskbar when your name is mentioned"
 #define ALWAYS_PLAY "Play highlight sound even when Chatterino is focused"
 
 namespace chatterino {
@@ -76,8 +71,8 @@ HighlightingPage::HighlightingPage()
                         .getElement();
                 view->addRegexHelpLink();
                 view->setTitles({"Pattern", "Show in\nMentions",
-                                 "Flash\ntaskbar", "Play\nsound",
-                                 "Enable\nregex", "Case-\nsensitive",
+                                 "Flash\ntaskbar", "Enable\nregex",
+                                 "Case-\nsensitive", "Play\nsound",
                                  "Custom\nsound", "Color"});
                 view->getTableView()->horizontalHeader()->setSectionResizeMode(
                     QHeaderView::Fixed);
@@ -88,7 +83,7 @@ HighlightingPage::HighlightingPage()
                 // dpiChanged
                 QTimer::singleShot(1, [view] {
                     view->getTableView()->resizeColumnsToContents();
-                    view->getTableView()->setColumnWidth(0, 200);
+                    view->getTableView()->setColumnWidth(0, 400);
                 });
 
                 view->addButtonPressed.connect([] {
@@ -127,8 +122,8 @@ HighlightingPage::HighlightingPage()
                 // Case-sensitivity doesn't make sense for user names so it is
                 // set to "false" by default & the column is hidden
                 view->setTitles({"Username", "Show in\nMentions",
-                                 "Flash\ntaskbar", "Play\nsound",
-                                 "Enable\nregex", "Case-\nsensitive",
+                                 "Flash\ntaskbar", "Enable\nregex",
+                                 "Case-\nsensitive", "Play\nsound",
                                  "Custom\nsound", "Color"});
                 view->getTableView()->horizontalHeader()->setSectionResizeMode(
                     QHeaderView::Fixed);
@@ -247,33 +242,57 @@ HighlightingPage::HighlightingPage()
         // MISC
         auto customSound = layout.emplace<QHBoxLayout>().withoutMargin();
         {
-            auto fallbackSound = customSound.append(this->createCheckBox(
-                "Fallback sound (played when no other sound is set)",
-                getSettings()->customHighlightSound));
-
-            auto getSelectFileText = [] {
-                const QString value = getSettings()->pathHighlightSound;
-                return value.isEmpty() ? "Select custom fallback sound"
-                                       : QUrl::fromLocalFile(value).fileName();
-            };
-
-            auto selectFile =
-                customSound.emplace<QPushButton>(getSelectFileText());
-
-            QObject::connect(
-                selectFile.getElement(), &QPushButton::clicked, this,
-                [=]() mutable {
-                    auto fileName = QFileDialog::getOpenFileName(
-                        this, tr("Open Sound"), "",
-                        tr("Audio Files (*.mp3 *.wav)"));
-
-                    getSettings()->pathHighlightSound = fileName;
-                    selectFile.getElement()->setText(getSelectFileText());
-
-                    // Set check box according to updated value
-                    fallbackSound->setCheckState(
-                        fileName.isEmpty() ? Qt::Unchecked : Qt::Checked);
-                });
+            auto label = customSound.append(this->createLabel<QString>(
+                [](const auto &value) {
+                    if (value.isEmpty())
+                    {
+                        return QString("Default sound: Chatterino Ping");
+                    }
+
+                    auto url = QUrl::fromLocalFile(value);
+                    return QString("Default sound: <a href=\"%1\"><span "
+                                   "style=\"color: white\">%2</span></a>")
+                        .arg(url.toString(QUrl::FullyEncoded),
+                             shortenString(url.fileName(), 50));
+                },
+                getSettings()->pathHighlightSound));
+            label->setToolTip(
+                "This sound will play for all highlight phrases that have "
+                "sound enabled and don't have a custom sound set.");
+            label->setTextFormat(Qt::RichText);
+            label->setTextInteractionFlags(Qt::TextBrowserInteraction |
+                                           Qt::LinksAccessibleByKeyboard);
+            label->setOpenExternalLinks(true);
+            customSound->setStretchFactor(label.getElement(), 1);
+
+            auto clearSound = customSound.emplace<QPushButton>("Clear");
+            auto selectFile = customSound.emplace<QPushButton>("Change...");
+
+            QObject::connect(selectFile.getElement(), &QPushButton::clicked,
+                             this, [=]() mutable {
+                                 auto fileName = QFileDialog::getOpenFileName(
+                                     this, tr("Open Sound"), "",
+                                     tr("Audio Files (*.mp3 *.wav)"));
+
+                                 getSettings()->pathHighlightSound = fileName;
+                             });
+            QObject::connect(clearSound.getElement(), &QPushButton::clicked,
+                             this, [=]() mutable {
+                                 getSettings()->pathHighlightSound = QString();
+                             });
+
+            getSettings()->pathHighlightSound.connect(
+                [clearSound = clearSound.getElement()](const auto &value) {
+                    if (value.isEmpty())
+                    {
+                        clearSound->hide();
+                    }
+                    else
+                    {
+                        clearSound->show();
+                    }
+                },
+                this->managedConnections_);
         }
 
         layout.append(createCheckBox(ALWAYS_PLAY,
@@ -294,13 +313,6 @@ void HighlightingPage::openSoundDialog(const QModelIndex &clicked,
                                                tr("Audio Files (*.mp3 *.wav)"));
     view->getModel()->setData(clicked, fileUrl, Qt::UserRole);
     view->getModel()->setData(clicked, fileUrl.fileName(), Qt::DisplayRole);
-
-    // Enable custom sound check box if user set a sound
-    if (!fileUrl.isEmpty())
-    {
-        QModelIndex checkBox = clicked.siblingAtColumn(soundColumn);
-        view->getModel()->setData(checkBox, Qt::Checked, Qt::CheckStateRole);
-    }
 }
 
 void HighlightingPage::openColorDialog(const QModelIndex &clicked,
@@ -356,7 +368,8 @@ void HighlightingPage::tableCellClicked(const QModelIndex &clicked,
                 (tab == HighlightTab::Messages &&
                  clicked.row() ==
                      HighlightModel::HighlightRowIndexes::WhisperRow);
-            if (clicked.column() == Column::SoundPath)
+            if (clicked.column() == Column::SoundPath &&
+                clicked.flags().testFlag(Qt::ItemIsEnabled))
             {
                 this->openSoundDialog(clicked, view, Column::SoundPath);
             }

src/widgets/settingspages/SettingsPage.hpp

@@ -66,6 +66,20 @@ class SettingsPage : public QFrame
     QLineEdit *createLineEdit(pajlada::Settings::Setting<QString> &setting);
     QSpinBox *createSpinBox(pajlada::Settings::Setting<int> &setting,
                             int min = 0, int max = 2500);
+    template <typename T>
+    SLabel *createLabel(const std::function<QString(const T &)> &makeText,
+                        pajlada::Settings::Setting<T> &setting)
+    {
+        auto *label = new SLabel();
+
+        setting.connect(
+            [label, makeText](const T &value, auto) {
+                label->setText(makeText(value));
+            },
+            this->managedConnections_);
+
+        return label;
+    }
 
     virtual void onShow()
     {