Fix the wire properties panel changing properties nobody edited

With "Show the properties of a selected conductor in the Selection
properties panel" switched on, editing a wire there could change things
the user never touched:

- Enter in a field (Function, Section...) is not used by the line edit,
  so it reaches the checkable "Multifilaire" group box around it, which
  takes it as a click. The wire was switched to single-line, gaining
  ground, neutral and phase symbols. The modal dialog never shows this
  because its OK button takes Enter first.
- Every edit wrote back the whole set of properties as the widget holds
  them, so a value the widget cannot show exactly was rewritten: a dash
  size of 1 became 2.

The panel now swallows Enter at its checkable group boxes, and writes
only the fields that differ from what it showed, through the new
ConductorProperties::applyChanges(). "Apply to all conductors of this
potential" uses the same rule, so the rest of the potential keeps its
own values too.

tst_conductorapplychanges checks each field on its own: change that one
field, and a wire whose every field differs takes it and keeps the rest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
ispyisail
2026-10-01 23:10:15 +13:00
parent 7fdc314e41
commit a8699f6f62
6 changed files with 307 additions and 35 deletions
+52
View File
@@ -757,6 +757,58 @@ void ConductorProperties::applyForEqualAttributes(QList<ConductorProperties> lis
equal = true;
}
/**
@brief ConductorProperties::applyChanges
Copy into this every attribute that differs between before and after,
and leave every other attribute as it is. The Selection properties
panel applies an edit this way, so a conductor keeps everything the
user did not change: its own text, function, cable... and any value
the panel cannot show exactly.
@param before : the properties as they were shown to the user
@param after : the properties after the user's edit
*/
void ConductorProperties::applyChanges(const ConductorProperties &before,
const ConductorProperties &after)
{
const auto take = [](auto &mine, const auto &was, const auto &now) {
if (was != now)
mine = now;
};
take(type, before.type, after.type);
take(color, before.color, after.color);
take(m_bicolor, before.m_bicolor, after.m_bicolor);
take(m_color_2, before.m_color_2, after.m_color_2);
take(m_dash_size, before.m_dash_size, after.m_dash_size);
take(style, before.style, after.style);
take(text, before.text, after.text);
take(text_color, before.text_color, after.text_color);
take(m_formula, before.m_formula, after.m_formula);
take(m_cable, before.m_cable, after.m_cable);
take(m_bus, before.m_bus, after.m_bus);
take(m_function, before.m_function, after.m_function);
take(m_tension_protocol, before.m_tension_protocol, after.m_tension_protocol);
take(m_wire_color, before.m_wire_color, after.m_wire_color);
take(m_wire_section, before.m_wire_section, after.m_wire_section);
take(m_show_text, before.m_show_text, after.m_show_text);
take(text_size, before.text_size, after.text_size);
take(cond_size, before.cond_size, after.cond_size);
take(verti_rotate_text, before.verti_rotate_text, after.verti_rotate_text);
take(horiz_rotate_text, before.horiz_rotate_text, after.horiz_rotate_text);
take(m_one_text_per_folio, before.m_one_text_per_folio, after.m_one_text_per_folio);
take(m_horizontal_alignment, before.m_horizontal_alignment, after.m_horizontal_alignment);
take(m_vertical_alignment, before.m_vertical_alignment, after.m_vertical_alignment);
//Single-line symbols, one by one
SingleLineProperties slp_before = before.singleLineProperties;
SingleLineProperties slp_after = after.singleLineProperties;
take(singleLineProperties.hasGround, slp_before.hasGround, slp_after.hasGround);
take(singleLineProperties.hasNeutral, slp_before.hasNeutral, slp_after.hasNeutral);
take(singleLineProperties.is_pen, slp_before.is_pen, slp_after.is_pen);
if (slp_before.phasesCount() != slp_after.phasesCount())
singleLineProperties.setPhasesCount(slp_after.phasesCount());
}
/**
@brief ConductorProperties::defaultProperties
@return the default properties stored in the setting file
+2
View File
@@ -126,6 +126,8 @@ class ConductorProperties
void fromSettings(QSettings &, const QString & = QString());
static QString typeToString(ConductorType);
void applyForEqualAttributes(QList<ConductorProperties> list);
void applyChanges(const ConductorProperties &before,
const ConductorProperties &after);
static ConductorProperties defaultProperties();
+67 -34
View File
@@ -34,6 +34,7 @@
#include <QCheckBox>
#include <QComboBox>
#include <QGroupBox>
#include <QKeyEvent>
#include <QLineEdit>
#include <QScrollArea>
#include <QSettings>
@@ -89,10 +90,35 @@ ConductorPropertiesEditorWidget::ConductorPropertiesEditorWidget(
// while keeping a minimum height so it stays usable when the dock is short.
setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding);
setMinimumHeight(200);
//Enter in a field (Function, Section...) is not used by the field: it
//goes on to the checkable "Multifilaire" or "Unifilaire" group box
//around it, which takes it as a click and switches the wire between
//multi-line and single-line. The modal dialog never shows this, its
//OK button takes Enter; here nothing does, so stop it at the box.
for (auto *gb : m_cpw->findChildren<QGroupBox *>())
if (gb->isCheckable())
gb->installEventFilter(this);
setDisabled(true);
setConductor(conductor);
}
/**
@brief ConductorPropertiesEditorWidget::eventFilter
Swallow Enter on the checkable group boxes, see the constructor.
*/
bool ConductorPropertiesEditorWidget::eventFilter(QObject *watched, QEvent *event)
{
if (event->type() == QEvent::KeyPress
|| event->type() == QEvent::KeyRelease)
{
const int key = static_cast<QKeyEvent *>(event)->key();
if (key == Qt::Key_Return || key == Qt::Key_Enter)
return true;
}
return PropertiesEditorWidget::eventFilter(watched, event);
}
ConductorPropertiesEditorWidget::~ConductorPropertiesEditorWidget()
{}
@@ -130,7 +156,7 @@ void ConductorPropertiesEditorWidget::apply()
if (!m_conductor || !m_conductor->diagram()) return;
if (QUndoCommand *undo = associatedUndo())
m_conductor->diagram()->undoStack().push(undo);
m_initial = m_conductor->properties();
m_shown = m_cpw->properties();
}
/**
@@ -212,13 +238,13 @@ void ConductorPropertiesEditorWidget::disconnectChangeSignals()
/**
@brief ConductorPropertiesEditorWidget::reset
Discard the in-progress edit, restoring the conductor's current properties.
Discard the in-progress edit, restoring what the widget showed.
*/
void ConductorPropertiesEditorWidget::reset()
{
if (!m_conductor) return;
m_updating = true;
m_cpw->setProperties(m_initial);
m_cpw->setProperties(m_shown);
m_updating = false;
}
@@ -230,54 +256,61 @@ void ConductorPropertiesEditorWidget::updateUi()
{
if (!m_conductor) return;
m_updating = true;
m_initial = m_conductor->properties();
m_cpw->setProperties(m_initial);
m_cpw->setProperties(m_conductor->properties());
//Read back rather than keep the conductor's own values: a value the
//widget cannot show exactly must not count as an edit.
m_shown = m_cpw->properties();
m_updating = false;
}
/**
@brief ConductorPropertiesEditorWidget::associatedUndo
@return the edit as a QPropertyUndoCommand, or nullptr if unchanged.
@return the edit as one undo step, or nullptr if nothing changes.
Only the fields the user changed are written: the conductor keeps every
other value, including one the widget cannot show exactly.
When "apply to all" is ticked, every conductor on the same potential is
updated in the same undo step (one undo reverts them all), exactly as the
modal dialog does (ConductorPropertiesDialog::PropertiesDialog). Otherwise
only the selected conductor is changed.
updated too, in the same undo step (one undo reverts them all), as the
modal dialog does (ConductorPropertiesDialog::PropertiesDialog).
*/
QUndoCommand *ConductorPropertiesEditorWidget::associatedUndo() const
{
if (!m_conductor) return nullptr;
const ConductorProperties new_properties = m_cpw->properties();
if (new_properties == m_conductor->properties()) return nullptr;
if (new_properties == m_shown) return nullptr;
QVariant old_value, new_value;
old_value.setValue(m_conductor->properties());
new_value.setValue(new_properties);
auto *undo = new QPropertyUndoCommand(
m_conductor, "properties", old_value, new_value);
undo->setText(tr("Modifier les propriétés d'un conducteur", "undo caption"));
// Propagate to every conductor on the same potential, as the modal dialog
// does: each related conductor becomes a child command of the same undo
// step, set to the same target properties.
QList<Conductor *> targets {m_conductor};
if (m_apply_all_cb && m_apply_all_cb->isChecked())
for (Conductor *potential_conductor : m_conductor->relatedPotentialConductors())
if (!targets.contains(potential_conductor))
targets << potential_conductor;
auto *undo = new QUndoCommand();
int changed = 0;
for (Conductor *conductor : std::as_const(targets))
{
const auto potential = m_conductor->relatedPotentialConductors();
if (!potential.isEmpty())
{
undo->setText(tr("Modifier les propriétés de plusieurs conducteurs",
"undo caption"));
for (Conductor *potential_conductor : potential)
{
QVariant old_v;
old_v.setValue(potential_conductor->properties());
new QPropertyUndoCommand(
potential_conductor, "properties", old_v, new_value, undo);
}
}
const ConductorProperties old_properties = conductor->properties();
ConductorProperties properties = old_properties;
properties.applyChanges(m_shown, new_properties);
if (properties == old_properties) continue;
QVariant old_value, new_value;
old_value.setValue(old_properties);
new_value.setValue(properties);
new QPropertyUndoCommand(conductor, "properties", old_value, new_value, undo);
++changed;
}
if (!changed)
{
delete undo;
return nullptr;
}
undo->setText(changed == 1
? tr("Modifier les propriétés d'un conducteur", "undo caption")
: tr("Modifier les propriétés de plusieurs conducteurs", "undo caption"));
return undo;
}
+6 -1
View File
@@ -54,6 +54,9 @@ class ConductorPropertiesEditorWidget : public PropertiesEditorWidget
QString title() const override;
bool setLiveEdit(bool live_edit) override;
protected:
bool eventFilter(QObject *watched, QEvent *event) override;
private:
void connectChangeSignals();
void disconnectChangeSignals();
@@ -62,7 +65,9 @@ class ConductorPropertiesEditorWidget : public PropertiesEditorWidget
ConductorPropertiesWidget *m_cpw = nullptr;
QCheckBox *m_apply_all_cb = nullptr;
Conductor *m_conductor = nullptr;
ConductorProperties m_initial;
//What the widget showed before the edit: the fields that
//differ from it are the ones the user changed.
ConductorProperties m_shown;
QList<QMetaObject::Connection> m_live_connections;
bool m_updating = false;
};
+13
View File
@@ -510,6 +510,19 @@ target_compile_definitions(tst_terminaluuids PRIVATE
"QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\""
"QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"")
# ConductorProperties::applyChanges() -- the Selection properties panel
# writes only the fields the user edited.
add_executable(
tst_conductorapplychanges
tst_conductorapplychanges.cpp
${QET_DIR}/sources/conductorproperties.cpp
${QET_DIR}/sources/qet.cpp
${QET_DIR}/sources/qeticons.cpp
${QET_DIR}/sources/shortcutmanager.cpp)
add_test(NAME tst_conductorapplychanges COMMAND tst_conductorapplychanges)
target_include_directories(tst_conductorapplychanges PRIVATE ${QET_DIR}/sources)
target_link_libraries(tst_conductorapplychanges PRIVATE Qt::Test Qt::Widgets Qt::Xml pugixml::pugixml)
# DiagramContext::fromXml() -- the two readers (QDom for projects, pugixml
# for element definitions in the collection) give the same values: stray
# spaces trimmed, accents kept.
+167
View File
@@ -0,0 +1,167 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include <QtTest>
#include <functional>
#include "conductorproperties.h"
#include "qetapp.h"
// qet.cpp needs it; the application is not linked
QString QETApp::m_interface_language;
// ConductorProperties::applyChanges(): the Selection properties panel
// applies to each wire only the fields the user changed. Every field is
// checked on its own: change that one field, and a wire whose every field
// differs must take that field and keep all the others.
class tst_conductorapplychanges : public QObject
{
Q_OBJECT
using Setter = std::function<void(ConductorProperties &, int)>;
// One setter per field. Variant 1 always differs from 0 and 2, so
// a field that is not copied leaves the result visibly wrong;
// two-valued fields reuse variant 0's value for 2.
static QList<QPair<const char *, Setter>> fields()
{
return {
{"type", [](ConductorProperties &p, int v) {
p.type = v == 1 ? ConductorProperties::Single : ConductorProperties::Multi; }},
{"color", [](ConductorProperties &p, int v) {
p.color = QColor::fromRgb(10 + v, 0, 0); }},
{"bicolor", [](ConductorProperties &p, int v) {
p.m_bicolor = v == 1; }},
{"color_2", [](ConductorProperties &p, int v) {
p.m_color_2 = QColor::fromRgb(0, 10 + v, 0); }},
{"dash_size", [](ConductorProperties &p, int v) {
p.m_dash_size = 2 + v; }},
{"style", [](ConductorProperties &p, int v) {
p.style = Qt::PenStyle(Qt::SolidLine + v); }},
{"text", [](ConductorProperties &p, int v) {
p.text = QStringLiteral("text%1").arg(v); }},
{"text_color", [](ConductorProperties &p, int v) {
p.text_color = QColor::fromRgb(0, 0, 10 + v); }},
{"formula", [](ConductorProperties &p, int v) {
p.m_formula = QStringLiteral("formula%1").arg(v); }},
{"cable", [](ConductorProperties &p, int v) {
p.m_cable = QStringLiteral("cable%1").arg(v); }},
{"bus", [](ConductorProperties &p, int v) {
p.m_bus = QStringLiteral("bus%1").arg(v); }},
{"function", [](ConductorProperties &p, int v) {
p.m_function = QStringLiteral("function%1").arg(v); }},
{"tension_protocol", [](ConductorProperties &p, int v) {
p.m_tension_protocol = QStringLiteral("24V%1").arg(v); }},
{"wire_color", [](ConductorProperties &p, int v) {
p.m_wire_color = QStringLiteral("BU%1").arg(v); }},
{"wire_section", [](ConductorProperties &p, int v) {
p.m_wire_section = QStringLiteral("1.%1").arg(v); }},
{"show_text", [](ConductorProperties &p, int v) {
p.m_show_text = v == 1; }},
{"text_size", [](ConductorProperties &p, int v) {
p.text_size = 7 + v; }},
{"cond_size", [](ConductorProperties &p, int v) {
p.cond_size = 1.5 + v; }},
{"verti_rotate_text", [](ConductorProperties &p, int v) {
p.verti_rotate_text = 90.0 * v; }},
{"horiz_rotate_text", [](ConductorProperties &p, int v) {
p.horiz_rotate_text = 45.0 * v; }},
{"one_text_per_folio", [](ConductorProperties &p, int v) {
p.m_one_text_per_folio = v == 1; }},
{"horizontal_alignment", [](ConductorProperties &p, int v) {
p.m_horizontal_alignment = v == 0 ? Qt::AlignTop
: v == 1 ? Qt::AlignBottom : Qt::AlignVCenter; }},
{"vertical_alignment", [](ConductorProperties &p, int v) {
p.m_vertical_alignment = v == 0 ? Qt::AlignLeft
: v == 1 ? Qt::AlignRight : Qt::AlignHCenter; }},
{"single_line_ground", [](ConductorProperties &p, int v) {
p.singleLineProperties.hasGround = v == 1; }},
{"single_line_neutral", [](ConductorProperties &p, int v) {
p.singleLineProperties.hasNeutral = v == 1; }},
{"single_line_pen", [](ConductorProperties &p, int v) {
p.singleLineProperties.is_pen = v == 1; }},
{"single_line_phases", [](ConductorProperties &p, int v) {
p.singleLineProperties.setPhasesCount(v + 1); }},
};
}
// Every field set to the given variant
static ConductorProperties all(int variant)
{
ConductorProperties p;
for (const auto &f : fields())
f.second(p, variant);
return p;
}
private slots:
void eachFieldAlone_data()
{
QTest::addColumn<int>("index");
const auto list = fields();
for (int i = 0; i < list.size(); ++i)
QTest::newRow(list.at(i).first) << i;
}
// The shown wire is all variant 0, the user changes one field
// to variant 1, another selected wire is all variant 2: it
// takes that one field and keeps every other one.
void eachFieldAlone()
{
QFETCH(int, index);
const Setter set = fields().at(index).second;
const ConductorProperties before = all(0);
ConductorProperties after = before;
set(after, 1);
QVERIFY(after != before);
ConductorProperties other = all(2);
ConductorProperties expected = other;
set(expected, 1);
other.applyChanges(before, after);
QVERIFY(other == expected);
}
// Nothing edited: nothing copied, whatever the wire holds
void noEditChangesNothing()
{
const ConductorProperties shown = all(0);
ConductorProperties other = all(2);
const ConductorProperties kept = other;
other.applyChanges(shown, shown);
QVERIFY(other == kept);
}
// Every field edited: the wire ends up exactly as edited, so
// no field compared by operator== is left out of applyChanges
void everyFieldEdited()
{
ConductorProperties other = all(2);
other.applyChanges(all(0), all(1));
QVERIFY(other == all(1));
}
// Set the function on a wire with its own number and cable
void functionOnTwoWires()
{
ConductorProperties shown;
shown.text = QStringLiteral("101");
shown.m_cable = QStringLiteral("W1");
ConductorProperties edited = shown;
edited.m_function = QStringLiteral("24V DC");
ConductorProperties second;
second.text = QStringLiteral("102");
second.m_cable = QStringLiteral("W2");
second.m_function = QStringLiteral("old");
second.applyChanges(shown, edited);
QCOMPARE(second.m_function, QStringLiteral("24V DC"));
QCOMPARE(second.text, QStringLiteral("102"));
QCOMPARE(second.m_cable, QStringLiteral("W2"));
}
};
QTEST_GUILESS_MAIN(tst_conductorapplychanges)
#include "tst_conductorapplychanges.moc"