More intuitive GUI settings behavior when -proxy is set#13818
More intuitive GUI settings behavior when -proxy is set#13818Sjors wants to merge 2 commits intobitcoin:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
utACK 6bde499 |
|
Concept ACK |
src/qt/optionsdialog.cpp
Outdated
There was a problem hiding this comment.
Not a big issue but to me it seems overkill to both disable the control and not connect the signals, isn't only disabling enough?
There was a problem hiding this comment.
These signal connections re-enable those fields, even if I disable them in the next line. I probably could have fixed with an async call somewhere, but this was easier.
6bde499 to
b39ca20
Compare
|
utACK b39ca20 |
|
Not strictly necessary, but it may also be safer to wait for #12833. I agree with the need to add test, though it may be a while before I get to it. Happy to cherry-pick and review tests by someone else in the mean time. |
|
Recently increased enthusiasm for rw_config means it's better to work on top of that once merged. I agree that adding tests would be a good idea; it's probably going to be a while before I get to that though. |
| The last travis run for this pull request was 307 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened. |
|
@Sjors: could you have a look in a better automated test approach for this? |
|
Closing this for now until the smoke clears around other settings PRs. |
Given
proxy=127.0.0.1:9051bitcoin.conf (or using-proxy):Before:

There are number of confusing aspects to this:
bitcoin.confSo I changed the behavior to check the box and populate the initial setting (only done at first launch or after you reset QT settings) if
-proxyis set (perhaps it's even to always do this?).In addition the user can no longer disable the check box or edit the settings when
-proxyis set. They have to remove the entry frombitcoin.conffirst.After:

#11082 and #12833 are a more rigorous solution, but there's some overlap. E.g. if a setting exists in the read-only
bitcoin.confit should be similarly disabled in the GUI, along with an instruction to remove it frombitcoin.confif the user wishes to edit it (using the read-writebitcoin-rw.conf).This UX pattern can be applied to a few other settings as well. I found
proxyparticularly useful because it's used with Tor where confusion is really not a good thing.I'm guessing that a separate Tor proxy through
-onionis less common, so I didn't touch that (yet). I find that setting confusing in general.