From 7dbe24b9f8a29d85ea58a83b48eca95dc8618731 Mon Sep 17 00:00:00 2001 From: Stephen Oliver Date: Thu, 16 Jan 2020 18:36:11 -0500 Subject: [PATCH] Don't save settings unless their value has changed This replaces the previous simplistic code that just set a modified property on the setting in the model and used that to detect which settings had actually changed. The problem is that for various reasons that flag gets set even when the setting value isn't actually different from the original value, presumably because the onValueChanged callback in QML gets triggered for other reasons. Instead we store the original setting value and check it during a save which should prevent any setting from being saved unless it actually changed. Fixes #55 --- qml/ui/GroundPiSettingsPanel.qml | 25 +++++++++++++------ qml/ui/delegates/ChoiceSettingDelegate.ui.qml | 1 - qml/ui/delegates/NumberSettingDelegate.ui.qml | 1 - qml/ui/delegates/RangeSettingDelegate.ui.qml | 1 - qml/ui/delegates/SwitchSettingDelegate.ui.qml | 1 - qml/ui/delegates/TextSettingDelegate.ui.qml | 1 - 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/qml/ui/GroundPiSettingsPanel.qml b/qml/ui/GroundPiSettingsPanel.qml index b9aa7b3b0..3a49a38fe 100644 --- a/qml/ui/GroundPiSettingsPanel.qml +++ b/qml/ui/GroundPiSettingsPanel.qml @@ -183,9 +183,9 @@ GroundPiSettingsPanelForm { "upperLimit": upperLimit, "interval": interval, "itemType": itemType, + "originalValue": finalValue, "value": finalValue, "unit": unit, - "modified": false, "disabled": disabled, "info": itemInfo}); } @@ -236,6 +236,7 @@ GroundPiSettingsPanelForm { "setting": setting, "itemType": "string", "value": String(allSettings[setting]), + "originalValue": String(allSettings[setting]), "disabled": disabled, "info": "No additional information available, check the Open.HD wiki"}); } @@ -263,15 +264,23 @@ GroundPiSettingsPanelForm { function _process(model, mapping) { for(var index = 0; index < model.count; index++) { var setting = model.get(index); - var modified = setting["modified"]; - // skip saving any settings the user hasn't actually changed - if (!modified) { + + var key = setting["setting"]; + var originalValue = setting["originalValue"]; + var newValue = setting["value"]; + + // skip saving any settings the user hasn't actually changed + if (originalValue === newValue) { continue; } - var key = setting["setting"]; - var initialValue = setting["value"]; + // Update the originalValue in the model itself. + + // This is a quick hack, a better solution would be to only update the originalValue + // property once the setting actually saves + model.get(index).originalValue = newValue; + // by default we pass through the value as-is, only map to another type if needed - var finalValue = initialValue; + var finalValue = newValue; // map bool values back to their expected representation for each setting, because // it's not the same for all of them @@ -281,7 +290,7 @@ GroundPiSettingsPanelForm { var falseValue = mapping[setting.setting]["falseValue"]; if (itemType === "bool") { - if (initialValue) { + if (newValue) { finalValue = trueValue; } else { finalValue = falseValue; diff --git a/qml/ui/delegates/ChoiceSettingDelegate.ui.qml b/qml/ui/delegates/ChoiceSettingDelegate.ui.qml index 019ed1f7b..e1ee157b8 100644 --- a/qml/ui/delegates/ChoiceSettingDelegate.ui.qml +++ b/qml/ui/delegates/ChoiceSettingDelegate.ui.qml @@ -40,7 +40,6 @@ BaseDelegate { onActivated: { // @disable-check M222 listModel.setProperty(itemIndex, "value", valueElement.model.get(index).value); - itemModel.modified = true; } /* diff --git a/qml/ui/delegates/NumberSettingDelegate.ui.qml b/qml/ui/delegates/NumberSettingDelegate.ui.qml index 4de67d944..641d9af11 100644 --- a/qml/ui/delegates/NumberSettingDelegate.ui.qml +++ b/qml/ui/delegates/NumberSettingDelegate.ui.qml @@ -20,7 +20,6 @@ BaseDelegate { // @disable-check M223 onValueChanged: { model.value = value - model.modified = true } enabled: !model.disabled } diff --git a/qml/ui/delegates/RangeSettingDelegate.ui.qml b/qml/ui/delegates/RangeSettingDelegate.ui.qml index b519cc764..426720f79 100644 --- a/qml/ui/delegates/RangeSettingDelegate.ui.qml +++ b/qml/ui/delegates/RangeSettingDelegate.ui.qml @@ -17,7 +17,6 @@ BaseDelegate { anchors.rightMargin: Qt.inputMethod.visible ? 78 : 18 anchors.top: parent.top anchors.topMargin: 8 - onValueChanged: model.modified = true ToolTip { parent: valueElement.handle diff --git a/qml/ui/delegates/SwitchSettingDelegate.ui.qml b/qml/ui/delegates/SwitchSettingDelegate.ui.qml index 5a120fb57..1bf5fb2aa 100644 --- a/qml/ui/delegates/SwitchSettingDelegate.ui.qml +++ b/qml/ui/delegates/SwitchSettingDelegate.ui.qml @@ -20,7 +20,6 @@ BaseDelegate { // @disable-check M223 onCheckedChanged: { model.value = checked - model.modified = true } enabled: !model.disabled } diff --git a/qml/ui/delegates/TextSettingDelegate.ui.qml b/qml/ui/delegates/TextSettingDelegate.ui.qml index 081dbaad0..20c317b1b 100644 --- a/qml/ui/delegates/TextSettingDelegate.ui.qml +++ b/qml/ui/delegates/TextSettingDelegate.ui.qml @@ -25,7 +25,6 @@ BaseDelegate { // @disable-check M223 onTextChanged: { model.value = text; - model.modified = true; } enabled: !model.disabled }