Fix review comments: sync/persist/tab-counter/multi-adapter issues

- adapters/json-rpc-spec.md: add comma after "Otherwise" (grammar)
- schemaformwidget.cpp: derive initial conditional branch from the
  trigger combo's current value instead of the incoming JSON values,
  preventing visible/values() mismatch on first render
- adaptersettings.cpp/h: remove redundant setLayout() call; extract
  add-tab lambda into named addItemTab() method with monotonic
  _nextItemTabIndex counter to avoid duplicate tab titles after a tab
  is closed
- settingsdialog.cpp: iterate all adapters (not just the first) when
  building dynamic settings pages; only persist settings when dialog
  is accepted (gate acceptAllValues on r == QDialog::Accepted) and
  remove side-effectful acceptAllValues() call from settingsStackSwitch
- tst_schemaformwidget.cpp: use isHidden() instead of isVisible() so
  the conditional-hide assertion works even when the parent is not shown

https://claude.ai/code/session_013ytSJa46akaAHkox9rjeRa

Author: claude

adapters/json-rpc-spec.md

@@ -120,7 +120,7 @@ Each property in `schema` includes the following additional fields (standard JSO
 | `title` | Standard JSON Schema annotation. UI-friendly label for the field, suitable for use in form inputs and dialog labels |
 | `x-enumLabels` | Custom extension. Present on enum properties only. A string array, parallel to `enum`, giving a UI-friendly display name for each allowed value |
 
-The connection schema uses JSON Schema Draft 7 `if`/`then`/`else` to express type-dependent fields. When `type` equals `"tcp"`, the fields in `then.properties` apply (TCP-specific). Otherwise the fields in `else.properties` apply (serial-specific). A UI can use this to enable or disable the relevant fields based on the selected connection type.
+The connection schema uses JSON Schema Draft 7 `if`/`then`/`else` to express type-dependent fields. When `type` equals `"tcp"`, the fields in `then.properties` apply (TCP-specific). Otherwise, the fields in `else.properties` apply (serial-specific). A UI can use this to enable or disable the relevant fields based on the selected connection type.
 
 ---
 

src/customwidgets/schemaformwidget.cpp

@@ -104,20 +104,25 @@ void SchemaFormWidget::setSchema(const QJsonObject& schema, const QJsonObject& v
         _pFormLayout->addRow(label + ":", widget);
     }
 
-    applyConditional(values.value(_conditionalTriggerKey).toVariant().toString());
-
+    QComboBox* triggerCombo = nullptr;
     for (const auto& [key, widget] : std::as_const(_fields))
     {
         if (key == _conditionalTriggerKey)
         {
-            auto* combo = qobject_cast<QComboBox*>(widget);
-            if (combo != nullptr)
-            {
-                connect(combo, &QComboBox::currentIndexChanged, this, &SchemaFormWidget::onTriggerChanged);
-            }
+            triggerCombo = qobject_cast<QComboBox*>(widget);
             break;
         }
     }
+
+    if (triggerCombo != nullptr)
+    {
+        applyConditional(triggerCombo->currentData().toString());
+        connect(triggerCombo, &QComboBox::currentIndexChanged, this, &SchemaFormWidget::onTriggerChanged);
+    }
+    else
+    {
+        applyConditional(values.value(_conditionalTriggerKey).toVariant().toString());
+    }
 }
 
 QWidget* SchemaFormWidget::createWidgetForProperty(const QJsonObject& propSchema, const QJsonValue& value)

src/dialogs/adaptersettings.cpp

@@ -15,7 +15,6 @@ AdapterSettings::AdapterSettings(SettingsModel* pSettingsModel,
     : QWidget(parent), _pSettingsModel(pSettingsModel), _adapterId(adapterId), _propertyKey(propertyKey)
 {
     auto* layout = new QVBoxLayout(this);
-    setLayout(layout);
 
     const AdapterData* pAdapter = pSettingsModel->adapterData(adapterId);
     const QJsonObject propSchema = pAdapter->schema().value("properties").toObject().value(propertyKey).toObject();
@@ -63,20 +62,9 @@ void AdapterSettings::buildSection(const QJsonObject& propSchema, const QJsonVal
             _pItemTabs->setTabs(pages, names);
         }
 
-        connect(_pItemTabs, &AddableTabWidget::addTabRequested, this, [this]() {
-            auto* form = new SchemaFormWidget(_pItemTabs);
-            QJsonObject defaultValues;
-            const QJsonArray defaultItems =
-              _pSettingsModel->adapterData(_adapterId)->defaults().value(_propertyKey).toArray();
-            if (!defaultItems.isEmpty())
-            {
-                defaultValues = defaultItems.first().toObject();
-            }
-            form->setSchema(_itemSchema, defaultValues);
-            const int newIndex = _pItemTabs->count() + 1;
-            const QString name = QString("%1 %2").arg(_propertyKey[0].toUpper() + _propertyKey.mid(1)).arg(newIndex);
-            _pItemTabs->addNewTab(name, form);
-        });
+        _nextItemTabIndex = itemsArray.size() + 1;
+
+        connect(_pItemTabs, &AddableTabWidget::addTabRequested, this, &AdapterSettings::addItemTab);
 
         layout->addWidget(_pItemTabs, 1);
     }
@@ -89,6 +77,23 @@ void AdapterSettings::buildSection(const QJsonObject& propSchema, const QJsonVal
     }
 }
 
