Merge pull request #584 from ispyisail/fix-potential-selector-cancel

Fix: potential-selector dialog can't actually be cancelled (#581)

sources/ui/potentialselectordialog.cpp

@@ -188,8 +188,11 @@ class LinkReportPotentialSelector : public AbstractPotentialSelector
 //### END PRIVATE CLASS ###//
 
 
-ConductorProperties PotentialSelectorDialog::chosenProperties(QList<ConductorProperties> list, QWidget *widget)
+ConductorProperties PotentialSelectorDialog::chosenProperties(QList<ConductorProperties> list, QWidget *widget, bool *cancelled)
 {
+	if (cancelled)
+		*cancelled = false;
+
 	if (list.isEmpty()) {
 		return ConductorProperties() ;
 	} else if (list.size() == 1) {
@@ -222,11 +225,25 @@ ConductorProperties PotentialSelectorDialog::chosenProperties(QList<ConductorPro
 		layout.addWidget(b);
 		H.insert(b, cp);
 	}
-	QDialogButtonBox *button_box = new QDialogButtonBox(QDialogButtonBox::Ok, &dialog);
+
+	// Pre-select the first entry: without this, accepting the dialog without
+	// ever touching a radio button silently returned blank properties too,
+	// the same failure mode as the missing Cancel button below.
+	if (!H.isEmpty())
+		H.constBegin().key()->setChecked(true);
+
+	QDialogButtonBox *button_box = new QDialogButtonBox(
+				QDialogButtonBox::Ok | QDialogButtonBox::Cancel, &dialog);
 	layout.addWidget(button_box);
 	connect(button_box, &QDialogButtonBox::accepted, &dialog, &QDialog::accept);
+	connect(button_box, &QDialogButtonBox::rejected, &dialog, &QDialog::reject);
+
+	if (dialog.exec() != QDialog::Accepted) {
+		if (cancelled)
+			*cancelled = true;
+		return ConductorProperties();
+	}
 
-	dialog.exec();
 	for (QRadioButton *b : H.keys()) {
 		if(b->isChecked()) {
 			return H.value(b);

sources/ui/potentialselectordialog.h

@@ -64,7 +64,11 @@ namespace Ui {
 
 	the static function chosenProperties,
 	open a dialog who ask user to make a choice between the given
-	properties
+	properties. If the dialog is cancelled (Cancel button, Escape, or the
+	window's close button) and @a cancelled is non-null, *cancelled is set
+	to true and an empty ConductorProperties() is returned; callers that
+	care about a real cancellation (as opposed to "no properties to choose
+	from") should check it rather than relying on the returned value alone.
 */
 class PotentialSelectorDialog : public QDialog
 {
@@ -73,7 +77,8 @@ class PotentialSelectorDialog : public QDialog
 	public:
 		static ConductorProperties chosenProperties(
 				QList<ConductorProperties> list,
-				QWidget *parent = nullptr);
+				QWidget *parent = nullptr,
+				bool *cancelled = nullptr);
 
 	public:
 		explicit PotentialSelectorDialog(

sources/ui/potentialselectordialog.ui

@@ -55,7 +55,7 @@ Veuillez choisir les propriétées à appliquer au nouveau potentiel.</string>
       <enum>Qt::Horizontal</enum>
      </property>
      <property name="standardButtons">
-      <set>QDialogButtonBox::Ok</set>
+      <set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
      </property>
     </widget>
    </item>

sources/utils/conductorcreator.cpp

@@ -42,8 +42,10 @@ ConductorCreator::ConductorCreator(Diagram *d, QList<Terminal *> terminals_list)
 		return;
 	}
 	m_properties = m_terminals_list.first()->diagram()->defaultConductorProperties;
-	
-	setUpPropertieToUse();
+
+	if (!setUpPropertieToUse()) {
+		return;
+	}
 	Terminal *hub_terminal = hubTerminal();
 	
 	d->undoStack().beginMacro(QObject::tr("Création de conducteurs"));
@@ -95,12 +97,15 @@ void ConductorCreator::create(Diagram *d, const QPolygonF &polygon)
 
 /**
 	@brief ConductorCreator::propertieToUse
-	@return the conductor properties to use for the new conductors.
+	@return true if the caller should proceed with conductor creation,
+	false if the user cancelled the potential-selection dialog (in which
+	case no properties were chosen and creation must be aborted rather
+	than proceeding with blank/default properties).
 */
-void ConductorCreator::setUpPropertieToUse()
+bool ConductorCreator::setUpPropertieToUse()
 {
 	QList<Conductor *> potentials = existingPotential();
-	
+
 		//There is an existing potential
 		//we get one of them
 	if (!potentials.isEmpty())
@@ -111,8 +116,12 @@ void ConductorCreator::setUpPropertieToUse()
 			for(Conductor *c : potentials) {
 				cp_list.append(c->properties());
 			}
-			
-			m_properties = PotentialSelectorDialog::chosenProperties(cp_list);
+
+			bool cancelled = false;
+			m_properties = PotentialSelectorDialog::chosenProperties(cp_list, nullptr, &cancelled);
+			if (cancelled) {
+				return false;
+			}
 			for (Conductor *c : potentials) {
 				if (c->properties() == m_properties) {
 					m_sequential_number = c->sequenceNum();
@@ -124,11 +133,12 @@ void ConductorCreator::setUpPropertieToUse()
 			m_properties = potentials.first()->properties();
 			m_sequential_number = potentials.first()->sequenceNum();
 		}
-		return;
+		return true;
 	}
-	
+
 		//get a new properties
 	ConductorAutoNumerotation::newProperties(m_terminals_list.first()->diagram(), m_properties, m_sequential_number);
+	return true;
 }
 
 /**

sources/utils/conductorcreator.h

@@ -40,7 +40,7 @@ class ConductorCreator
 		static void create(Diagram *d, const QPolygonF &polygon);
 		
 	private:
-		void setUpPropertieToUse();
+		bool setUpPropertieToUse();
 		QList<Conductor *> existingPotential();
 		Terminal *hubTerminal();