From: Juergen Spitzmueller <spitz@lyx.org>
Date: Thu, 22 Dec 2022 14:01:58 +0000 (+0100)
Subject: Inform user if panel or tab has invalid content (#10827)
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=d888d0d130edc7b72d4afea01a93ede2ea0ce648;p=features.git

Inform user if panel or tab has invalid content (#10827)

This adds a warning icon to either the tab header or the panel stack
entry item if a widget on the panel/stack has invalid content.

Particularly helpful to get aware of such content on other tabs/panes
than the one currently selected.
---

diff --git a/src/frontends/qt/ButtonController.cpp b/src/frontends/qt/ButtonController.cpp
index 47d9c7e615..a2c7359a0d 100644
--- a/src/frontends/qt/ButtonController.cpp
+++ b/src/frontends/qt/ButtonController.cpp
@@ -11,6 +11,8 @@
 #include <config.h>
 
 #include "ButtonController.h"
+#include "GuiApplication.h"
+#include "PanelStack.h"
 
 #include "qt_helpers.h"
 
@@ -21,6 +23,7 @@
 #include <QLineEdit>
 #include <QLabel>
 #include <QList>
+#include <QTabWidget>
 #include <QValidator>
 
 
@@ -36,18 +39,25 @@ namespace frontend {
 class CheckedLineEdit
 {
 public:
-	CheckedLineEdit(QLineEdit * input, QWidget * label = nullptr);
+	CheckedLineEdit(QLineEdit * input, QWidget * label = nullptr,
+			int tabindex = -1, QString const panel = QString());
+	/// check the widget and do visual marking
 	bool check() const;
+	/// reset all visual markings for tabs or panel sections
+	void setSectionsValid() const;
 
 private:
 	// non-owned
 	QLineEdit * input_;
-	QWidget * label_;
+	QWidget * target_;
+	int tab_index_;
+	QString panel_name_;
 };
 
 
-CheckedLineEdit::CheckedLineEdit(QLineEdit * input, QWidget * label)
-	: input_(input), label_(label)
+CheckedLineEdit::CheckedLineEdit(QLineEdit * input, QWidget * label,
+				 int tabindex, QString const panel)
+	: input_(input), target_(label), tab_index_(tabindex), panel_name_(panel)
 {}
 
 
@@ -55,8 +65,8 @@ bool CheckedLineEdit::check() const
 {
 	if (!input_->isEnabled()) {
 		// we do not check diabled widgets
-		if (label_)
-			setValid(label_, true);
+		if (target_)
+			setValid(target_, true);
 		return true;
 	}
 
@@ -70,13 +80,37 @@ bool CheckedLineEdit::check() const
 
 	// Visual feedback.
 	setValid(input_, valid);
-	if (label_)
-		setValid(label_, valid);
+	if (target_) {
+		if (!valid && !panel_name_.isEmpty() && qobject_cast<PanelStack*>(target_) != nullptr) {
+			qobject_cast<PanelStack*>(target_)->markPanelValid(panel_name_, false);
+			// this is a panel, so stop here.
+			return valid;
+		}
+		setValid(target_, valid);
+		if (!valid && tab_index_ >= 0 && qobject_cast<QTabWidget*>(target_) != nullptr) {
+			QIcon warn(getPixmap("images/", "emblem-shellescape", "svgz,png"));
+			QTabBar * tb = qobject_cast<QTabWidget*>(target_)->tabBar();
+			tb->setTabIcon(tab_index_, warn);
+			tb->setTabToolTip(tab_index_, qt_("This tab contains invalid input. Please fix!"));
+		}
+	}
 
 	return valid;
 }
 
 
+void CheckedLineEdit::setSectionsValid() const
+{
+	if (target_ && tab_index_ >= 0 && qobject_cast<QTabWidget*>(target_) != nullptr) {
+		QTabBar * tb = qobject_cast<QTabWidget*>(target_)->tabBar();
+		tb->setTabIcon(tab_index_, QIcon());
+		tb->setTabToolTip(tab_index_, QString());
+	}
+	else if (!panel_name_.isEmpty() && qobject_cast<PanelStack*>(target_) != nullptr)
+		qobject_cast<PanelStack*>(target_)->markPanelValid(panel_name_, true);
+}
+
+
 /////////////////////////////////////////////////////////////////////////
 //
 // ButtonController::Private
@@ -94,6 +128,9 @@ public:
 	bool checkWidgets() const
 	{
 		bool valid = true;
+		for (const CheckedLineEdit & w : checked_widgets_) {
+			w.setSectionsValid();
+		}
 		for (const CheckedLineEdit & w : checked_widgets_)
 			valid &= w.check();
 		return valid;
@@ -240,9 +277,15 @@ void ButtonController::refresh() const
 }
 
 
-void ButtonController::addCheckedLineEdit(QLineEdit * input, QWidget * label)
+void ButtonController::addCheckedLineEdit(QLineEdit * input, QWidget * target, int tabindex)
+{
+	d->checked_widgets_.append(CheckedLineEdit(input, target, tabindex));
+}
+
+
+void ButtonController::addCheckedLineEditPanel(QLineEdit * input, QWidget * target, QString const panel)
 {
-	d->checked_widgets_.append(CheckedLineEdit(input, label));
+	d->checked_widgets_.append(CheckedLineEdit(input, target, -1, panel));
 }
 
 
diff --git a/src/frontends/qt/ButtonController.h b/src/frontends/qt/ButtonController.h
index c34c0cdf13..fa694e6f33 100644
--- a/src/frontends/qt/ButtonController.h
+++ b/src/frontends/qt/ButtonController.h
@@ -18,6 +18,7 @@ class QWidget;
 class QPushButton;
 class QLineEdit;
 class QCheckBox;
+class QString;
 
 namespace lyx {
 namespace frontend {
@@ -106,7 +107,12 @@ public:
 	/** Add a widget to the list of all widgets whose validity should
 	 *  be checked explicitly when the buttons are refreshed.
 	 */
-	void addCheckedLineEdit(QLineEdit * input, QWidget * label = 0);
+	void addCheckedLineEdit(QLineEdit * input, QWidget * target = 0, int tabindex = -1);
+
+	/** Add a widget to the list of all widgets whose validity should
+	 *  be checked explicitly when the buttons are refreshed.
+	 */
+	void addCheckedLineEditPanel(QLineEdit * input, QWidget * target, QString const panel);
 
 private:
 	/// noncopyable
diff --git a/src/frontends/qt/GuiDocument.cpp b/src/frontends/qt/GuiDocument.cpp
index 6a6221915e..22e2069d24 100644
--- a/src/frontends/qt/GuiDocument.cpp
+++ b/src/frontends/qt/GuiDocument.cpp
@@ -576,6 +576,7 @@ void PreambleModule::editExternal() {
 		preambleTE->document()->setPlainText(toqstr(s));
 		tempfile_.reset();
 		editPB->setText(qt_("&Edit Externally"));
+		editPB->setStyleSheet("");
 		changed();
 		return;
 	}
@@ -592,6 +593,8 @@ void PreambleModule::editExternal() {
 	preambleTE->setReadOnly(true);
 	theFormats().edit(*current_id_, tempfilename, format);
 	editPB->setText(qt_("&End Edit"));
+	editPB->setStyleSheet(
+		colorButtonStyleSheet(QColor(255, 0, 0)));
 	changed();
 }
 
@@ -855,9 +858,9 @@ GuiDocument::GuiDocument(GuiView & lv)
 	textLayoutModule->lspacingLE->setValidator(new QDoubleValidator(
 		textLayoutModule->lspacingLE));
 	textLayoutModule->indentLE->setValidator(new LengthValidator(
-		textLayoutModule->indentLE));
+		textLayoutModule->indentLE, false));
 	textLayoutModule->skipLE->setValidator(new LengthValidator(
-		textLayoutModule->skipLE));
+		textLayoutModule->skipLE, false));
 
 	textLayoutModule->indentCO->addItem(qt_("Default"), toqstr("default"));
 	textLayoutModule->indentCO->addItem(qt_("Custom"), toqstr("custom"));
@@ -877,7 +880,9 @@ GuiDocument::GuiDocument(GuiView & lv)
 		Spacing::Other, qt_("Custom"));
 	// initialize the length validator
 	bc().addCheckedLineEdit(textLayoutModule->indentLE, textLayoutModule->indentRB);