+void AdapterSettings::addItemTab()
+{
+    auto* form = new SchemaFormWidget(_pItemTabs);
+    QJsonObject defaultValues;
+    const QJsonArray defaultItems =
+        _pSettingsModel->adapterData(_adapterId)->defaults().value(_propertyKey).toArray();
+    if (!defaultItems.isEmpty())
+    {
+        defaultValues = defaultItems.first().toObject();
+    }
+    form->setSchema(_itemSchema, defaultValues);
+    const QString name =
+        QString("%1 %2").arg(_propertyKey[0].toUpper() + _propertyKey.mid(1)).arg(_nextItemTabIndex);
+    _nextItemTabIndex++;
+    _pItemTabs->addNewTab(name, form);
+}
+
 void AdapterSettings::acceptValues()
 {
     QJsonObject config = _pSettingsModel->adapterData(_adapterId)->effectiveConfig();

src/dialogs/adaptersettings.h

@@ -46,6 +46,7 @@ class AdapterSettings : public QWidget
 
 private:
     void buildSection(const QJsonObject& propSchema, const QJsonValue& configValue, QVBoxLayout* layout);
+    void addItemTab();
 
     SettingsModel* _pSettingsModel;
     QString _adapterId;
@@ -55,6 +56,7 @@ class AdapterSettings : public QWidget
     SchemaFormWidget* _pForm{ nullptr };
 
     QJsonObject _itemSchema;
+    int _nextItemTabIndex{ 1 };
 };
 
 #endif // ADAPTERSETTINGS_H

src/dialogs/settingsdialog.cpp

@@ -28,22 +28,16 @@ SettingsDialog::SettingsDialog(SettingsModel* pSettingsModel, QWidget* parent)
         _pUi->settingsStack->removeWidget(_pUi->settingsStack->widget(0));
     }
 
-    // Find first adapter with a populated schema
-    QString adapterId;
+    // Build dynamic pages from schema.properties of all adapters (all keys except "devices")
     const QStringList adapterIds = pSettingsModel->adapterIds();
     for (const auto& id : adapterIds)
     {
-        if (!pSettingsModel->adapterData(id)->schema().isEmpty())
+        if (pSettingsModel->adapterData(id)->schema().isEmpty())
         {
-            adapterId = id;
-            break;
+            continue;
         }
-    }
 
-    // Build dynamic pages from schema.properties (all keys except "devices")
-    if (!adapterId.isEmpty())
-    {
-        const QJsonObject schemaProps = pSettingsModel->adapterData(adapterId)->schema().value("properties").toObject();
+        const QJsonObject schemaProps = pSettingsModel->adapterData(id)->schema().value("properties").toObject();
 
         for (auto it = schemaProps.constBegin(); it != schemaProps.constEnd(); ++it)
         {
@@ -65,7 +59,7 @@ SettingsDialog::SettingsDialog(SettingsModel* pSettingsModel, QWidget* parent)
             listItem->setIcon(QIcon(":/menu_icon/icons/settings.svg"));
             _pUi->settingsList->addItem(listItem);
 
-            auto* page = new AdapterSettings(pSettingsModel, adapterId, key, this);
+            auto* page = new AdapterSettings(pSettingsModel, id, key, this);
             _dynamicSettings.append(page);
             _pUi->settingsStack->addWidget(page);
         }
@@ -107,15 +101,15 @@ void SettingsDialog::acceptAllValues()
 
 void SettingsDialog::done(int r)
 {
-    Q_UNUSED(r);
-
-    acceptAllValues();
+    if (r == QDialog::Accepted)
+    {
+        acceptAllValues();
+    }
 
-    QDialog::done(QDialog::Accepted);
+    QDialog::done(r);
 }
 
 void SettingsDialog::settingsStackSwitch(int newRow)
 {
-    acceptAllValues();
     _pUi->settingsStack->setCurrentIndex(newRow);
 }

tests/customwidgets/tst_schemaformwidget.cpp

@@ -414,7 +414,7 @@ void TestSchemaFormWidget::labelHiddenWithWidget()
         if (lbl->text() == QStringLiteral("Port Name:"))
         {
             portNameLabelFound = true;
-            QVERIFY(!lbl->isVisible());
+            QVERIFY(lbl->isHidden());
         }
     }
     QVERIFY(portNameLabelFound);