-
Notifications
You must be signed in to change notification settings - Fork 339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Console] Improve interactive-mode preferences saving #445
Conversation
It is good to explain the problem that is being solved in the commit message because it is not always self-evident and it took a while for me to understand this issue. I can see now that the preferences close dialog actions by default requires a two-step action, first select one-of There is a second more subtle issue where if the user hits cancel in preferences then the changes made are not discarded so the user might still think those preferences have been saved. Something a bit hacky like this should solve it: - def _update_preferences(self, core_config):
+ def _update_preferences(self, core_config, console_config=None):
self.core_config = core_config
for pane in self.panes:
- pane.update_values(core_config)
+ if isinstance(pane, InterfacePane) and console_config:
+ pane.update_values(console_config)
+ else:
+ pane.update_values(core_config)
def _actions_read(self, c):
self.action_input.handle_read(c)
if c in [curses.KEY_ENTER, util.KEY_ENTER2]:
# take action
if self.action_input.selected_index == 0: # Cancel
+ # Reload stored config for panes
+ self._update_preferences(self.core_config, self.console_config)
self.back_to_parent()
elif self.action_input.selected_index == 1: # Apply
self._apply_prefs() This also raises the question about how the preference panes are refreshed when other interfaces update the core config since it appears core config is only loaded once per session (unless Apply selected). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment
257289c
to
e8adbfb
Compare
The bottom options for cancel/apply/ok where confusing for end-users as being checkboxes needing spacebar prepended to activate firstly, before return/enter to activate said previous selection, but changed now to omit. Also fixed not showing canceled options as sticking.
I'm sorry Cas, I didn't actually understood those where checkboxes and thought regular buttons(so return/enter only), so thought actual bug and not just usability issue, thanks for explaining, appreciated and i'll try be better with verbosity, which agreed particularly bad here, despite my confusion. As said before, if feel faster for you to just make yourself (here/elsewhere) then please just close and don't need explain, little anxious about being a nuisance honestly :) You understand hopefully. Thanks! |
Thanks for understanding, these are helpful commits and I am happy to have the discussion to ensure the changes are what we expect them to be :) |
The bottom options for cancel/apply/ok where confusing for end-users as being checkboxes needing spacebar prepended to activate firstly, before return/enter to activate said previous selection, but changed now to omit. Also fixed not showing canceled options as sticking. Co-authored-by: Calum Lind <[email protected]> Closes: deluge-torrent#445
The bottom options for cancel/apply/ok where confusing for end-users as being checkboxes needing spacebar prepended to activate firstly, before return/enter to activate said previous selection, but changed now to omit. Also fixed not showing canceled options as sticking.