+	bc().addCheckedLineEditPanel(textLayoutModule->indentLE, docPS, N_("Text Layout"));
 	bc().addCheckedLineEdit(textLayoutModule->skipLE, textLayoutModule->skipRB);
+	bc().addCheckedLineEditPanel(textLayoutModule->skipLE, docPS, N_("Text Layout"));
 
 	textLayoutModule->tableStyleCO->addItem(qt_("Default"), toqstr("default"));
 	getTableStyles();
@@ -1184,8 +1189,10 @@ GuiDocument::GuiDocument(GuiView & lv)
 	pageLayoutModule->pagestyleCO->addItem(qt_("fancy"));
 	bc().addCheckedLineEdit(pageLayoutModule->paperheightLE,
 		pageLayoutModule->paperheightL);
+	bc().addCheckedLineEditPanel(pageLayoutModule->paperheightLE, docPS, N_("Page Layout"));
 	bc().addCheckedLineEdit(pageLayoutModule->paperwidthLE,
 		pageLayoutModule->paperwidthL);
+	bc().addCheckedLineEditPanel(pageLayoutModule->paperwidthLE, docPS, N_("Page Layout"));
 
 	QComboBox * cb = pageLayoutModule->papersizeCO;
 	cb->addItem(qt_("Default"));
@@ -1287,20 +1294,36 @@ GuiDocument::GuiDocument(GuiView & lv)
 
 	bc().addCheckedLineEdit(marginsModule->topLE,
 		marginsModule->topL);
