Issue #68: Added a colored box around an item to indicate an error

This along with the text displayed in red when not selected, indicates there is an error in the item.

CHANGELOG.md

@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 - [Splashscreen displayed on the same screen than Hive will be shown](https://github.com/christophe-calmejane/Hive/issues/20)
 - Refresh issue when gPTP changes for some non-milan devices
+- [Red text no longer applied when item is selected (fixed by using a colored box around the item)](https://github.com/christophe-calmejane/Hive/issues/68)
 
 ## [1.1.0] - 2019-05-21
 ### Added

src/CMakeLists.txt

@@ -127,7 +127,7 @@ set(HEADER_FILES_COMMON
 	controlledEntityTreeWidget.hpp
 	entityInspector.hpp
 	entityLogoCache.hpp
-	highlightForegroundItemDelegate.hpp
+	errorItemDelegate.hpp
 	imageItemDelegate.hpp
 	mappingMatrix.hpp
 	activeNetworkInterfaceModel.hpp
@@ -206,7 +206,7 @@ set(SOURCE_FILES_COMMON
 	settingsDialog.cpp
 	aecpCommandComboBox.cpp
 	entityLogoCache.cpp
-	highlightForegroundItemDelegate.cpp
+	errorItemDelegate.cpp
 	imageItemDelegate.cpp
 	controlledEntityTreeWidget.cpp
 	entityInspector.cpp

src/avdecc/controllerModel.cpp

@@ -19,16 +19,21 @@
 
 #include "controllerModel.hpp"
 #include "helper.hpp"
-#include <la/avdecc/utils.hpp>
-#include <la/avdecc/logger.hpp>
+#include "entityLogoCache.hpp"
+#include "imageItemDelegate.hpp"
+#include "errorItemDelegate.hpp"
 #include "avdecc/controllerManager.hpp"
 #include "avdecc/mcDomainManager.hpp"
-#include "entityLogoCache.hpp"
 #include "settingsManager/settings.hpp"
 #include "toolkit/material/color.hpp"
+
+#include <la/avdecc/utils.hpp>
+#include <la/avdecc/logger.hpp>
+
+#include <QFont>
+
 #include <algorithm>
 #include <array>
-#include <QFont>
 #include <set>
 #include <unordered_map>
 #include <unordered_set>
@@ -318,15 +323,22 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 		}
 		else if (column == ControllerModel::Column::EntityID)
 		{
-			if (role == Qt::ForegroundRole)
+			if (role == ErrorItemDelegate::ErrorRole || role == Qt::ForegroundRole)
 			{
 				auto const it = _entitiesWithErrorCounter.find(entityID);
 				if (it != std::end(_entitiesWithErrorCounter))
 				{
 					auto const& entityWithErrorCounter{ it->second };
 					if (entityWithErrorCounter.hasError())
 					{
-						return qt::toolkit::material::color::value(qt::toolkit::material::color::Name::Red);
+						if (role == ErrorItemDelegate::ErrorRole)
+						{
+							return true;
+						}
+						else if (role == Qt::ForegroundRole)
+						{
+							return qt::toolkit::material::color::value(qt::toolkit::material::color::Name::Red);
+						}
 					}
 				}
 			}
@@ -339,7 +351,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 		}
 		else if (column == ControllerModel::Column::EntityLogo)
 		{
-			if (role == Qt::UserRole)
+			if (role == ImageItemDelegate::ImageRole)
 			{
 				if (data.aemSupported)
 				{
@@ -356,7 +368,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 		{
 			switch (role)
 			{
-				case Qt::UserRole:
+				case ImageItemDelegate::ImageRole:
 				{
 					try
 					{
@@ -389,7 +401,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 		{
 			switch (role)
 			{
-				case Qt::UserRole:
+				case ImageItemDelegate::ImageRole:
 				{
 					try
 					{
@@ -411,7 +423,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 		{
 			switch (role)
 			{
-				case Qt::UserRole:
+				case ImageItemDelegate::ImageRole:
 				{
 					try
 					{
@@ -722,7 +734,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 			data.acquireState = computeAcquireState(acquireState);
 			data.acquireStateTooltip = helper::acquireStateToString(acquireState, owningEntity);
 
-			dataChanged(*row, ControllerModel::Column::AcquireState, { Qt::UserRole });
+			dataChanged(*row, ControllerModel::Column::AcquireState, { ImageItemDelegate::ImageRole });
 		}
 	}
 
@@ -735,7 +747,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 			data.lockState = computeLockState(lockState);
 			data.lockStateTooltip = helper::lockStateToString(lockState, lockingEntity);
 
-			dataChanged(*row, ControllerModel::Column::LockState, { Qt::UserRole });
+			dataChanged(*row, ControllerModel::Column::LockState, { ImageItemDelegate::ImageRole });
 		}
 	}
 
@@ -847,7 +859,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 		{
 			if (auto const row = entityRow(entityID))
 			{
-				dataChanged(*row, ControllerModel::Column::EntityLogo, { Qt::UserRole });
+				dataChanged(*row, ControllerModel::Column::EntityLogo, { ImageItemDelegate::ImageRole });
 			}
 		}
 	}
@@ -866,7 +878,7 @@ class ControllerModelPrivate : public QObject, private settings::SettingsManager
 				auto const topLeft = q->createIndex(0, column, nullptr);
 				auto const bottomRight = q->createIndex(rowCount(), column, nullptr);
 
-				emit q->dataChanged(topLeft, bottomRight, { Qt::UserRole });
+				emit q->dataChanged(topLeft, bottomRight, { ImageItemDelegate::ImageRole });
 			}
 		}
 	}

src/connectionMatrix/model.hpp

@@ -20,7 +20,7 @@
 #pragma once
 
 // Debugging options
-#define ENABLE_CONNECTION_MATRIX_DEBUG 1
+//#define ENABLE_CONNECTION_MATRIX_DEBUG 1
 #define ENABLE_CONNECTION_MATRIX_HIGHLIGHT_DATA_CHANGED 1
 #define ENABLE_CONNECTION_MATRIX_TOOLTIP 1
 

src/controlledEntityTreeWidget.cpp

@@ -18,13 +18,13 @@
 */
 
 #include "controlledEntityTreeWidget.hpp"
-
-#include <la/avdecc/controller/internals/avdeccControlledEntity.hpp>
 #include "avdecc/controllerManager.hpp"
 #include "avdecc/helper.hpp"
-
+#include "errorItemDelegate.hpp"
 #include "nodeVisitor.hpp"
 
+#include <la/avdecc/controller/internals/avdeccControlledEntity.hpp>
+
 #include <QHeaderView>
 #include <QMenu>
 
@@ -54,6 +54,7 @@ class NodeItem : public QObject, public QTreeWidgetItem
 	void setHasError(bool const hasError)
 	{
 		_hasError = hasError;
+		setData(0, ErrorItemDelegate::ErrorRole, _hasError);
 		setForeground(0, _hasError ? Qt::red : Qt::black);
 
 		// Also update the parent node
@@ -236,7 +237,7 @@ class ControlledEntityTreeWidgetPrivate : public QObject, public la::avdecc::con
 			return;
 		}
 
-		if (auto* item = findItem({ la::avdecc::entity::model::DescriptorType::Entity, descriptorIndex }))
+		if (auto* item = findItem({ la::avdecc::entity::model::DescriptorType::StreamInput, descriptorIndex }))
 		{
 			item->setHasError(!errorCounters.empty());
 		}

src/entityInspector.cpp

@@ -31,12 +31,14 @@ EntityInspector::EntityInspector(QWidget* parent)
 	_layout.setContentsMargins(0, 0, 0, 0);
 	_layout.addWidget(&_splitter);
 
+	_controlledEntityTreeWiget.setItemDelegate(&_itemDelegate);
+	_nodeTreeWiget.setItemDelegate(&_itemDelegate);
+
 	_splitter.addWidget(&_controlledEntityTreeWiget);
 	_splitter.addWidget(&_nodeTreeWiget);
 	_splitter.setStretchFactor(0, 0); // Descriptors List has less weight than
 	_splitter.setStretchFactor(1, 1); // the Descriptor itself, as far as expand is concerned
 
-
 	_nodeTreeWiget.setColumnCount(2);
 	_nodeTreeWiget.setHeaderLabels({ "", "" });
 

src/entityInspector.hpp

@@ -53,4 +53,5 @@ class EntityInspector : public QWidget
 	QSplitter _splitter{ Qt::Vertical, this };
 	ControlledEntityTreeWidget _controlledEntityTreeWiget{ this };
 	NodeTreeWidget _nodeTreeWiget{ this };
+	ErrorItemDelegate _itemDelegate{ this };
 };

src/errorItemDelegate.cpp

@@ -0,0 +1,63 @@
+/*
+* Copyright (C) 2017-2019, Emilien Vallot, Christophe Calmejane and other contributors
+
+* This file is part of Hive.
+
+* Hive is free software: you can redistribute it and/or modify
+* it under the terms of the GNU Lesser General Public License as published by
+* the Free Software Foundation, either version 3 of the License, or
+* (at your option) any later version.
+
+* Hive is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU Lesser General Public License for more details.
+
+* You should have received a copy of the GNU Lesser General Public License
+* along with Hive.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "errorItemDelegate.hpp"
+#include "toolkit/material/color.hpp"
+
+ErrorItemDelegate::ErrorItemDelegate(QObject* parent) noexcept
+	: QStyledItemDelegate(parent)
+{
+	// Configure settings observers
+	auto& settings = settings::SettingsManager::getInstance();
+	settings.registerSettingObserver(settings::ThemeColorIndex.name, this);
+}
+
+ErrorItemDelegate::~ErrorItemDelegate() noexcept
+{
+	// Remove settings observers
+	auto& settings = settings::SettingsManager::getInstance();
+	settings.unregisterSettingObserver(settings::ThemeColorIndex.name, this);
+}
+
+void ErrorItemDelegate::paint(QPainter* painter, QStyleOptionViewItem const& option, QModelIndex const& index) const
+{
+	QStyledItemDelegate::paint(painter, option, index);
+
+	if (index.data(ErrorRole).toBool())
+	{
+		if (option.state & QStyle::StateFlag::State_Selected)
+		{
+			painter->setPen(QPen{ qt::toolkit::material::color::complementatyValue(_colorName, qt::toolkit::material::color::Shade::Shade600), 2 });
+			painter->drawRect(option.rect.adjusted(1, 1, -1, -1));
+		}
+		else
+		{
+			painter->setPen(qt::toolkit::material::color::value(qt::toolkit::material::color::Name::Red));
+			painter->drawRect(option.rect.adjusted(0, 0, -1, -1));
+		}
+	}
+}
+
+void ErrorItemDelegate::onSettingChanged(settings::SettingsManager::Setting const& name, QVariant const& value) noexcept
+{
+	if (name == settings::ThemeColorIndex.name)
+	{
+		_colorName = qt::toolkit::material::color::Palette::name(value.toInt());
+	}
+}

src/highlightForegroundItemDelegate.hpp → src/errorItemDelegate.hpp

@@ -19,14 +19,26 @@
 
 #pragma once
 
+#include "settingsManager/settings.hpp"
+
 #include <QStyledItemDelegate>
 
-// This custom delegate allows to keep the Qt::ForegroundRole visible even when a cell is highlighted
-class HighlightForegroundItemDelegate : public QStyledItemDelegate
+// This custom delegate allows to indicate a cell is on error
+class ErrorItemDelegate : public QStyledItemDelegate, private settings::SettingsManager::Observer
 {
 public:
-	using QStyledItemDelegate::QStyledItemDelegate;
+	static constexpr auto ErrorRole = Qt::UserRole + 1;
+
+	explicit ErrorItemDelegate(QObject* parent = nullptr) noexcept;
+	~ErrorItemDelegate() noexcept;
 
 protected:
 	virtual void paint(QPainter* painter, QStyleOptionViewItem const& option, QModelIndex const& index) const override;
+
+private:
+	// settings::SettingsManager::Observer overrides
+	virtual void onSettingChanged(settings::SettingsManager::Setting const& name, QVariant const& value) noexcept override;
+
+	// Private members
+	qt::toolkit::material::color::Name _colorName{ qt::toolkit::material::color::DefaultColor };
 };

src/imageItemDelegate.cpp

@@ -24,7 +24,7 @@ void ImageItemDelegate::paint(QPainter* painter, QStyleOptionViewItem const& opt
 {
 	QStyledItemDelegate::paint(painter, option, index);
 
-	auto const userData{ index.data(Qt::UserRole) };
+	auto const userData{ index.data(ImageRole) };
 	if (!userData.canConvert<QImage>())
 	{
 		return;

src/imageItemDelegate.hpp

@@ -25,6 +25,7 @@ class ImageItemDelegate : public QStyledItemDelegate
 {
 public:
 	using QStyledItemDelegate::QStyledItemDelegate;
+	static constexpr auto ImageRole = Qt::UserRole + 1;
 
 protected:
 	virtual void paint(QPainter* painter, QStyleOptionViewItem const& option, QModelIndex const& index) const override;

src/mainWindow.cpp

@@ -54,7 +54,7 @@
 #include "aboutDialog.hpp"
 #include "deviceDetailsDialog.hpp"
 #include "entityLogoCache.hpp"
-#include "highlightForegroundItemDelegate.hpp"
+#include "errorItemDelegate.hpp"
 #include "imageItemDelegate.hpp"
 #include "nodeVisitor.hpp"
 #include "settingsDialog.hpp"
@@ -401,8 +401,8 @@ void MainWindowImpl::createControllerView()
 	controllerTableView->setItemDelegateForColumn(la::avdecc::utils::to_integral(avdecc::ControllerModel::Column::AcquireState), imageItemDelegate);
 	controllerTableView->setItemDelegateForColumn(la::avdecc::utils::to_integral(avdecc::ControllerModel::Column::LockState), imageItemDelegate);
 
-	auto* highlightForegroundItemDelegate{ new HighlightForegroundItemDelegate{ _parent } };
-	controllerTableView->setItemDelegateForColumn(la::avdecc::utils::to_integral(avdecc::ControllerModel::Column::EntityID), highlightForegroundItemDelegate);
+	auto* errorItemDelegate{ new ErrorItemDelegate{ _parent } };
+	controllerTableView->setItemDelegateForColumn(la::avdecc::utils::to_integral(avdecc::ControllerModel::Column::EntityID), errorItemDelegate);
 
 	_controllerDynamicHeaderView.setHighlightSections(false);
 	_controllerDynamicHeaderView.setMandatorySection(la::avdecc::utils::to_integral(avdecc::ControllerModel::Column::EntityID));