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

PotentialSelectorDialog::chosenProperties() built an OK-only dialog
and discarded exec()'s return value entirely:

    dialog.exec();
    for (QRadioButton *b : H.keys()) {
        if (b->isChecked()) return H.value(b);
    }
    return ConductorProperties();

Escape and the window close button already trigger QDialog::reject()
on a plain QDialog, but since the result was never checked, dismissing
the dialog without picking anything just silently returned blank
ConductorProperties() -- the same value returned when a real potential
was chosen but happened to produce empty properties. There was no way
to distinguish "the user cancelled" from "the user chose an empty
potential", so the caller always proceeded as if a choice had been
made.

Add a real Cancel button, check dialog.exec() == QDialog::Accepted,
and report cancellation through a new optional `bool *cancelled`
out-parameter. Also pre-select the first entry, closing a related gap
where clicking OK without ever touching a radio button hit the exact
same "silently returns blank properties" failure mode.

Thread the result through ConductorCreator::setUpPropertieToUse()
(now returning bool) so the calling constructor aborts and creates no
conductors at all when the user cancels, instead of proceeding with
blank properties.

The sibling constructor-based PotentialSelectorDialog (used for
conductor/report potential linking, a separate flow) already gates its
side effects behind on_buttonBox_accepted(), so cancelling it was
already safe -- gave it a visible Cancel button too for consistency
while touching this file, no behavior change there.

Verified with real Qt event simulation (QTest::mouseClick/keyClick)
against the exact new dialog-building logic: clicking Cancel and
pressing Escape both correctly report cancellation with empty
properties; clicking OK untouched returns the pre-selected first
entry; selecting the second option then OK returns that selection.

See discussion #581.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ispyisail
2026-07-31 20:39:04 +12:00
parent 8793df4c86
commit 68e75b4c55
5 changed files with 48 additions and 16 deletions
+20 -3
View File
@@ -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);
+7 -2
View File
@@ -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(
+1 -1
View File
@@ -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>
+19 -9
View File
@@ -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;
}
/**
+1 -1
View File
@@ -40,7 +40,7 @@ class ConductorCreator
static void create(Diagram *d, const QPolygonF &polygon);
private:
void setUpPropertieToUse();
bool setUpPropertieToUse();
QList<Conductor *> existingPotential();
Terminal *hubTerminal();