+	bc().addCheckedLineEditPanel(marginsModule->topLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->bottomLE,
 		marginsModule->bottomL);
+	bc().addCheckedLineEditPanel(marginsModule->bottomLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->innerLE,
 		marginsModule->innerL);
+	bc().addCheckedLineEditPanel(marginsModule->innerLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->outerLE,
 		marginsModule->outerL);
+	bc().addCheckedLineEditPanel(marginsModule->outerLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->headsepLE,
 		marginsModule->headsepL);
+	bc().addCheckedLineEditPanel(marginsModule->headsepLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->headheightLE,
 		marginsModule->headheightL);
+	bc().addCheckedLineEditPanel(marginsModule->headheightLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->footskipLE,
 		marginsModule->footskipL);
+	bc().addCheckedLineEditPanel(marginsModule->footskipLE,
+				     docPS, N_("Page Margins"));
 	bc().addCheckedLineEdit(marginsModule->columnsepLE,
 		marginsModule->columnsepL);
+	bc().addCheckedLineEditPanel(marginsModule->columnsepLE,
+				     docPS, N_("Page Margins"));
 
 
 	// color
@@ -1539,9 +1562,10 @@ GuiDocument::GuiDocument(GuiView & lv)
 	mathsModule->MathIndentCO->addItem(qt_("Default"), toqstr("default"));
 	mathsModule->MathIndentCO->addItem(qt_("Custom"), toqstr("custom"));
 	mathsModule->MathIndentLE->setValidator(new LengthValidator(
-		mathsModule->MathIndentLE));
+		mathsModule->MathIndentLE, false));
 	// initialize the length validator
 	bc().addCheckedLineEdit(mathsModule->MathIndentLE, mathsModule->MathIndentCB);
+	bc().addCheckedLineEditPanel(mathsModule->MathIndentLE, docPS, N_("Math Options"));
 	mathsModule->MathNumberingPosCO->addItem(qt_("Left"));
 	mathsModule->MathNumberingPosCO->addItem(qt_("Default"));
 	mathsModule->MathNumberingPosCO->addItem(qt_("Right"));
