BitcoinDesign / Bitcoin-Core-App

Tracking the design process for the Bitcoin Core App.
https://bitcoincore.app
MIT License
56 stars 11 forks source link

Settings that are overridden #44

Open jarolrod opened 1 year ago

jarolrod commented 1 year ago

Settings can be overridden either by passing command line options or by being defined in a bitcoin.conf. If it is overridden, then the value for the setting that is in settings.json will be ignored.

This is the warning that the qt widgets gui gives in such a case:

Screen Shot 2023-02-06 at 1 23 50 AM

How should we handle such a case? I think we could just disable the icon and include a description within the setting row that this setting is overridden

GBKS commented 1 year ago

Something like this? These are two options, with the right option being more explicit.

image

yashrajd commented 1 year ago

If a setting is overridden from CLI or conf file, can we just update the UI to display the right information?

Greying out the setting & icon etc. makes sense, add the help text inside the box saying "This setting has been set from the command line/.conf file"

yashrajd commented 1 year ago

The options that do not have a UI item can be listed in the way Qt app does it currently, possibly in the Developer Options section...

GBKS commented 1 year ago

The greying out works. Very subtle, but should work. I'd only grey out the actual value itself, not the item text on left.

yashrajd commented 1 year ago

I'd like to work on this, please assign it to me.

yashrajd commented 1 year ago

@jarolrod is it possible to identify and show where the setting is getting overridden from? It'd be great to not just show that a setting is being overridden but let users potentially action it.

edit: also need to confirm that we can display the actual overriding value in the UI?

yashrajd commented 1 year ago
Screenshot 2023-07-17 at 22 33 21 Screenshot 2023-07-17 at 22 34 49
GBKS commented 1 year ago

Thanks for creating those mock-ups. I find the language too technical. Let's try to avoid terms like CLI and .conf. It's also not clear what they mean. Do the tags indicate that you can override the setting that's in the conf file? Or is it the other way around?

Maybe just disable those options, add a small info circle, and repeat that the bottom of the screen with a message like "These options are defined in your settings can file and cannot be changed here."?

ryanofsky commented 1 year ago

As of https://github.com/bitcoin-core/gui/pull/602 in bitcoin-qt, GUI settings that have been changed override settings in the bitcoin.conf file. The bitcoin.conf file just provides default settings values that can be customized in the GUI.

CLI settings still override GUI settings so they can be used for debugging and testing. CLI settings should be temporary and probably non-technical users should never encounter them. It seems fine to disable settings in the GUI that are overridden by the command line. Also fine to allow them to be set but note that they won't have an effect until bitcoin is restarted.

yashrajd commented 1 year ago

Thanks for the correction @ryanofsky. Let's see if I understood that currently, and what it implies:

As of bitcoin-core/gui#602 in bitcoin-qt, GUI settings that have been changed override settings in the bitcoin.conf file. The bitcoin.conf file just provides default settings values that can be customized in the GUI.

CLI settings still override GUI settings so they can be used for debugging and testing. CLI settings should be temporary and probably non-technical users should never encounter them. It seems fine to disable settings in the GUI that are overridden by the command line. Also fine to allow them to be set but note that they won't have an effect until bitcoin is restarted.

If a setting is changed from CLI, we can simply show the updated value in the GUI and inform the user of the fact (retain the CLI override label, with better language). We do not disable the control and let users try and change it. If they to attempt to change the value, we update it in the GUI along with the note that restart is needed, per #43 .

ryanofsky commented 1 year ago

re: https://github.com/BitcoinDesign/Bitcoin-Core-App/issues/44#issuecomment-1650541231, yes that's all correct.

Personally, I don't think the "CLI override" label by itself really explains very much, and it is potentially confusing because it isn't clear about whether the displayed value is the saved value which will take effect next restart, or the current effective value which comes from the command line.

IMO, the ideal interface would not have a "CLI override" tag, but instead would have a "Pending" tag. The pending tag would be shown next to any value which was saved to settings, but will not take effect until the next restart. To be even more explicit, below each pending setting there could be a little note like: "The value above is saved and will take effect the next time this application is started. The current effective value is XXX." If desired, in the case of a CLI override, the note could be even more specific and say "The current effective value is XXX, which was specified on the command line."

yashrajd commented 1 year ago

Implemented both your & @GBKS 's suggestions in a way that (hopefully) reflect the following learnings:

Screenshot 2023-08-01 at 21 27 40
GBKS commented 1 year ago

Nice! I like the simplicity of this. Small tweak could be to remove "Bitcoin Core" from the label on the right, so it would just be "Restart to apply this change".

There is as balance of how explicit we want/need to be. Yashraj's solution is more minimal, while Ryan's is more expressive with full sentences that describe the change. Personally, I lean more towards the minimal one.

Another thing to consider is whether we want to give an option to revert changes. Not 100% sure this is necessary. We can take our approach again where we start with a simple solution and only add more complexity if people ask for it (or run into problems). That way we don't overcomplicate right from the start due to defensive thinking.

Yashraj, have you looked for established patterns on this from other applications or operating systems? I can't think of any examples spontaneously, but there is probably some material to borrow from out there.