From 1136bc91f8be46b7bde0d37cab485233ca37a77a Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Wed, 23 Apr 2025 22:50:03 +0200 Subject: [PATCH] Gui: Fix value field for DlgAddPropertyVarSet (#20824) * Gui: Fix value field for DlgAddPropertyVarSet The value field would only appear if a name was provided resulting in a strange user experience. This commit fixes this and simplifies the logic of the dialog. * Process comments from review Co-authored-by: Kacper Donat --------- Co-authored-by: Kacper Donat --- src/Gui/Dialogs/DlgAddPropertyVarSet.cpp | 709 +++++++++++------------ src/Gui/Dialogs/DlgAddPropertyVarSet.h | 78 +-- 2 files changed, 372 insertions(+), 415 deletions(-) diff --git a/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp b/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp index f78a457247..525dd99804 100644 --- a/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp +++ b/src/Gui/Dialogs/DlgAddPropertyVarSet.cpp @@ -1,5 +1,6 @@ /**************************************************************************** * Copyright (c) 2024 Ondsel * + * Copyright (c) 2025 Pieter Hijma * * * * This file is part of the FreeCAD CAx development system. * * * @@ -25,6 +26,7 @@ # include # include # include +# include #endif #include @@ -32,11 +34,12 @@ #include #include #include +#include +#include #include #include "Dialogs/DlgAddPropertyVarSet.h" #include "ui_DlgAddPropertyVarSet.h" -#include "MainWindow.h" #include "ViewProviderVarSet.h" FC_LOG_LEVEL_INIT("DlgAddPropertyVarSet", true, true) @@ -44,11 +47,7 @@ FC_LOG_LEVEL_INIT("DlgAddPropertyVarSet", true, true) using namespace Gui; using namespace Gui::Dialog; -const std::string DlgAddPropertyVarSet::GROUP_BASE = "Base"; - -const bool CLEAR_NAME = true; -const bool ABORT = true; -const bool COMMIT = false; +const std::string DlgAddPropertyVarSet::GroupBase = "Base"; DlgAddPropertyVarSet::DlgAddPropertyVarSet(QWidget* parent, ViewProviderVarSet* viewProvider) @@ -67,33 +66,67 @@ DlgAddPropertyVarSet::DlgAddPropertyVarSet(QWidget* parent, DlgAddPropertyVarSet::~DlgAddPropertyVarSet() = default; +int DlgAddPropertyVarSet::findLabelRow(const char* labelName, QFormLayout* layout) +{ + for (int row = 0; row < layout->rowCount(); ++row) { + QLayoutItem* labelItem = layout->itemAt(row, QFormLayout::LabelRole); + if (labelItem == nullptr) { + continue; + } + + if (auto label = qobject_cast(labelItem->widget())) { + if (label->objectName() == QString::fromLatin1(labelName)) { + return row; + } + } + } + return -1; +} + +void DlgAddPropertyVarSet::setWidgetForLabel(const char* labelName, QWidget* widget) +{ + auto formLayout = qobject_cast(layout()); + if (formLayout == nullptr) { + FC_ERR("Form layout not found"); + return; + } + + int labelRow = findLabelRow(labelName, formLayout); + if (labelRow < 0) { + FC_ERR("Couldn't find row for '" << labelName << "'"); + return; + } + + formLayout->setWidget(labelRow, QFormLayout::FieldRole, widget); +} + void DlgAddPropertyVarSet::initializeGroup() { comboBoxGroup.setObjectName(QStringLiteral("comboBoxGroup")); comboBoxGroup.setInsertPolicy(QComboBox::InsertAtTop); comboBoxGroup.setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); - auto formLayout = qobject_cast(layout()); - formLayout->setWidget(1, QFormLayout::FieldRole, &comboBoxGroup); + + setWidgetForLabel("labelGroup", &comboBoxGroup); std::vector properties; varSet->getPropertyList(properties); + std::unordered_set groupNames; - for (auto prop : properties) { + for (const auto* prop : properties) { const char* groupName = varSet->getPropertyGroup(prop); - groupNames.insert(groupName ? groupName : GROUP_BASE); + groupNames.insert(groupName ? groupName : GroupBase); } + std::vector groupNamesSorted(groupNames.begin(), groupNames.end()); - std::sort(groupNamesSorted.begin(), groupNamesSorted.end(), [](std::string& a, std::string& b) { + std::ranges::sort(groupNamesSorted, [](const std::string& a, const std::string& b) { // prefer anything else other than Base, so move it to the back - if (a == GROUP_BASE) { + if (a == GroupBase) { return false; } - else if (b == GROUP_BASE) { + if (b == GroupBase) { return true; } - else { - return a < b; - } + return a < b; }); for (const auto& groupName : groupNamesSorted) { @@ -101,20 +134,26 @@ void DlgAddPropertyVarSet::initializeGroup() } comboBoxGroup.setEditText(QString::fromStdString(groupNamesSorted[0])); - connComboBoxGroup = connect(&comboBoxGroup, &EditFinishedComboBox::editFinished, - this, &DlgAddPropertyVarSet::onEditFinished); + connComboBoxGroup = connect(&comboBoxGroup, &EditFinishedComboBox::currentTextChanged, + this, &DlgAddPropertyVarSet::onTextFieldChanged); } -void DlgAddPropertyVarSet::getSupportedTypes(std::vector& types) +std::vector DlgAddPropertyVarSet::getSupportedTypes() { - std::vector proptypes; - Base::Type::getAllDerivedFrom(Base::Type::fromName("App::Property"), proptypes); - std::copy_if(proptypes.begin(), proptypes.end(), std::back_inserter(types), [](const Base::Type& type) { - return type.canInstantiate(); - }); - std::sort(types.begin(), types.end(), [](Base::Type a, Base::Type b) { + std::vector supportedTypes; + std::vector allTypes; + Base::Type::getAllDerivedFrom(Base::Type::fromName("App::Property"), allTypes); + + std::ranges::copy_if(allTypes, std::back_inserter(supportedTypes), + [](const Base::Type& type) { + return type.canInstantiate(); + }); + + std::ranges::sort(supportedTypes, [](Base::Type a, Base::Type b) { return strcmp(a.getName(), b.getName()) < 0; }); + + return supportedTypes; } void DlgAddPropertyVarSet::initializeTypes() @@ -122,18 +161,18 @@ void DlgAddPropertyVarSet::initializeTypes() auto paramGroup = App::GetApplication().GetParameterGroupByPath( "User parameter:BaseApp/Preferences/PropertyView"); auto lastType = Base::Type::fromName( - paramGroup->GetASCII("NewPropertyType","App::PropertyLength").c_str()); - if(lastType.isBad()) { + paramGroup->GetASCII("NewPropertyType", "App::PropertyLength").c_str()); + if (lastType.isBad()) { lastType = App::PropertyLength::getClassTypeId(); } - std::vector types; - getSupportedTypes(types); + std::vector types = getSupportedTypes(); for(const auto& type : types) { ui->comboBoxType->addItem(QString::fromLatin1(type.getName())); - if(type == lastType) + if (type == lastType) { ui->comboBoxType->setCurrentIndex(ui->comboBoxType->count()-1); + } } completerType.setModel(ui->comboBoxType->model()); @@ -143,106 +182,7 @@ void DlgAddPropertyVarSet::initializeTypes() ui->comboBoxType->setInsertPolicy(QComboBox::NoInsert); connComboBoxType = connect(ui->comboBoxType, &QComboBox::currentTextChanged, - this, &DlgAddPropertyVarSet::onEditFinished); -} - -/* -// keep some debugging code for debugging tab order -static void printFocusChain(QWidget *widget) { - FC_ERR("Focus Chain:"); - QWidget* start = widget; - int i = 0; - do { - FC_ERR(" " << widget->objectName().toStdString()); - widget = widget->nextInFocusChain(); - i++; - } while (widget != nullptr && i < 30 && start != widget); - QWidget *currentWidget = QApplication::focusWidget(); - FC_ERR(" Current focus widget:" << (currentWidget ? currentWidget->objectName().toStdString() : "None") << std::endl << std::endl); -} -*/ - -void DlgAddPropertyVarSet::initializeWidgets(ViewProviderVarSet* viewProvider) -{ - initializeGroup(); - initializeTypes(); - - connect(this, &QDialog::finished, - this, [viewProvider](int result) { viewProvider->onFinished(result); }); - connLineEditNameEditFinished = connect(ui->lineEditName, &QLineEdit::editingFinished, - this, &DlgAddPropertyVarSet::onEditFinished); - connLineEditNameTextChanged = connect(ui->lineEditName, &QLineEdit::textChanged, - this, &DlgAddPropertyVarSet::onNamePropertyChanged); - - setTitle(); - setOkEnabled(false); - - ui->lineEditName->setFocus(); - - QWidget::setTabOrder(ui->lineEditName, &comboBoxGroup); - QWidget::setTabOrder(&comboBoxGroup, ui->comboBoxType); - - // FC_ERR("Initialize widgets"); - // printFocusChain(ui->lineEditName); -} - -void DlgAddPropertyVarSet::setTitle() -{ - setWindowTitle(QObject::tr("Add a property to %1").arg(QString::fromStdString(varSet->getFullName()))); -} - -void DlgAddPropertyVarSet::setOkEnabled(bool enabled) -{ - QPushButton *okButton = ui->buttonBox->button(QDialogButtonBox::Ok); - okButton->setEnabled(enabled); -} - -void DlgAddPropertyVarSet::clearEditors(bool clearName) -{ - if (clearName) { - bool beforeBlocked = ui->lineEditName->blockSignals(true); - ui->lineEditName->clear(); - ui->lineEditName->blockSignals(beforeBlocked); - } - removeEditor(); - ui->lineEditToolTip->clear(); - setOkEnabled(false); - namePropertyToAdd.clear(); -} - -void DlgAddPropertyVarSet::removeEditor() -{ - if (editor) { - layout()->removeWidget(editor.get()); - QWidget::setTabOrder(ui->comboBoxType, ui->checkBoxAdd); - editor = nullptr; - - // FC_ERR("remove editor"); - // printFocusChain(ui->comboBoxType); - } -} - -void DlgAddPropertyVarSet::changeEvent(QEvent* e) -{ - if (e->type() == QEvent::LanguageChange) { - ui->retranslateUi(this); - setTitle(); - } - QDialog::changeEvent(e); -} - -static PropertyEditor::PropertyItem *createPropertyItem(App::Property *prop) -{ - const char *editor = prop->getEditorName(); - if (!editor || !editor[0]) { - return nullptr; - } - auto item = static_cast( - PropertyEditor::PropertyItemFactory::instance().createPropertyItem(editor)); - if (!item) { - qWarning("No property item for type %s found\n", editor); - } - return item; + this, &DlgAddPropertyVarSet::onTypeChanged); } void DlgAddPropertyVarSet::removeSelectionEditor() @@ -256,109 +196,235 @@ void DlgAddPropertyVarSet::removeSelectionEditor() } } -void DlgAddPropertyVarSet::addEditor(PropertyEditor::PropertyItem* propertyItem, - [[maybe_unused]]std::string& type) +void DlgAddPropertyVarSet::addEditor(PropertyEditor::PropertyItem* propertyItem) { editor.reset(propertyItem->createEditor(this, [this]() { this->valueChanged(); }, PropertyEditor::FrameOption::WithFrame)); - editor->blockSignals(true); + if (editor == nullptr) { + return; + } + + QSignalBlocker blockSignals(editor.get()); + + // To set the data in the editor, we need to set the data in the + // propertyItem. The propertyItem needs to have a property set to make + // sure that we get a correct value and the unit. propertyItem->setEditorData( editor.get(), propertyItem->data(PropertyEditor::PropertyItem::ValueColumn, Qt::EditRole)); editor->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred); editor->setObjectName(QStringLiteral("editor")); - auto formLayout = qobject_cast(layout()); - formLayout->setWidget(3, QFormLayout::FieldRole, editor.get()); + + setWidgetForLabel("labelValue", editor.get()); QWidget::setTabOrder(ui->comboBoxType, editor.get()); QWidget::setTabOrder(editor.get(), ui->checkBoxAdd); removeSelectionEditor(); - editor->blockSignals(false); +} - // FC_ERR("add editor"); - // printFocusChain(editor.get()); +bool DlgAddPropertyVarSet::isTypeWithEditor(const Base::Type& type) +{ + static const std::initializer_list subTypesWithEditor = { + // These types and their subtypes have editors. + App::PropertyBool::getClassTypeId(), + App::PropertyColor::getClassTypeId(), + App::PropertyFileIncluded::getClassTypeId(), + App::PropertyFloat::getClassTypeId(), + App::PropertyInteger::getClassTypeId() + }; + + static const std::initializer_list typesWithEditor = { + // These types have editors but not necessarily their subtypes. + App::PropertyFile::getClassTypeId(), + App::PropertyFloatList::getClassTypeId(), + App::PropertyFont::getClassTypeId(), + App::PropertyIntegerList::getClassTypeId(), + App::PropertyMaterialList::getClassTypeId(), + App::PropertyPath::getClassTypeId(), + App::PropertyString::getClassTypeId(), + App::PropertyStringList::getClassTypeId(), + App::PropertyVectorList::getClassTypeId() + }; + + const auto isDerivedFromType = [&type](const Base::Type& t) { + return type.isDerivedFrom(t); + }; + + return std::ranges::find(typesWithEditor, type) != typesWithEditor.end() || + std::ranges::any_of(subTypesWithEditor, isDerivedFromType); } bool DlgAddPropertyVarSet::isTypeWithEditor(const std::string& type) { - return typesWithoutEditor.find(type) == typesWithoutEditor.end(); + Base::Type propType = + Base::Type::getTypeIfDerivedFrom(type.c_str(), App::Property::getClassTypeId(), true); + return isTypeWithEditor(propType); } -void DlgAddPropertyVarSet::createProperty() +static PropertyEditor::PropertyItem *createPropertyItem(App::Property *prop) +{ + const char *editor = prop->getEditorName(); + if (Base::Tools::isNullOrEmpty(editor)) { + return nullptr; + } + return PropertyEditor::PropertyItemFactory::instance().createPropertyItem(editor); +} + +void DlgAddPropertyVarSet::createEditorForType(const Base::Type& type) +{ + // Temporarily create a property for two reasons: + // - to acquire the editor name from the instance, and + // - to acquire an initial value from the instance possibly with the correct unit. + void* propInstance = type.createInstance(); + if (!propInstance) { + FC_THROWM(Base::RuntimeError, "Failed to create a property of type " << type.getName()); + } + + // When prop goes out of scope, it can be deleted because we obtained the + // propertyItem (if applicable) and we initialized the editor with the data + // from the property. + std::unique_ptr prop( + static_cast(propInstance), + [](App::Property* p) { delete p; }); + prop->setContainer(varSet); + + propertyItem.reset(createPropertyItem(prop.get())); + + if (propertyItem && isTypeWithEditor(type)) { + propertyItem->setPropertyData({prop.get()}); + addEditor(propertyItem.get()); + propertyItem->removeProperty(prop.get()); + } +} + +void DlgAddPropertyVarSet::initializeValue() +{ + std::string type = ui->comboBoxType->currentText().toStdString(); + + Base::Type propType = + Base::Type::getTypeIfDerivedFrom(type.c_str(), App::Property::getClassTypeId(), true); + if (propType.isBad()) { + FC_THROWM(Base::TypeError, "Invalid type " << type << " for property"); + } + + if (isTypeWithEditor(propType)) { + createEditorForType(propType); + } +} + +void DlgAddPropertyVarSet::setTitle() +{ + setWindowTitle(QObject::tr("Add a property to %1").arg(QString::fromStdString(varSet->getFullName()))); +} + +void DlgAddPropertyVarSet::setOkEnabled(bool enabled) +{ + QPushButton *okButton = ui->buttonBox->button(QDialogButtonBox::Ok); + okButton->setEnabled(enabled); +} + +void DlgAddPropertyVarSet::initializeWidgets(ViewProviderVarSet* viewProvider) +{ + initializeGroup(); + initializeTypes(); + initializeValue(); + + connect(this, &QDialog::finished, + this, [viewProvider](int result) { viewProvider->onFinished(result); }); + connLineEditNameTextChanged = connect(ui->lineEditName, &QLineEdit::textChanged, + this, &DlgAddPropertyVarSet::onTextFieldChanged); + + setTitle(); + setOkEnabled(false); + + ui->lineEditName->setFocus(); + + QWidget::setTabOrder(ui->lineEditName, &comboBoxGroup); + QWidget::setTabOrder(&comboBoxGroup, ui->comboBoxType); +} + +bool DlgAddPropertyVarSet::propertyExists(const std::string& name) +{ + App::Property* prop = varSet->getPropertyByName(name.c_str()); + return prop && prop->getContainer() == varSet; +} + +bool DlgAddPropertyVarSet::isNameValid() { std::string name = ui->lineEditName->text().toStdString(); - std::string group = comboBoxGroup.currentText().toStdString(); - std::string type = ui->comboBoxType->currentText().toStdString(); - std::string doc = ui->lineEditToolTip->text().toStdString(); - App::Property* prop; - try { - prop = varSet->addDynamicProperty(type.c_str(), name.c_str(), - group.c_str(), doc.c_str()); - } - catch (Base::Exception& e) { - e.ReportException(); - critical(QObject::tr("Add property"), - QObject::tr("Failed to add property to '%1': %2").arg( - QString::fromLatin1(varSet->getFullName().c_str()), - QString::fromUtf8(e.what()))); - clearEditors(); + return !name.empty() && + name == Base::Tools::getIdentifier(name) && + !App::ExpressionParser::isTokenAConstant(name) && + !App::ExpressionParser::isTokenAUnit(name) && + !propertyExists(name); +} + +bool DlgAddPropertyVarSet::isGroupValid() +{ + std::string group = comboBoxGroup.currentText().toStdString(); + return !group.empty() && group == Base::Tools::getIdentifier(group); +} + +bool DlgAddPropertyVarSet::isTypeValid() +{ + std::string type = ui->comboBoxType->currentText().toStdString(); + + return !Base::Type::fromName(type.c_str()).isBad(); +} + +bool DlgAddPropertyVarSet::areFieldsValid() +{ + return isNameValid() && isGroupValid() && isTypeValid(); +} + +void DlgAddPropertyVarSet::onTextFieldChanged([[maybe_unused]] const QString& text) +{ + setOkEnabled(areFieldsValid()); +} + +void DlgAddPropertyVarSet::removeEditor() +{ + if (editor == nullptr) { return; } - namePropertyToAdd = name; - - objectIdentifier = std::make_unique(*prop); - // creating a propertyItem here because it has all kinds of logic for - // editors that we can reuse - removeEditor(); - propertyItem.reset(createPropertyItem(prop)); - if (propertyItem && isTypeWithEditor(type)) { - propertyItem->setPropertyData({prop}); - propertyItem->bind(*objectIdentifier); - addEditor(propertyItem.get(), type); - } - - setOkEnabled(true); + layout()->removeWidget(editor.get()); + QWidget::setTabOrder(ui->comboBoxType, ui->checkBoxAdd); + editor = nullptr; } -App::Property* DlgAddPropertyVarSet::getPropertyToAdd() { - // This function should be called only if it is certain the property exists. - // It will throw a runtime error if not. - App::Property* prop = varSet->getPropertyByName(namePropertyToAdd.c_str()); - if (prop == nullptr) { - FC_THROWM(Base::RuntimeError, "A property with name '" << namePropertyToAdd << "' does not exist."); +void DlgAddPropertyVarSet::onTypeChanged(const QString& text) +{ + std::string type = text.toStdString(); + + if (Base::Type::fromName(type.c_str()).isBad() || !isTypeWithEditor(type)) { + propertyItem = nullptr; + removeEditor(); + } + else { + initializeValue(); } - return prop; + setOkEnabled(areFieldsValid()); } -void DlgAddPropertyVarSet::changePropertyToAdd() { - // we were already adding a new property, the only option to get here - // is a change of type or group. - - std::string name = ui->lineEditName->text().toStdString(); - assert(name == namePropertyToAdd); - - // performs a check for nullptr - App::Property* prop = getPropertyToAdd(); - - std::string group = comboBoxGroup.currentText().toStdString(); - std::string doc = ui->lineEditToolTip->text().toStdString(); - if (prop->getGroup() != group) { - varSet->changeDynamicProperty(prop, group.c_str(), doc.c_str()); - } - - std::string type = ui->comboBoxType->currentText().toStdString(); - if (prop->getTypeId() != Base::Type::fromName(type.c_str())) { - // the property should have a different type - varSet->removeDynamicProperty(namePropertyToAdd.c_str()); - createProperty(); +void DlgAddPropertyVarSet::changeEvent(QEvent* e) +{ + if (e->type() == QEvent::LanguageChange) { + ui->retranslateUi(this); + setTitle(); } + QDialog::changeEvent(e); } +void DlgAddPropertyVarSet::valueChanged() +{ + QVariant data = propertyItem->editorData(editor.get()); + propertyItem->setData(data); +} /* We use these functions rather than the functions provided by App::Document * because this dialog may be opened when another transaction is in progress. @@ -372,140 +438,6 @@ void DlgAddPropertyVarSet::openTransaction() transactionID = App::GetApplication().setActiveTransaction("Add property VarSet"); } - -bool DlgAddPropertyVarSet::hasPendingTransaction() -{ - return transactionID != 0; -} - - -void DlgAddPropertyVarSet::closeTransaction(bool abort) -{ - if (transactionID != 0) { - App::GetApplication().closeActiveTransaction(abort, transactionID); - transactionID = 0; - } -} - - -void DlgAddPropertyVarSet::clearCurrentProperty() -{ - removeEditor(); - varSet->removeDynamicProperty(namePropertyToAdd.c_str()); - if (hasPendingTransaction()) { - closeTransaction(ABORT); - } - setOkEnabled(false); - namePropertyToAdd.clear(); -} - -class CreatePropertyException : public std::exception { -public: - explicit CreatePropertyException(const std::string& message) : msg(message) {} - - const char* what() const noexcept override { - return msg.c_str(); - } - -private: - std::string msg; -}; - -void DlgAddPropertyVarSet::checkName() { - std::string name = ui->lineEditName->text().toStdString(); - if(name.empty() || name != Base::Tools::getIdentifier(name)) { - QMessageBox::critical(getMainWindow(), - QObject::tr("Invalid name"), - QObject::tr("The property name must only contain alpha numericals, " - "underscore, and must not start with a digit.")); - clearEditors(!CLEAR_NAME); - throw CreatePropertyException("Invalid name"); - } - - if(App::ExpressionParser::isTokenAUnit(name) || App::ExpressionParser::isTokenAConstant(name)) { - critical(QObject::tr("Invalid name"), - QObject::tr("The property name is a reserved word.")); - clearEditors(!CLEAR_NAME); - throw CreatePropertyException("Invalid name"); - } - - if (namePropertyToAdd.empty()) { - // we are adding a new property, check whether it doesn't already exist - auto prop = varSet->getPropertyByName(name.c_str()); - if(prop && prop->getContainer() == varSet) { - critical(QObject::tr("Invalid name"), - QObject::tr("The property '%1' already exists in '%2'").arg( - QString::fromLatin1(name.c_str()), - QString::fromLatin1(varSet->getFullName().c_str()))); - clearEditors(!CLEAR_NAME); - throw CreatePropertyException("Invalid name"); - } - } -} - -void DlgAddPropertyVarSet::checkGroup() { - std::string group = comboBoxGroup.currentText().toStdString(); - - if (group.empty() || group != Base::Tools::getIdentifier(group)) { - critical(QObject::tr("Invalid name"), - QObject::tr("The group name must only contain alpha numericals,\n" - "underscore, and must not start with a digit.")); - comboBoxGroup.setEditText(QStringLiteral("Base")); - throw CreatePropertyException("Invalid name"); - } -} - -void DlgAddPropertyVarSet::checkType() { - std::string type = ui->comboBoxType->currentText().toStdString(); - - if (Base::Type::fromName(type.c_str()).isBad()) { - throw CreatePropertyException("Invalid name"); - } -} - -void DlgAddPropertyVarSet::onEditFinished() { - /* The editor for the value is dynamically created if 1) the name has been - * determined and 2) if the type of the property has been determined. The - * group of the property is important too, but it is not essential, because - * we can change the group after the property has been created. - * - * In this function we check whether we can create a property and therefore - * an editor. - */ - - try { - checkName(); - checkGroup(); - checkType(); - // no check for tooltip, we accept any string - } - catch (const CreatePropertyException&) { - if (!namePropertyToAdd.empty()) { - clearCurrentProperty(); - } - return; - } - - if (namePropertyToAdd.empty()) { - // we are adding a new property - openTransaction(); - createProperty(); - } - else { - // we were already adding a new property that should now be changed - changePropertyToAdd(); - } -} - -void DlgAddPropertyVarSet::onNamePropertyChanged(const QString& text) -{ - if (!namePropertyToAdd.empty() && text.toStdString() != namePropertyToAdd) { - // The user decided to change the name of the property. This - // invalidates the editor that is strictly associated with the property. - clearCurrentProperty(); - } -} - void DlgAddPropertyVarSet::critical(const QString& title, const QString& text) { static bool criticalDialogShown = false; if (!criticalDialogShown) { @@ -515,72 +447,93 @@ void DlgAddPropertyVarSet::critical(const QString& title, const QString& text) { } } -void DlgAddPropertyVarSet::valueChanged() +bool DlgAddPropertyVarSet::createProperty() { - QVariant data; - data = propertyItem->editorData(editor.get()); - propertyItem->setData(data); -} - -void DlgAddPropertyVarSet::addDocumentation() { - /* Add the documentation to an existing property. - * Note that this method assumes the property exists. - * - * Since there is no check on documentation (we accept any string), there - * is no signal handler for the documentation field. This method updates - * the property that is being added with the text inserted as - * documentation/tooltip. - * - * This function should be called at a late stage, before doing the accept. - */ - + std::string name = ui->lineEditName->text().toStdString(); std::string group = comboBoxGroup.currentText().toStdString(); + std::string type = ui->comboBoxType->currentText().toStdString(); std::string doc = ui->lineEditToolTip->text().toStdString(); - // performs a check for nullptr - App::Property* prop = getPropertyToAdd(); - varSet->changeDynamicProperty(prop, group.c_str(), doc.c_str()); + App::Property* prop = nullptr; + try { + prop = varSet->addDynamicProperty(type.c_str(), name.c_str(), + group.c_str(), doc.c_str()); + } + catch (Base::Exception& e) { + e.ReportException(); + critical(QObject::tr("Add property"), + QObject::tr("Failed to add property to '%1': %2").arg( + QString::fromLatin1(varSet->getFullName().c_str()), + QString::fromUtf8(e.what()))); + return false; + } + + if (propertyItem) { + objectIdentifier = std::make_unique(*prop); + propertyItem->setPropertyData({prop}); + propertyItem->bind(*objectIdentifier); + + QVariant data = propertyItem->editorData(editor.get()); + propertyItem->setData(data); + } + + return true; +} + +void DlgAddPropertyVarSet::closeTransaction(TransactionOption option) +{ + if (transactionID == 0) { + return; + } + + App::GetApplication().closeActiveTransaction(static_cast(option), transactionID); + transactionID = 0; +} + +void DlgAddPropertyVarSet::clearFields() +{ + { + QSignalBlocker blocker(ui->lineEditName); + ui->lineEditName->clear(); + } + ui->lineEditToolTip->clear(); + initializeValue(); + setOkEnabled(false); } void DlgAddPropertyVarSet::accept() { - addDocumentation(); - closeTransaction(COMMIT); + openTransaction(); + if (createProperty()) { + closeTransaction(TransactionOption::Commit); + std::string group = comboBoxGroup.currentText().toStdString(); + std::string type = ui->comboBoxType->currentText().toStdString(); + auto paramGroup = App::GetApplication().GetParameterGroupByPath( + "User parameter:BaseApp/Preferences/PropertyView"); + paramGroup->SetASCII("NewPropertyType", type.c_str()); + paramGroup->SetASCII("NewPropertyGroup", group.c_str()); - if (ui->checkBoxAdd->isChecked()) { - clearEditors(); - openTransaction(); - ui->lineEditName->setFocus(); - return; + if (ui->checkBoxAdd->isChecked()) { + clearFields(); + ui->lineEditName->setFocus(); + } + else { + // we are done, close the dialog + QDialog::accept(); + } + } + else { + closeTransaction(TransactionOption::Abort); } - - std::string group = comboBoxGroup.currentText().toStdString(); - std::string type = ui->comboBoxType->currentText().toStdString(); - auto paramGroup = App::GetApplication().GetParameterGroupByPath( - "User parameter:BaseApp/Preferences/PropertyView"); - paramGroup->SetASCII("NewPropertyType", type.c_str()); - paramGroup->SetASCII("NewPropertyGroup", group.c_str()); - QDialog::accept(); } void DlgAddPropertyVarSet::reject() { - // On reject we can disconnect the signal handlers because nothing useful - // is to be done. Otherwise, signals may activate the handlers that assume - // that a new property has been created, an assumption that will be - // violated by aborting the transaction because it will remove the newly - // created property. disconnect(connComboBoxGroup); disconnect(connComboBoxType); - disconnect(connLineEditNameEditFinished); disconnect(connLineEditNameTextChanged); - // a transaction is not pending if a name has not been determined. - if (hasPendingTransaction()) { - closeTransaction(ABORT); - } QDialog::reject(); } - #include "moc_DlgAddPropertyVarSet.cpp" diff --git a/src/Gui/Dialogs/DlgAddPropertyVarSet.h b/src/Gui/Dialogs/DlgAddPropertyVarSet.h index 3c797db686..d0b539ae24 100644 --- a/src/Gui/Dialogs/DlgAddPropertyVarSet.h +++ b/src/Gui/Dialogs/DlgAddPropertyVarSet.h @@ -1,5 +1,6 @@ /**************************************************************************** * Copyright (c) 2024 Ondsel * + * Copyright (c) 2025 Pieter Hijma * * * * This file is part of the FreeCAD CAx development system. * * * @@ -24,9 +25,11 @@ #define GUI_DIALOG_DLG_ADD_PROPERTY_VARSET_H #include -#include + #include #include +#include + #include #include @@ -68,10 +71,16 @@ class GuiExport DlgAddPropertyVarSet : public QDialog Q_OBJECT public: - static const std::string GROUP_BASE; + static const std::string GroupBase; public: - DlgAddPropertyVarSet(QWidget *parent, ViewProviderVarSet* viewProvider); + DlgAddPropertyVarSet(QWidget* parent, ViewProviderVarSet* viewProvider); + + DlgAddPropertyVarSet(const DlgAddPropertyVarSet&) = delete; + DlgAddPropertyVarSet(DlgAddPropertyVarSet&&) = delete; + DlgAddPropertyVarSet& operator=(const DlgAddPropertyVarSet&) = delete; + DlgAddPropertyVarSet& operator=(DlgAddPropertyVarSet&&) = delete; + ~DlgAddPropertyVarSet() override; void changeEvent(QEvent* e) override; @@ -82,67 +91,62 @@ public Q_SLOTS: void valueChanged(); private: + enum class TransactionOption : bool { + Commit = false, + Abort = true + }; + + int findLabelRow(const char* labelName, QFormLayout* layout); + void setWidgetForLabel(const char* labelName, QWidget* widget); void initializeGroup(); + + std::vector getSupportedTypes(); void initializeTypes(); - void initializeWidgets(ViewProviderVarSet* viewProvider); + + void removeSelectionEditor(); + void addEditor(PropertyEditor::PropertyItem* propertyItem); + bool isTypeWithEditor(const Base::Type& type); + bool isTypeWithEditor(const std::string& type); + void createEditorForType(const Base::Type& type); + void initializeValue(); void setTitle(); void setOkEnabled(bool enabled); - void clearEditors(bool clearName = true); - void clearCurrentProperty(); + void initializeWidgets(ViewProviderVarSet* viewProvider); + + bool propertyExists(const std::string& name); + bool isNameValid(); + bool isGroupValid(); + bool isTypeValid(); + bool areFieldsValid(); + + void onTextFieldChanged(const QString& text); void removeEditor(); - void removeSelectionEditor(); - void addEditor(PropertyEditor::PropertyItem* propertyItem, std::string& type); - - bool isTypeWithEditor(const std::string& type); - void createProperty(); - void changePropertyToAdd(); + void onTypeChanged(const QString& text); void openTransaction(); - bool hasPendingTransaction(); - void abortTransaction(); - void closeTransaction(bool abort); - - void checkName(); - void checkGroup(); - void checkType(); - void onEditFinished(); - void onNamePropertyChanged(const QString& text); void critical(const QString& title, const QString& text); - - void getSupportedTypes(std::vector& types); - App::Property* getPropertyToAdd(); - void addDocumentation(); + bool createProperty(); + void closeTransaction(TransactionOption option); + void clearFields(); private: - std::unordered_set typesWithoutEditor = { - "App::PropertyVector", "App::PropertyVectorDistance", "App::PropertyMatrix", - "App::PropertyRotation", "App::PropertyPlacement", "App::PropertyEnumeration", - "App::PropertyDirection", "App::PropertyPlacementList", "App::PropertyPosition", - "App::PropertyExpressionEngine", "App::PropertyIntegerSet", - "Sketcher::PropertyConstraintList"}; - App::VarSet* varSet; std::unique_ptr ui; EditFinishedComboBox comboBoxGroup; QCompleter completerType; - // state between adding properties std::unique_ptr editor; - std::unique_ptr expressionEditor; - std::string namePropertyToAdd; std::unique_ptr propertyItem; std::unique_ptr objectIdentifier; // a transactionID of 0 means that there is no active transaction. int transactionID; - // connections QMetaObject::Connection connComboBoxGroup; QMetaObject::Connection connComboBoxType; - QMetaObject::Connection connLineEditNameEditFinished; QMetaObject::Connection connLineEditNameTextChanged; };