improve color support for themes (#590)

* improve color support for themes based on ros-visualization/rviz#1319 * fixup style issues * try this again * use Qt::ForegroundRole
ros2 · Aug 20, 2020 · c4e28db · c4e28db
1 parent 5d401ea
commit c4e28db
Show file tree

Hide file tree

Showing 8 changed files with 16 additions and 31 deletions.
diff --git a/rviz_common/help/help.html b/rviz_common/help/help.html
@@ -8,8 +8,8 @@
 <html>
 <head>
 <style type="text/css">
-.odd { background-color: #e0e0e0 }
-.even { background-color: #f0f0f0 }
+.odd { background-color: transparent }
+.even { background-color: transparent }
 .key { font-weight: bold; color: #04597f }
 p { margin-left: 10px }
 </style>

diff --git a/rviz_common/include/rviz_common/properties/status_property.hpp b/rviz_common/include/rviz_common/properties/status_property.hpp
@@ -60,10 +60,6 @@ class RVIZ_COMMON_PUBLIC StatusProperty : public Property
   Qt::ItemFlags getViewFlags(int column) const override;
 
   /// Return the color appropriate for the given status level.
-  /**
-   * Returns an invalid QColor for Ok status, meaning we should use the default
-   * text color.
-   */
   static QColor statusColor(Level level);
 
   /// Return the word appropriate for the given status level: "Ok", "Warn", or "Error".

diff --git a/rviz_common/src/rviz_common/display.cpp b/rviz_common/src/rviz_common/display.cpp
@@ -111,16 +111,6 @@ void Display::queueRender()
 QVariant Display::getViewData(int column, int role) const
 {
   switch (role) {
-    case Qt::BackgroundRole:
-      {
-        /*
-        QLinearGradient q( 0,0, 0,5);
-        q.setColorAt( 0.0, QColor(230,230,230));
-        q.setColorAt( 1.0, Qt::white);
-        return QBrush( q);
-        */
-        return QColor(Qt::white);
-      }
     case Qt::ForegroundRole:
       {
         // if we're item-enabled (not greyed out) and in warn/error state, set appropriate color
@@ -134,7 +124,7 @@ QVariant Display::getViewData(int column, int role) const
               return QColor(40, 120, 197);
             }
           } else {
-            return QColor(Qt::black);
+            return QApplication::palette().color(QPalette::Text);
           }
         }
         break;

diff --git a/rviz_common/src/rviz_common/panel_dock_widget.cpp b/rviz_common/src/rviz_common/panel_dock_widget.cpp
@@ -30,6 +30,7 @@
 
 #include "rviz_common/panel_dock_widget.hpp"
 
+#include <QApplication>
 #include <QChildEvent>
 #include <QCloseEvent>
 #include <QHBoxLayout>
@@ -47,7 +48,7 @@ PanelDockWidget::PanelDockWidget(const QString & name)
   QWidget * title_bar = new QWidget(this);
 
   QPalette pal(palette());
-  pal.setColor(QPalette::Window, QColor(200, 200, 200));
+  pal.setColor(QPalette::Window, QApplication::palette().color(QPalette::Mid));
   title_bar->setAutoFillBackground(true);
   title_bar->setPalette(pal);
   title_bar->setContentsMargins(0, 0, 0, 0);

diff --git a/rviz_common/src/rviz_common/properties/property.cpp b/rviz_common/src/rviz_common/properties/property.cpp
@@ -34,6 +34,8 @@
 #include <climits>  // for INT_MIN and INT_MAX
 #include <string>
 
+#include <QApplication>  // NOLINT: cpplint is unable to handle the include order here
+#include <QPalette>  // NOLINT: cpplint is unable to handle the include order here
 #include <QLineEdit>  // NOLINT: cpplint is unable to handle the include order here
 #include <QSpinBox>  // NOLINT: cpplint is unable to handle the include order here
 
@@ -251,14 +253,8 @@ void Property::setParent(Property * new_parent)
 
 QVariant Property::getViewData(int column, int role) const
 {
-  if (role == Qt::ForegroundRole &&
-    ( parent_ && parent_->getDisableChildren() ) )
-  {
-#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
-    return Qt::gray;
-#else
-    return QColor(Qt::gray);
-#endif
+  if (role == Qt::ForegroundRole && parent_ && parent_->getDisableChildren()) {
+    return QApplication::palette().brush(QPalette::Disabled, QPalette::Text);
   }
 
   switch (column) {

diff --git a/rviz_common/src/rviz_common/properties/property_tree_with_help.cpp b/rviz_common/src/rviz_common/properties/property_tree_with_help.cpp
@@ -71,7 +71,7 @@ void PropertyTreeWithHelp::showHelpForProperty(const Property * property)
   if (property) {
     QString body_text = property->getDescription();
     QString heading = property->getName();
-    QString html = "<html><body bgcolor=\"#EFEBE7\"><strong>" + heading + "</strong><br>" +
+    QString html = "<html><body><strong>" + heading + "</strong><br>" +
       body_text + "</body></html>";
     help_->setHtml(html);
   } else {

diff --git a/rviz_common/src/rviz_common/properties/status_property.cpp b/rviz_common/src/rviz_common/properties/status_property.cpp
@@ -28,7 +28,9 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <QApplication>
 #include <QColor>
+#include <QPalette>
 
 #include "rviz_common/load_resource.hpp"
 #include "rviz_common/properties/property_tree_model.hpp"
@@ -54,6 +56,10 @@ StatusProperty::StatusProperty(
   status_icons_[0] = loadPixmap("package://rviz_common/icons/ok.png");
   status_icons_[1] = loadPixmap("package://rviz_common/icons/warning.png");
   status_icons_[2] = loadPixmap("package://rviz_common/icons/error.png");
+
+  if (!status_colors_[0].isValid()) {  // initialize default text color once
+    status_colors_[0] = QApplication::palette().color(QPalette::Text);
+  }
 }
 
 bool StatusProperty::setValue(const QVariant & new_value)

diff --git a/rviz_common/src/rviz_common/view_controller.cpp b/rviz_common/src/rviz_common/view_controller.cpp
@@ -153,10 +153,6 @@ QVariant ViewController::getViewData(int column, int role) const
 
   if (is_active_) {
     switch (role) {
-      case Qt::BackgroundRole:
-        {
-          return QColor(0xba, 0xad, 0xa4);
-        }
       case Qt::FontRole:
         {
           QFont font;