@@ -1996,16 +2020,16 @@ void GuiDocument::setListingsMessage()
 	static bool isOK = true;
 	QString msg = validateListingsParameters();
 	if (msg.isEmpty()) {
+		listingsModule->listingsTB->setTextColor(QColor());
 		if (isOK)
 			return;
 		isOK = true;
-		// listingsModule->listingsTB->setTextColor("black");
 		listingsModule->listingsTB->setPlainText(
 			qt_("Input listings parameters below. "
 		            "Enter ? for a list of parameters."));
 	} else {
 		isOK = false;
-		// listingsModule->listingsTB->setTextColor("red");
+		listingsModule->listingsTB->setTextColor(QColor(255, 0, 0));
 		listingsModule->listingsTB->setPlainText(msg);
 	}
 }
@@ -2040,6 +2064,8 @@ void GuiDocument::setIndent(int item)
 	textLayoutModule->indentLengthCO->setEnabled(enable);
 	textLayoutModule->skipLE->setEnabled(false);
 	textLayoutModule->skipLengthCO->setEnabled(false);
+	// needed to catch empty custom case
+	bc().refresh();
 	isValid();
 }
 
@@ -2060,6 +2086,8 @@ void GuiDocument::setSkip(int item)
 	bool const enable = (kind == VSpace::LENGTH);
 	textLayoutModule->skipLE->setEnabled(enable);
 	textLayoutModule->skipLengthCO->setEnabled(enable);
+	// needed to catch empty custom case
+	bc().refresh();
 	isValid();
 }
 
@@ -2091,6 +2119,8 @@ void GuiDocument::enableMathIndent(int item)
 	bool const enable = (item == 1);
 	mathsModule->MathIndentLE->setEnabled(enable);
 	mathsModule->MathIndentLengthCO->setEnabled(enable);
+	// needed to catch empty custom case
+	bc().refresh();
 	isValid();
 }
 
@@ -4881,39 +4911,15 @@ void GuiDocument::setLayoutComboByIDString(string const & idString)
 
 bool GuiDocument::isValid()
 {
-	return
-		validateListingsParameters().isEmpty() &&
-		!localLayout->editing() &&
-		!preambleModule->editing() &&
-		(
-			// if we're asking for skips between paragraphs
-			!textLayoutModule->skipRB->isChecked() ||
-			// then either we haven't chosen custom
-			VSpace::VSpaceKind(
-				textLayoutModule->skipCO->itemData(
-					textLayoutModule->skipCO->currentIndex()).toInt())
-				!= VSpace::LENGTH ||
-			// or else a length has been given
-			!textLayoutModule->skipLE->text().isEmpty()
-		) &&
-		(
-			// if we're asking for indentation
-			!textLayoutModule->indentRB->isChecked() ||
-			// then either we haven't chosen custom
-			(textLayoutModule->indentCO->itemData(
-				textLayoutModule->indentCO->currentIndex()) != "custom") ||
-			// or else a length has been given
-			!textLayoutModule->indentLE->text().isEmpty()
-		) &&
-		(
-			// if we're asking for math indentation
-			!mathsModule->MathIndentCB->isChecked() ||
-			// then either we haven't chosen custom
-			(mathsModule->MathIndentCO->itemData(
-				mathsModule->MathIndentCO->currentIndex()) != "custom") ||
-			// or else a length has been given
-			!mathsModule->MathIndentLE->text().isEmpty()
-		);
+	bool const listings_valid = validateListingsParameters().isEmpty();
+	bool const local_layout_valid = !localLayout->editing();
+	bool const preamble_valid = !preambleModule->editing();
+
+	docPS->markPanelValid(N_("Listings[[inset]]"), listings_valid);
+	docPS->markPanelValid(N_("Local Layout"), local_layout_valid && localLayout->isValid());
+	docPS->markPanelValid(N_("LaTeX Preamble"), preamble_valid);
+	
+	return listings_valid && local_layout_valid && preamble_valid;
 }
 
 
diff --git a/src/frontends/qt/GuiExternal.cpp b/src/frontends/qt/GuiExternal.cpp
index 65fb8610a1..70239f19e2 100644
--- a/src/frontends/qt/GuiExternal.cpp
+++ b/src/frontends/qt/GuiExternal.cpp
@@ -187,6 +187,8 @@ GuiExternal::GuiExternal(GuiView & lv)
 	bc().addReadOnly(extraFormatCO);
 	bc().addReadOnly(extraED);
 
