From ecf992cb62ee0b4c2c52466dbad0fd7c84cb1361 Mon Sep 17 00:00:00 2001 From: "Tom Dewey tom.dewey@raspberrypi.com" Date: Tue, 17 Oct 2023 15:21:49 +0100 Subject: [PATCH] Rework OS Customization In the new flow, it doesn't make sense to _not_ save the OS customization parameters, so remove the ComboBox. Additionally, our data model was failing to notify the UI of changes to the saved settings state. Due to time constraints, I'm not able to resolve the binding in the 'correct' manner, but I can introduce a makeshift status signalling mechanism to prevent UI inconsistency. --- src/OptionsPopup.qml | 93 +++++++++++++++++++---------------- src/UseSavedSettingsPopup.qml | 18 ++++++- src/imagewriter.cpp | 3 ++ src/main.qml | 6 +++ 4 files changed, 76 insertions(+), 44 deletions(-) diff --git a/src/OptionsPopup.qml b/src/OptionsPopup.qml index db43505..7b41d2a 100644 --- a/src/OptionsPopup.qml +++ b/src/OptionsPopup.qml @@ -29,6 +29,8 @@ Window { property string cloudinitwrite property string cloudinitnetwork + signal saveSettingsSignal(var settings) + ColumnLayout { id: cl spacing: 10 @@ -51,16 +53,6 @@ Window { Label { text: qsTr("OS customization options") } - ComboBox { - id: comboSaveSettings - model: { - [qsTr("for this session only"), - qsTr("to always use")] - } - Layout.minimumWidth: 250 - Layout.maximumHeight: 40 - enabled: !imageWriter.isEmbeddedMode() - } } TabBar { @@ -453,7 +445,6 @@ Window { fieldKeyboardLayout.model = imageWriter.getKeymapLayoutList() if (Object.keys(settings).length) { - comboSaveSettings.currentIndex = 1 hasSavedSettings = true } if ('hostname' in settings) { @@ -807,43 +798,61 @@ Window { function saveSettings() { - if (comboSaveSettings.currentIndex == 1) { - hasSavedSettings = true - var settings = { }; - if (chkHostname.checked && fieldHostname.length) { - settings.hostname = fieldHostname.text - } - if (chkSetUser.checked) { - settings.sshUserName = fieldUserName.text - settings.sshUserPassword = fieldUserPassword.alreadyCrypted ? fieldUserPassword.text : imageWriter.crypt(fieldUserPassword.text) - } + var settings = { }; + if (chkHostname.checked && fieldHostname.length) { + settings.hostname = fieldHostname.text + } + if (chkSetUser.checked) { + settings.sshUserName = fieldUserName.text + settings.sshUserPassword = fieldUserPassword.alreadyCrypted ? fieldUserPassword.text : imageWriter.crypt(fieldUserPassword.text) + } - settings.sshEnabled = chkSSH.checked - if (chkSSH.checked && radioPubKeyAuthentication.checked) { - settings.sshAuthorizedKeys = fieldPublicKey.text + settings.sshEnabled = chkSSH.checked + if (chkSSH.checked && radioPubKeyAuthentication.checked) { + settings.sshAuthorizedKeys = fieldPublicKey.text + } + if (chkWifi.checked) { + settings.wifiSSID = fieldWifiSSID.text + if (chkWifiSSIDHidden.checked) { + settings.wifiSSIDHidden = true } - if (chkWifi.checked) { - settings.wifiSSID = fieldWifiSSID.text - if (chkWifiSSIDHidden.checked) { - settings.wifiSSIDHidden = true - } - settings.wifiPassword = fieldWifiPassword.text.length == 64 ? fieldWifiPassword.text : imageWriter.pbkdf2(fieldWifiPassword.text, fieldWifiSSID.text) - settings.wifiCountry = fieldWifiCountry.editText - } - if (chkLocale.checked) { - settings.timezone = fieldTimezone.editText - settings.keyboardLayout = fieldKeyboardLayout.editText - } - - imageWriter.setSavedCustomizationSettings(settings) - - } else if (hasSavedSettings) { - imageWriter.clearSavedCustomizationSettings() - hasSavedSettings = false + settings.wifiPassword = fieldWifiPassword.text.length == 64 ? fieldWifiPassword.text : imageWriter.pbkdf2(fieldWifiPassword.text, fieldWifiSSID.text) + settings.wifiCountry = fieldWifiCountry.editText + } + if (chkLocale.checked) { + settings.timezone = fieldTimezone.editText + settings.keyboardLayout = fieldKeyboardLayout.editText } imageWriter.setSetting("beep", chkBeep.checked) imageWriter.setSetting("eject", chkEject.checked) imageWriter.setSetting("telemetry", chkTelemtry.checked) + + if (chkHostname.checked || chkSetUser.checked || chkSSH.checked || chkWifi.checked || chkLocale.checked) { + /* OS customization to be applied. */ + hasSavedSettings = true + saveSettingsSignal(settings) + } + } + + function clearCustomizationFields() + { + fieldHostname.clear() + fieldUserName.clear() + fieldUserPassword.clear() + radioPubKeyAuthentication.checked = false + radioPasswordAuthentication.checked = false + fieldPublicKey.clear() + fieldWifiSSID.clear() + fieldWifiCountry.currentIndex = -1 + fieldWifiPassword.clear() + fieldTimezone.currentIndex = -1 + fieldKeyboardLayout.currentIndex = -1 + chkSetUser.checked = false + chkSSH.checked = false + chkLocale.checked = false + chkWifi.checked = false + chkWifiSSIDHidden.checked = false + chkHostname.checked = false } } diff --git a/src/UseSavedSettingsPopup.qml b/src/UseSavedSettingsPopup.qml index d17790d..a6b60d7 100644 --- a/src/UseSavedSettingsPopup.qml +++ b/src/UseSavedSettingsPopup.qml @@ -18,6 +18,8 @@ Popup { padding: 0 closePolicy: Popup.CloseOnEscape | Popup.CloseOnPressOutside + property bool hasSavedSettings: false + signal yes() signal no() signal noClearSettings() @@ -105,6 +107,7 @@ Popup { } ImButton { + id: noAndClearButton text: qsTr("NO, CLEAR SETTINGS") onClicked: { msgpopup.close() @@ -112,10 +115,11 @@ Popup { } Material.foreground: activeFocus ? "#d1dcfb" : "#ffffff" Material.background: "#c51a4a" - enabled: imageWriter.hasSavedCustomizationSettings() ? true : false + enabled: false } ImButton { + id: yesButton text: qsTr("YES") onClicked: { msgpopup.close() @@ -123,7 +127,7 @@ Popup { } Material.foreground: activeFocus ? "#d1dcfb" : "#ffffff" Material.background: "#c51a4a" - enabled: imageWriter.hasSavedCustomizationSettings() ? true : false + enabled: false } ImButton { @@ -142,6 +146,16 @@ Popup { function openPopup() { open() + if (hasSavedSettings) { + /* HACK: Bizarrely, the button enabled characteristics are not re-evaluated on open. + * So, let's manually _force_ these buttons to be enabled */ + yesButton.enabled = true + noAndClearButton.enabled = true + } else { + yesButton.enabled = false + noAndClearButton.enabled = false + } + // trigger screen reader to speak out message msgpopupbody.forceActiveFocus() } diff --git a/src/imagewriter.cpp b/src/imagewriter.cpp index 5d5b397..2a86281 100644 --- a/src/imagewriter.cpp +++ b/src/imagewriter.cpp @@ -1039,6 +1039,7 @@ void ImageWriter::setSavedCustomizationSettings(const QVariantMap &map) _settings.setValue(key, map.value(key)); } _settings.endGroup(); + _settings.sync(); } QVariantMap ImageWriter::getSavedCustomizationSettings() @@ -1060,10 +1061,12 @@ void ImageWriter::clearSavedCustomizationSettings() _settings.beginGroup("imagecustomization"); _settings.remove(""); _settings.endGroup(); + _settings.sync(); } bool ImageWriter::hasSavedCustomizationSettings() { + _settings.sync(); _settings.beginGroup("imagecustomization"); bool result = !_settings.childKeys().isEmpty(); _settings.endGroup(); diff --git a/src/main.qml b/src/main.qml index a3f6cbd..d755026 100644 --- a/src/main.qml +++ b/src/main.qml @@ -1183,6 +1183,10 @@ ApplicationWindow { OptionsPopup { id: optionspopup + onSaveSettingsSignal: { + imageWriter.setSavedCustomizationSettings(settings) + usesavedsettingspopup.hasSavedSettings = true + } } UseSavedSettingsPopup { @@ -1196,6 +1200,8 @@ ApplicationWindow { confirmwritepopup.askForConfirmation() } onNoClearSettings: { + hasSavedSettings = false + optionspopup.clearCustomizationFields() imageWriter.clearSavedCustomizationSettings() confirmwritepopup.askForConfirmation() }