Better Highlights: Fix Unintentional Color Update (#1522)

* HighlightPhrase: Fix wrong documentation

* Use right constructor for new HighlightPhrases

* Fix preset highlights changing unintentionally

Prior to this commit, the callback for reacting to user input on the
highlight table (namely, `HighlightingPage::tableCellClicked`) only
checked for the row number in order to determine whether preset
highlights (self highlights, whispers, and subscriptions) need to be
updated. Hence, changing rows 0 through 2 in the "User Highlights" tab
would also update the preset highlights.

This commit adds a check to determine whether the callback was triggered
by the "Messages" highlight tab, or not.

src/controllers/highlights/HighlightPhrase.cpp

@@ -12,11 +12,6 @@ bool HighlightPhrase::operator==(const HighlightPhrase &other) const
                                               other.soundUrl_, other.color_);
 }
 
-/**
- * @brief Create a new HighlightPhrase.
- *
- * Use this constructor when updating an existing HighlightPhrase's color.
- */
 HighlightPhrase::HighlightPhrase(const QString &pattern, bool hasAlert,
                                  bool hasSound, bool isRegex,
                                  bool isCaseSensitive, const QString &soundUrl,
@@ -36,11 +31,6 @@ HighlightPhrase::HighlightPhrase(const QString &pattern, bool hasAlert,
     this->color_ = std::make_shared<QColor>(color);
 }
 
-/**
- * @brief Create a new HighlightPhrase.
- *
- * Use this constructor when creating a new HighlightPhrase.
- */
 HighlightPhrase::HighlightPhrase(const QString &pattern, bool hasAlert,
                                  bool hasSound, bool isRegex,
                                  bool isCaseSensitive, const QString &soundUrl,

src/controllers/highlights/HighlightPhrase.hpp

@@ -15,10 +15,20 @@ class HighlightPhrase
 public:
     bool operator==(const HighlightPhrase &other) const;
 
+    /**
+     * @brief Create a new HighlightPhrase.
+     *
+     * Use this constructor when creating a new HighlightPhrase.
+     */
     HighlightPhrase(const QString &pattern, bool hasAlert, bool hasSound,
                     bool isRegex, bool isCaseSensitive, const QString &soundUrl,
                     QColor color);
 
+    /**
+     * @brief Create a new HighlightPhrase.
+     *
+     * Use this constructor when updating an existing HighlightPhrase's color.
+     */
     HighlightPhrase(const QString &pattern, bool hasAlert, bool hasSound,
                     bool isRegex, bool isCaseSensitive, const QString &soundUrl,
                     std::shared_ptr<QColor> color);

src/widgets/settingspages/HighlightingPage.cpp

@@ -121,7 +121,7 @@ HighlightingPage::HighlightingPage()
                     getApp()->highlights->highlightedUsers.appendItem(
                         HighlightPhrase{"highlighted user", true, false, false,
                                         false, "",
-                                        ColorProvider::instance().color(
+                                        *ColorProvider::instance().color(
                                             ColorType::SelfHighlight)});
                 });
 
@@ -243,20 +243,31 @@ void HighlightingPage::tableCellClicked(const QModelIndex &clicked,
                 view->getModel()->setData(clicked, selected,
                                           Qt::DecorationRole);
 
-                // For special highlights we need to manually update the color map
+                // Hacky (?) way to figure out what tab the cell was clicked in
-                auto instance = ColorProvider::instance();
+                const bool fromMessages = (dynamic_cast<HighlightModel *>(
-                switch (clicked.row())
+                                               view->getModel()) != nullptr);
+
+                if (fromMessages)
                 {
-                    case 0:
+                    /*
-                        instance.updateColor(ColorType::SelfHighlight,
+                     * For preset highlights in the "Messages" tab, we need to
-                                             selected);
+                     * manually update the color map.
-                        break;
+                     */
-                    case 1:
+                    auto instance = ColorProvider::instance();
-                        instance.updateColor(ColorType::Whisper, selected);
+                    switch (clicked.row())
-                        break;
+                    {
-                    case 2:
+                        case 0:
-                        instance.updateColor(ColorType::Subscription, selected);
+                            instance.updateColor(ColorType::SelfHighlight,
-                        break;
+                                                 selected);
+                            break;
+                        case 1:
+                            instance.updateColor(ColorType::Whisper, selected);
+                            break;
+                        case 2:
+                            instance.updateColor(ColorType::Subscription,
+                                                 selected);
+                            break;
+                    }
                 }
             }
         });