Merge pull request #759 from isanae/unstyled-icons-fix

Fixing the icons on the toolbar and menus

2 files changed, 121 insertions, 18 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index b19e7573..911d0ff1 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -401,8 +401,6 @@ MainWindow::MainWindow(QSettings &initSettings
 
   connect(&m_IntegratedBrowser, SIGNAL(requestDownload(QUrl,QNetworkReply*)), &m_OrganizerCore, SLOT(requestDownload(QUrl,QNetworkReply*)));
 
-  connect(this, SIGNAL(styleChanged(QString)), this, SLOT(updateStyle(QString)));
-
   m_CheckBSATimer.setSingleShot(true);
   connect(&m_CheckBSATimer, SIGNAL(timeout()), this, SLOT(checkBSAList()));
 
@@ -465,17 +463,76 @@ MainWindow::MainWindow(QSettings &initSettings
 
   refreshExecutablesList();
   updatePinnedExecutables();
+  resetActionIcons();
+  updatePluginCount();
+  updateModCount();
+}
 
-  for (QAction *action : ui->toolBar->actions()) {
-    // set the name of the widget to the name of the action to allow styling
-    QWidget *actionWidget = ui->toolBar->widgetForAction(action);
-    actionWidget->setObjectName(action->objectName());
-    actionWidget->style()->unpolish(actionWidget);
-    actionWidget->style()->polish(actionWidget);
+void MainWindow::resetActionIcons()
+{
+  // this is a bit of a hack
+  //
+  // the .qss files have historically set qproperty-icon by id and these ids
+  // correspond to the QActions created in the .ui file
+  //
+  // the problem is that QActions do not support having their icon property
+  // set from a .qss because they're not widgets (they don't inherit from
+  // QWidget), and styling only works on widget
+  //
+  // a QAction _does_ have an associated icon, it just can't be set from a .qss
+  // file
+  //
+  // so here, a dummy QToolButton widget is created for each QAction and is
+  // given the same name as the action, which makes it pick up the icon
+  // specified in the .qss file
+  //
+  // that icon is then given to the widget used by the QAction (if it's some
+  // sort of button, which typically happens on the toolbar) _and_ to the
+  // QAction itself, which is used in the menu bar
+
+  // clearing the notification, will be set below if the stylesheet has set
+  // anything for it
+  m_originalNotificationIcon = {};
+
+  // QActions created from the .ui file are children of the main window
+  for (QAction* action : findChildren<QAction*>()) {
+    // creating a dummy button
+    auto dummy = std::make_unique<QToolButton>();
+
+    // reusing the action name
+    dummy->setObjectName(action->objectName());
+
+    // styling the button, this has to be done manually because the button is
+    // never added anywhere
+    style()->polish(dummy.get());
+
+    // the button's icon may be null if it wasn't specified in the .qss file,
+    // which can happen if the stylesheet just doesn't override icons, or for
+    // other actions like the pinned custom executables
+    const auto icon = dummy->icon();
+    if (icon.isNull()) {
+      continue;
+    }
+
+    // button associated with the action on the toolbar
+    QWidget* actionWidget = ui->toolBar->widgetForAction(action);
+
+    if (auto* actionButton=dynamic_cast<QAbstractButton*>(actionWidget)) {
+      actionButton->setIcon(icon);
+    }
+
+    // the action's icon is used by the menu bar
+    action->setIcon(icon);
+
+    if (action == ui->actionNotifications) {
+      // if the stylesheet has set a notification icon, remember it here so it
+      // can be used in updateProblemsButton()
+      m_originalNotificationIcon = icon;
+    }
   }
 
-  updatePluginCount();
-  updateModCount();
+  // update the button for the potentially new icon
+  updateProblemsButton();
 }
 
 
@@ -565,8 +622,7 @@ void MainWindow::allowListResize()
 
 void MainWindow::updateStyle(const QString&)
 {
-  // no effect?
-  ensurePolished();
+  resetActionIcons();
 }
 
 void MainWindow::resizeEvent(QResizeEvent *event)
@@ -788,20 +844,52 @@ void MainWindow::scheduleUpdateButton()
 
 void MainWindow::updateProblemsButton()
 {
-  size_t numProblems = checkForProblems();
+  // if the current stylesheet doesn't provide an icon, this is used instead
+  const char* DefaultIconName = ":/MO/gui/warning";
+
+  const std::size_t numProblems = checkForProblems();
+
+  // original icon without a count painted on it
+  const QIcon original = m_originalNotificationIcon.isNull() ?
+    QIcon(DefaultIconName) : m_originalNotificationIcon;
+
+  // final icon
+  QIcon final;
+
   if (numProblems > 0) {
     ui->actionNotifications->setToolTip(tr("There are notifications to read"));
 
-    QPixmap mergedIcon = QPixmap(":/MO/gui/warning").scaled(64, 64);
+    // will contain the original icon, plus a notification count; this also
+    // makes sure the pixmap is exactly 64x64 by requesting the icon that's
+    // as close to 64x64 as possible, and then scaling it up if it's too small
+    QPixmap merged = original.pixmap(64, 64).scaled(64, 64);
+
     {
-      QPainter painter(&mergedIcon);
-      std::string badgeName = std::string(":/MO/gui/badge_") + (numProblems < 10 ? std::to_string(static_cast<long long>(numProblems)) : "more");
+      QPainter painter(&merged);
+
+      const std::string badgeName =
+        std::string(":/MO/gui/badge_") +
+        (numProblems < 10 ? std::to_string(static_cast<long long>(numProblems)) : "more");
+
       painter.drawPixmap(32, 32, 32, 32, QPixmap(badgeName.c_str()));
     }
-    ui->actionNotifications->setIcon(QIcon(mergedIcon));
+
+    final = QIcon(merged);
   } else {
     ui->actionNotifications->setToolTip(tr("There are no notifications"));
-    ui->actionNotifications->setIcon(QIcon(":/MO/gui/warning"));
+
+    // no change
+    final = original;
+  }
+
+  // setting the icon on the action (shown on the menu)
+  ui->actionNotifications->setIcon(final);
+
+  // setting the icon on the toolbar button
+  if (auto* actionWidget=ui->toolBar->widgetForAction(ui->actionNotifications)) {
+    if (auto* button=dynamic_cast<QAbstractButton*>(actionWidget)) {
+      button->setIcon(final);
+    }
   }
 }
 
@@ -1049,6 +1137,16 @@ void MainWindow::showEvent(QShowEvent *event)
   QMainWindow::showEvent(event);
 
   if (!m_WasVisible) {
+    // this needs to be connected here instead of in the constructor because the
+    // actual changing of the stylesheet is done by MOApplication, which
+    // connects its signal in runApplication() (in main.cpp), and that happens
+    // _after_ the MainWindow is constructed, but _before_ it is shown
+    //
+    // by connecting the event here, changing the style setting will first be
+    // handled by MOApplication, and then in updateStyle(), at which point the
+    // stylesheet has already been set correctly
+    connect(this, SIGNAL(styleChanged(QString)), this, SLOT(updateStyle(QString)));
+
     // only the first time the window becomes visible
     m_Tutorial.registerControl();
 
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 426b0881..b8d9f49f 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -398,6 +398,10 @@ private:
 
   MOBase::DelayedFileWriter m_ArchiveListWriter;
 
+  // icon set by the stylesheet, used to remember its original appearance
+  // when painting the count
+  QIcon m_originalNotificationIcon;
+
   enum class ShortcutType {
     Toolbar,
     Desktop,
@@ -631,6 +635,7 @@ private slots:
   void search_activated();
   void searchClear_activated();
 
+  void resetActionIcons();
   void updateModCount();
   void updatePluginCount();