feat: settings menu #859
feat: settings menu #859
Conversation
CyanVoxel
added
Type: Enhancement
CyanVoxel
added
the
Type: Refactor
|
this is now ready for review |
There was a problem hiding this comment.
This is looking great! I've got a couple questions I wanted to ask just so everything's square, but other than that this is approved once the remaining comments are addressed.
- Is the legacy .ini file intended to be used as a sort of cache file for things like the recently used libraries list now?
- Was the
thumb_cache_size_limitsetting left to remain in the.inifile since it's eventually intended to be a library setting? (This is totally fine if so)
As for general comments, I noticed that changing the page size in the settings doesn't seem to update the actual page size (even after re-searching) except for an initial change back to what seems to be the default of 500. For example if I set the size to 1,000 and restart the program, if I then change it to 10 and re-search then the page size will seem to show 500 entries per page until restarting. If this can't be fixed easily then just showing the "restart required" label can do for now.
Finally for the dark mode setting, if it's not too much I feel this would be better as a three-way option between "Light", "Dark", and "System" with the default being "System". Xarvex has been fixing up the system theme detection issues that were present across several Linux machines so there should be fewer chances of flash-banging users with dark system themes.
Thank you so much for your work on this!
src/tagstudio/qt/ts_qt.py
Outdated
| def open_settings_modal(self): | ||
| # TODO: Implement a proper settings panel, and don't re-create it each time it's opened. | ||
| settings_panel = SettingsPanel(self) | ||
| modal = PanelModal( | ||
| widget=settings_panel, | ||
| done_callback=lambda: self.update_language_settings(settings_panel.get_language()), | ||
| has_save=False, | ||
| ) | ||
| modal.setTitle(Translations["settings.title"]) | ||
| modal.setWindowTitle(Translations["settings.title"]) | ||
| modal.show() | ||
|
|
||
| def update_language_settings(self, language: str): | ||
| Translations.change_language(language) | ||
|
|
||
| self.settings.setValue(SettingItems.LANGUAGE, language) | ||
| self.settings.sync() | ||
| # TODO: don't re-create each time this is opened. | ||
| SettingsPanel.build_modal(self).show() |
There was a problem hiding this comment.
Is this TODO still true now that build_modal is a classmethod? If not, SettingsPanel.build_modal(self).show could be used in place of open_settings_modal
There was a problem hiding this comment.
That is indeed not a TODO anymore and for the same reason that SettingsPanel.build_modal(self).show can't be used instead of open_settings_modal. Because open_settings_modal is connected to a QSignal and replacing it would cause the show method to always be called on the same instance of the modal, therefore breaking the cancel functionality of the modal.
PS: I removed the TODO
Yes and yes, what didn't belong into the global settings I just left as is.
works now.
done. |
Summary
Implements a settings menu for global settings, addressing the roadmap item.
Close #647.
Currently provides the following settings:
Tasks Completed