From 635c49bb3b234399e008435a52f9e48c926c4b71 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Mon, 19 Oct 2026 09:40:46 +0200 Subject: [PATCH] #12925: Improve caf editors, label links and nested settings write --- .../cafPdmUiCore/cafPdmUiLabel.cpp | 16 +++ .../cafPdmUiCore/cafPdmUiLabel.h | 9 ++ .../cafPdmXml/cafPdmSettings.cpp | 11 +-- .../cafPdmXml_UnitTests/CMakeLists.txt | 8 +- .../cafPdmSettingsTest.cpp | 94 ++++++++++++++++++ .../cafPdmUiFormLayoutObjectEditor.cpp | 15 +++ .../cafUserInterface/cafPdmUiListEditor.cpp | 32 +++++- .../cafUserInterface/cafPdmUiListEditor.h | 4 +- .../cafUserInterface_UnitTests/CMakeLists.txt | 1 + .../cafPdmUiFormLayoutObjectEditorTest.cpp | 97 +++++++++++++++++++ 10 files changed, 276 insertions(+), 11 deletions(-) create mode 100644 Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/cafPdmSettingsTest.cpp create mode 100644 Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.cpp index 352a12e91a..59458ed17b 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.cpp @@ -62,4 +62,20 @@ bool PdmUiLabel::isUiGroup() const return false; } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void PdmUiLabel::setLinkActivatedCallback( const std::function& callback ) +{ + m_linkActivatedCallback = callback; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +const std::function& PdmUiLabel::linkActivatedCallback() const +{ + return m_linkActivatedCallback; +} + } // End of namespace caf diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.h b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.h index 2969ef1522..b9db90d6a1 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.h +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.h @@ -38,6 +38,8 @@ #include "cafPdmUiItem.h" +#include + namespace caf { //================================================================================================== @@ -51,6 +53,13 @@ public: explicit PdmUiLabel( const QString& labelText ); bool isUiGroup() const override; + + // Called with the href when a hyperlink in a rich text label is clicked + void setLinkActivatedCallback( const std::function& callback ); + const std::function& linkActivatedCallback() const; + +private: + std::function m_linkActivatedCallback; }; } // End of namespace caf diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmSettings.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmSettings.cpp index 6f66ab2a91..60a70b22fd 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmSettings.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmSettings.cpp @@ -117,14 +117,11 @@ void PdmSettings::writeFieldsToApplicationStore( const caf::PdmObjectHandle* obj caf::PdmObjectHandle* child = children[childIdx]; if ( !child ) continue; - QString subContext; - if ( context.isEmpty() ) - { - caf::PdmXmlObjectHandle* xmlObjHandle = xmlObj( child ); - if ( !xmlObjHandle ) continue; + caf::PdmXmlObjectHandle* xmlObjHandle = xmlObj( child ); + if ( !xmlObjHandle ) continue; - subContext = xmlObjHandle->classKeyword() + "/"; - } + // Use the same nested context as when reading, to support children of children + const QString subContext = context + xmlObjHandle->classKeyword() + "/"; writeFieldsToApplicationStore( child, subContext ); } diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/CMakeLists.txt b/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/CMakeLists.txt index 43a04c2f03..6d30ee54e9 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/CMakeLists.txt +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/CMakeLists.txt @@ -20,9 +20,13 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR} # required for gtest-all.cpp ) set(PROJECT_FILES - cafPdmXml_UnitTests.cpp gtest/gtest-all.cpp cafPdmXmlBasicTest.cpp - cafPdmAdvancedTemplateTest.cpp cafPdmXmlNumberTest.cpp + cafPdmXml_UnitTests.cpp + gtest/gtest-all.cpp + cafPdmXmlBasicTest.cpp + cafPdmAdvancedTemplateTest.cpp + cafPdmXmlNumberTest.cpp cafPdmPtrArrayTest.cpp + cafPdmSettingsTest.cpp ) qt_add_executable(${PROJECT_NAME} MACOSX_BUNDLE ${PROJECT_FILES}) diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/cafPdmSettingsTest.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/cafPdmSettingsTest.cpp new file mode 100644 index 0000000000..5fa1227c40 --- /dev/null +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/cafPdmSettingsTest.cpp @@ -0,0 +1,94 @@ +#include "gtest/gtest.h" + +#include "cafPdmChildField.h" +#include "cafPdmDataValueField.h" +#include "cafPdmObjectHandle.h" +#include "cafPdmSettings.h" +#include "cafPdmXmlObjectHandle.h" +#include "cafPdmXmlObjectHandleMacros.h" + +#include +#include +#include + +class SettingsLeaf : public caf::PdmObjectHandle, public caf::PdmXmlObjectHandle +{ + CAF_PDM_XML_HEADER_INIT; + +public: + SettingsLeaf() + : PdmObjectHandle() + , PdmXmlObjectHandle( this, false ) + { + CAF_PDM_XML_InitField( &m_value, "Value" ); + } + + caf::PdmDataValueField m_value; +}; +CAF_PDM_XML_SOURCE_INIT( SettingsLeaf, "SettingsLeaf" ); + +class SettingsMiddle : public caf::PdmObjectHandle, public caf::PdmXmlObjectHandle +{ + CAF_PDM_XML_HEADER_INIT; + +public: + SettingsMiddle() + : PdmObjectHandle() + , PdmXmlObjectHandle( this, false ) + { + CAF_PDM_XML_InitField( &m_leaf, "Leaf" ); + m_leaf = new SettingsLeaf; + } + + ~SettingsMiddle() { delete m_leaf(); } + + caf::PdmChildField m_leaf; +}; +CAF_PDM_XML_SOURCE_INIT( SettingsMiddle, "SettingsMiddle" ); + +class SettingsRoot : public caf::PdmObjectHandle, public caf::PdmXmlObjectHandle +{ + CAF_PDM_XML_HEADER_INIT; + +public: + SettingsRoot() + : PdmObjectHandle() + , PdmXmlObjectHandle( this, false ) + { + CAF_PDM_XML_InitField( &m_middle, "Middle" ); + m_middle = new SettingsMiddle; + } + + ~SettingsRoot() { delete m_middle(); } + + caf::PdmChildField m_middle; +}; +CAF_PDM_XML_SOURCE_INIT( SettingsRoot, "SettingsRoot" ); + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( PdmSettings, NestedChildrenRoundTrip ) +{ + QSettings::setDefaultFormat( QSettings::IniFormat ); + QSettings::setPath( QSettings::IniFormat, QSettings::UserScope, QDir::tempPath() ); + QCoreApplication::setOrganizationName( "cafPdmSettingsTest" ); + QCoreApplication::setApplicationName( "cafPdmSettingsTest" ); + QSettings().clear(); + + { + SettingsRoot root; + root.m_middle()->m_leaf()->m_value = 42; + caf::PdmSettings::writeFieldsToApplicationStore( &root ); + } + + // Written with the same nested context as used when reading + EXPECT_TRUE( QSettings().contains( "SettingsMiddle/SettingsLeaf/Value" ) ); + + SettingsRoot restored; + restored.m_middle()->m_leaf()->m_value = 0; + caf::PdmSettings::readFieldsFromApplicationStore( &restored ); + EXPECT_EQ( 42, restored.m_middle()->m_leaf()->m_value() ); + + QSettings().clear(); +} diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp index 91b27243ee..5f12f642c0 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp @@ -488,6 +488,13 @@ QLabel* caf::PdmUiFormLayoutObjectEditor::createLabel( QWidget* parent, const Pd qLabel->setText( label.uiName( uiConfigName ) ); qLabel->setWordWrap( true ); + if ( auto callback = label.linkActivatedCallback() ) + { + qLabel->setTextFormat( Qt::RichText ); + qLabel->setTextInteractionFlags( Qt::LinksAccessibleByMouse ); + QObject::connect( qLabel, &QLabel::linkActivated, qLabel, [callback]( const QString& link ) { callback( link ); } ); + } + m_labels.push_back( qLabel ); return qLabel; } @@ -562,6 +569,14 @@ caf::PdmUiFieldEditorHandle* caf::PdmUiFormLayoutObjectEditor::findOrCreateField std::map::iterator it = m_fieldViews.find( field->fieldHandle() ); + // The editor's field was deleted and a new field was allocated at the same address + if ( it != m_fieldViews.end() && it->second->uiField() != field ) + { + delete it->second; + m_fieldViews.erase( it ); + it = m_fieldViews.end(); + } + if ( it == m_fieldViews.end() ) { fieldEditor = PdmUiFieldEditorHelper::createFieldEditorForField( field, uiConfigName ); diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.cpp index d9bc03cc8c..0cdebb289d 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.cpp @@ -76,8 +76,24 @@ public: void setItemsEditable( bool isEditable ) { m_isItemsEditable = isEditable; } + void setIcons( const QList& icons ) { m_icons = icons; } + + QVariant data( const QModelIndex& index, int role ) const override + { + if ( role == Qt::DecorationRole ) + { + if ( index.isValid() && index.row() < m_icons.size() && !m_icons[index.row()].isNull() ) + { + return m_icons[index.row()]; + } + return QVariant(); + } + return QStringListModel::data( index, role ); + } + private: - bool m_isItemsEditable; + bool m_isItemsEditable; + QList m_icons; }; //================================================================================================== @@ -194,6 +210,19 @@ void PdmUiListEditor::configureAndUpdateUi( const QString& uiConfigName ) strListModel->setItemsEditable( false ); QModelIndex currentItem = m_listView->selectionModel()->currentIndex(); QStringList texts = PdmOptionItemInfo::extractUiTexts( options ); + + QList icons; + for ( const auto& option : options ) + { + auto icon = option.icon(); + icons.push_back( icon ? *icon : QIcon() ); + } + strListModel->setIcons( icons ); + + if ( auto iconSize = uiField()->attribute( Keys::ICON_SIZE, uiConfigName ) ) + { + m_listView->setIconSize( *iconSize ); + } strListModel->setStringList( texts ); QVariant fieldValue = uiField()->uiValue(); @@ -247,6 +276,7 @@ void PdmUiListEditor::configureAndUpdateUi( const QString& uiConfigName ) QVariant fieldValue = uiField()->uiValue(); QStringList texts = fieldValue.toStringList(); texts.push_back( "" ); + strListModel->setIcons( {} ); strListModel->setStringList( texts ); strListModel->setItemsEditable( true ); diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.h b/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.h index 1f1e487516..d2248fc09e 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.h +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.h @@ -66,13 +66,15 @@ public: static inline const QString BASE_COLOR = QStringLiteral( "baseColor" ); static inline const QString QSS_STATE = QStringLiteral( "qssState" ); static inline const QString ALLOW_HORIZONTAL_SCROLL_BAR = QStringLiteral( "allowHorizontalScrollBar" ); + static inline const QString ICON_SIZE = QStringLiteral( "iconSize" ); // QSize }; // Set of all supported attributes for validation inline static const std::set SUPPORTED_ATTRIBUTES = { Keys::HEIGHT_HINT, Keys::BASE_COLOR, Keys::QSS_STATE, - Keys::ALLOW_HORIZONTAL_SCROLL_BAR }; + Keys::ALLOW_HORIZONTAL_SCROLL_BAR, + Keys::ICON_SIZE }; protected: QWidget* createEditorWidget( QWidget* parent ) override; diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt index 0036550edd..061d1497c5 100644 --- a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt @@ -25,6 +25,7 @@ set(PROJECT_FILES cafPdmUiCheckBoxAndTextEditorTest.cpp cafPdmUiPropertyViewDialogTest.cpp cafPdmUiEditorHandleTest.cpp + cafPdmUiFormLayoutObjectEditorTest.cpp cafIconProviderTest.cpp cafProgressInfoTest.cpp gtest/gtest-all.cpp diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp new file mode 100644 index 0000000000..726c0d309b --- /dev/null +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp @@ -0,0 +1,97 @@ +#include "gtest/gtest.h" + +#include "cafPdmChildField.h" +#include "cafPdmField.h" +#include "cafPdmObject.h" +#include "cafPdmUiOrdering.h" +#include "cafPdmUiPropertyView.h" + +#include + +using namespace caf; + +namespace +{ +alignas( std::max_align_t ) unsigned char childStorage[4096]; +} // namespace + +//-------------------------------------------------------------------------------------------------- +/// Always allocated at the same address, to reproduce a deleted object being replaced by one at the same address +//-------------------------------------------------------------------------------------------------- +class FormLayoutTestChild : public caf::PdmObject +{ + CAF_PDM_HEADER_INIT; + +public: + FormLayoutTestChild() + { + CAF_PDM_InitObject( "FormLayoutTestChild" ); + CAF_PDM_InitField( &m_text, "Text", QString( "text" ), "Text" ); + } + + static void* operator new( size_t size ) + { + EXPECT_LE( size, sizeof( childStorage ) ); + return childStorage; + } + static void operator delete( void* ) {} + + caf::PdmField m_text; +}; +CAF_PDM_SOURCE_INIT( FormLayoutTestChild, "FormLayoutTestChild" ); + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +class FormLayoutTestParent : public caf::PdmObject +{ + CAF_PDM_HEADER_INIT; + +public: + FormLayoutTestParent() + { + CAF_PDM_InitObject( "FormLayoutTestParent" ); + CAF_PDM_InitFieldNoDefault( &m_child, "Child", "Child" ); + m_child = new FormLayoutTestChild; + } + + void replaceChild() + { + delete m_child(); + m_child = new FormLayoutTestChild; + } + + caf::PdmChildField m_child; + +protected: + void defineUiOrdering( QString uiConfigName, caf::PdmUiOrdering& uiOrdering ) override + { + uiOrdering.add( &m_child->m_text ); + uiOrdering.skipRemainingFields(); + } +}; +CAF_PDM_SOURCE_INIT( FormLayoutTestParent, "FormLayoutTestParent" ); + +//-------------------------------------------------------------------------------------------------- +/// A field editor bound to a deleted field must not be reused for a new field at the same address +//-------------------------------------------------------------------------------------------------- +TEST( PdmUiFormLayoutObjectEditorTest, FieldReplacedAtSameAddress ) +{ + FormLayoutTestParent parent; + + PdmUiPropertyView view; + view.showProperties( &parent ); + + auto oldField = &parent.m_child->m_text; + ASSERT_EQ( 1u, oldField->uiCapability()->connectedEditors().size() ); + + parent.replaceChild(); + ASSERT_EQ( oldField, &parent.m_child->m_text ); + ASSERT_TRUE( parent.m_child->m_text.uiCapability()->connectedEditors().empty() ); + + parent.updateConnectedEditors(); + + EXPECT_EQ( 1u, parent.m_child->m_text.uiCapability()->connectedEditors().size() ); + + view.showProperties( nullptr ); +}