+	// Add validated widgets to those that will be
+	// visually marked if invalid
 	bc().addCheckedLineEdit(angleED, angleLA);
 	bc().addCheckedLineEdit(displayscaleED, scaleLA);
 	bc().addCheckedLineEdit(heightED, heightLA);
@@ -197,6 +199,18 @@ GuiExternal::GuiExternal(GuiView & lv)
 	bc().addCheckedLineEdit(ytED, rtLA);
 	bc().addCheckedLineEdit(fileED, fileLA);
 
+	// We also mark the tabs the widgets are in
+	int const tabindex = tab->indexOf(sizetab);
+	bc().addCheckedLineEdit(angleED, tab, tabindex);
+	bc().addCheckedLineEdit(heightED, tab, tabindex);
+	bc().addCheckedLineEdit(widthED, tab, tabindex);
+	bc().addCheckedLineEdit(xlED, tab, tabindex);
+	bc().addCheckedLineEdit(ybED, tab, tabindex);
+	bc().addCheckedLineEdit(xrED, tab, tabindex);
+	bc().addCheckedLineEdit(ytED, tab, tabindex);
+	bc().addCheckedLineEdit(displayscaleED, tab, tab->indexOf(lyxviewtab));
+	bc().addCheckedLineEdit(fileED, tab, tab->indexOf(filetab));
+
 	external::TemplateManager::Templates::const_iterator i1, i2;
 	i1 = external::TemplateManager::get().getTemplates().begin();
 	i2 = external::TemplateManager::get().getTemplates().end();
diff --git a/src/frontends/qt/GuiGraphics.cpp b/src/frontends/qt/GuiGraphics.cpp
index 4ec478af8c..6801f4c5d0 100644
--- a/src/frontends/qt/GuiGraphics.cpp
+++ b/src/frontends/qt/GuiGraphics.cpp
@@ -224,17 +224,32 @@ GuiGraphics::GuiGraphics(GuiView & lv)
 	bc().addReadOnly(getPB);
 	bc().addReadOnly(rotateOrderCB);
 
-	// initialize the length validator
+	// Add validated widgets to those that will be
+	// visually marked if invalid
 	bc().addCheckedLineEdit(Scale, scaleCB);
 	bc().addCheckedLineEdit(Width, WidthCB);
 	bc().addCheckedLineEdit(Height, HeightCB);
-	bc().addCheckedLineEdit(displayscale, scaleLA);
 	bc().addCheckedLineEdit(angle, angleL);
+	bc().addCheckedLineEdit(filename, filenameL);
+	bc().addCheckedLineEdit(displayscale, scaleLA);
 	bc().addCheckedLineEdit(lbX, xL);
 	bc().addCheckedLineEdit(lbY, yL);
 	bc().addCheckedLineEdit(rtX, xL_2);
 	bc().addCheckedLineEdit(rtY, yL_2);
-	bc().addCheckedLineEdit(filename, filenameL);
+
+	// We also mark the tabs the widgets are in
+	int tabindex = tabWidget->indexOf(Graphics);
+	bc().addCheckedLineEdit(Scale, tabWidget, tabindex);
+	bc().addCheckedLineEdit(Width, tabWidget, tabindex);
+	bc().addCheckedLineEdit(Height, tabWidget, tabindex);
+	bc().addCheckedLineEdit(angle, tabWidget, tabindex);
+	bc().addCheckedLineEdit(filename, tabWidget, tabindex);
+	bc().addCheckedLineEdit(displayscale, tabWidget, tabWidget->indexOf(ExtraOptions));
+	tabindex = tabWidget->indexOf(Clipping);
+	bc().addCheckedLineEdit(lbX, tabWidget, tabindex);
+	bc().addCheckedLineEdit(lbY, tabWidget, tabindex);
+	bc().addCheckedLineEdit(rtX, tabWidget, tabindex);
+	bc().addCheckedLineEdit(rtY, tabWidget, tabindex);
 }
 
 
