[Feature]: Web interface should not allow "Save to EEPROM" if user has not clicked "Update Settings" first
Closed this issue · 11 comments
What would you like to add in terms of software changes?
At present it is possible for a user to click the "Save to EEPROM" button in the Proton Pack Settings or Neutrona Wand Settings menus without having clicked "Update Settings," which has no actual effect but may cause the user to incorrectly believe that the settings they just changed in the web interface before clicking the button were saved to the EEPROM when they were not.
In fact it might be worth going a step further and making it so both buttons are disabled unless the user actually makes a change, after which "Update Settings" becomes enabled. Then only after clicking "Update Settings" does "Save to EEPROM" become enabled.
Would this request involve any specific hardware?
ESP32 Wifi Module/Attenuator.
Homework Completed
- I did my part!
This was mainly by design. The actions are separate and do their own thing, if that's what the user wants to do. I thought that it is possible the user can exit the EEPROM menu without saving (rare, but possible) so they may want to save settings after the fact. Users are told clearly on each preference screen to update to test THEN save to EEPROM.
It is not possible to exit the EEPROM Settings menu without making any changes. The only options in the EEPROM Menu are "Clear EEPROM and exit" or "Save current settings to EEPROM and exit."
The fact that it is possible to make changes on the web interface then directly click "Save to EEPROM" without clicking "Update Settings" both means the user never sees the prompt telling them to test before saving to EEPROM, and means that they are led to believe (falsely) that their changes were saved to EEPROM.
Did we close all loopholes that would've allowed a toggle flip or other action to cause the user to get thrown out of the EEPROM menu?
We did, yes. Flipping switches no longer has any effect in the EEPROM menus.
EDIT: With one exception that would be trivial to patch if we wanted, which is that if you somehow get the pack to boot up (wifi?) it will kick the wand out of the EEPROM menu without saving to the EEPROM (though the actual settings themselves will be modified for runtime variables).
But again, even so, that has nothing to do with the enhancement being discussed here. The point is it is currently possible for a user to not be warned about clicking update settings and testing first and just click "Save to EEPROM" and believe that by doing so, the settings they changed were saved to the EEPROM. When in reality, the "Save to EEPROM" button only has an actual effect after "Update Settings" has been clicked.
I'm just establishing why the functionality existed as it does, and why it has no bearing on saving settings. But if we've strengthened the menu system to avoid most ways of exiting without first saving, that changes the impact of the separate buttons. In fact, that now means we could chain prompts after a successful save to ask if the user wants to save to eeprom as a follow up action.
Ah, I gotcha now. So just make it a single button instead of two, and have a popup prompt that determines whether to send the EEPROM Save message. That works too!
Yup. Just another day, another XY Problem. Understanding why the conditions are set as they are helps eliminate the true problem. In this case, we don't really need the separate button any longer, so we should focus on making sure users follow a set path which is actually similar to that of the source system (EEPROM menu in this case). If they can only save AND exit then we should attempt to do the same.
I'd likely offer it as a simple JavaScript modal confirm() with callback to ask if they want to permanently save to EEPROM, so if they say "OK" we save, and if they say "Cancel" we leave the settings intact but don't update the EEPROM. If they want to save to EEPROM later, they must force the saving of current values first, then answer "OK" to the prompt asking if they wish to commit to EEPROM. All that would save me from checking the state of the fields on the page to know whether they've made a modification, and allows the last update of settings to ensure everything is up to date.
FYI, I've started working on this in my new VSCode environment. I want to make sure that works out and the feature behaves as expected, and I'll port back to the v5 dev code. So you won't see updates as I work on it though it's being handled. I do like this new change--should simplify things a lot.
Migrating this over to the v5 code and testing. You'll get 2 modals: an alert to tell you that the updates were saved and to make sure you test everything first, then immediately followed by the confirmation to save to EEPROM. It may be a bit much, but I really want to make sure people know they need to test out the changes and this really guides them through the process to save permanently.
Just pushed, and calling this done until we have user feedback on whether to simplify further in a future update.
Just flashed it and yep, that works! Now there's no way to click an update button without the settings actually taking effect.
Perhaps in a future update there could be a toggle above the button for "Save to EEPROM" but for now, this works just dandy. Thanks a bunch!