diff --git a/src/frontends/qt/GuiInclude.cpp b/src/frontends/qt/GuiInclude.cpp
index 3d4f3dde1b..6120d669ef 100644
--- a/src/frontends/qt/GuiInclude.cpp
+++ b/src/frontends/qt/GuiInclude.cpp
@@ -84,7 +84,9 @@ GuiInclude::GuiInclude(GuiView & lv)
 	bc().addReadOnly(typeCO);
 	bc().addReadOnly(listingsED);
 
-	bc().addCheckedLineEdit(filenameED, filenameLA);
+	// FIXME does not make sense, as we do not have a validator
+	// for this widget
+	//bc().addCheckedLineEdit(filenameED, filenameLA);
 }
 
 
diff --git a/src/frontends/qt/PanelStack.cpp b/src/frontends/qt/PanelStack.cpp
index c70c7ae543..9b6cb639c8 100644
--- a/src/frontends/qt/PanelStack.cpp
+++ b/src/frontends/qt/PanelStack.cpp
@@ -146,6 +146,22 @@ void PanelStack::showPanel(QString const & name, bool show)
 }
 
 
+void PanelStack::markPanelValid(QString const & name, bool valid)
+{
+	QTreeWidgetItem * item = panel_map_.value(name, 0);
+	LASSERT(item, return);
+
+	if (valid) {
+		item->setIcon(0, QIcon());
+		item->setToolTip(0, QString());
+	} else {
+		QIcon warn(getPixmap("images/", "emblem-shellescape", "svgz,png"));
+		item->setIcon(0, warn);
+		item->setToolTip(0, qt_("This section contains invalid input. Please fix!"));
+	}
+}
+
+
 void PanelStack::setCurrentPanel(QString const & name)
 {
 	QTreeWidgetItem * item = panel_map_.value(name, 0);
diff --git a/src/frontends/qt/PanelStack.h b/src/frontends/qt/PanelStack.h
index 7405954b96..1411b6f965 100644
--- a/src/frontends/qt/PanelStack.h
+++ b/src/frontends/qt/PanelStack.h
@@ -41,6 +41,8 @@ public:
 		QString const & parent = QString());
 	/// show or hide panel
 	void showPanel(QString const & name, bool show);
+	/// Mark panel (content) valid
+	void markPanelValid(QString const & name, bool valid);
 	/// set current panel by logical name
 	void setCurrentPanel(QString const &);
 	///
diff --git a/src/frontends/qt/Validator.cpp b/src/frontends/qt/Validator.cpp
index a59d14dbb2..8ad1831052 100644
--- a/src/frontends/qt/Validator.cpp
+++ b/src/frontends/qt/Validator.cpp
@@ -33,13 +33,15 @@ using namespace std;
 namespace lyx {
 namespace frontend {
 
-LengthValidator::LengthValidator(QWidget * parent)
-	: QValidator(parent)
+LengthValidator::LengthValidator(QWidget * parent, bool const accept_empty)
+	: QValidator(parent), acceptable_if_empty_(accept_empty)
 {}
 
 
 QValidator::State LengthValidator::validate(QString & qtext, int &) const
 {
+	if (!acceptable_if_empty_ && qtext.isEmpty())
+		return QValidator::Intermediate;
 	QLocale loc;
 	bool ok;
 	double d = loc.toDouble(qtext.trimmed(), &ok);
diff --git a/src/frontends/qt/Validator.h b/src/frontends/qt/Validator.h
index 681541f05a..0793cf8104 100644
--- a/src/frontends/qt/Validator.h
+++ b/src/frontends/qt/Validator.h
@@ -48,7 +48,7 @@ class LengthValidator : public QValidator
 	Q_OBJECT
 public:
 	/// Define a validator for widget @c parent.
-	LengthValidator(QWidget * parent);
+	LengthValidator(QWidget * parent, bool const accept_empty = true);
 
 	/** @returns QValidator::Acceptable if @c data is a GlueLength.
 	 *  If not, returns QValidator::Intermediate.
@@ -73,6 +73,7 @@ private:
 	bool glue_length_ = false;
 	bool unsigned_ = false;
 	bool positive_ = false;
+	bool acceptable_if_empty_ = false;
 };