Turn settings menu to a dialog window #605
Conversation
rpallai
changed the title
There was a problem hiding this comment.
Qt Designer is certainly not banned, and I agree that using it for a preferences window is reasonable. What I can't accept though is that on my machine the preferences dialog is too small for the contents, with scrollbars in both directions even though there's enough room on the screen to make the dialog larger. Can you please somehow bind the size of the dialog to its contents up to a certain limit?
One more suboptimal thing: I understand that tabs are almost inevitable in the long term. But having both tabs and groups looks really over the top. I'd probably favour tabs over bigger captions. Or, if you prefer everything to be on a single page (I fully agree it's better from usability perspective) then hide the tab strip until it's really needed.
|
If I'm not mistaken, the window size issue is fixed, please test. The tab strip has been hidden for now. |
|
Still not :( |
|
It must be good now. |
|
Finally got to this - thanks, it works pretty much as expected now. Would you please rebase it to the latest master (and, if you don't mind, add a tab with fonts configuration)? |
rpallai
force-pushed
the
settingsdialog
branch
from
edf3db8
to
dc6d5f2
Compare
|
Rebased and font settings have been exposed. I think adding tabs is unnecessary for now as the settings window is still pretty short. |
rpallai
force-pushed
the
settingsdialog
branch
from
dc6d5f2
to
cf4ba05
Compare
There was a problem hiding this comment.
Thanks a lot! I have one major piece of feedback regarding the font part - instead of a pair of combo boxes it would probably be better to use a font dialog (despite the dialog-over-dialog ugliness). The reason here is that some fonts have a few weights that can be equally fitting (light, regular and semibold, e.g.), and 6-18 range for font sizes might not be quite right for some extreme cases. In other words, a platform-native font dialog would certainly cover the use case better than the font configuration approximation with two combo boxes. What do you think?
| </property> | ||
| </widget> | ||
| </item> | ||
| </layout> |
There was a problem hiding this comment.
I appreciate the attempt to reduce the number of popups but is there any other reason not to use a button summoning a QFontDialog?
| const auto curUIFontPointSize = SettingsGroup("UI").value("Fonts/pointSize", ""); | ||
| uiFontPointSize->setCurrentIndex(curUIFontPointSize.toInt() - 6); | ||
|
|
||
| connect(uiFontPointSize, QOverload<const QString &>::of(&QComboBox::currentIndexChanged), this, [this](const QString &text) { |
There was a problem hiding this comment.
Can we use QComboBox::currentTextChanged() that would lend for a more concise syntax (assuming we keep the two combo boxes instead of a font dialog)?
| Ui_GeneralPage::setupUi(this); | ||
|
|
||
| // uiFont | ||
| const auto curUIFontFamily = SettingsGroup("UI").value("Fonts/family", "").toString(); |
There was a problem hiding this comment.
JFYI, the recent lib has Settings::get template method that allows to write as follows (I'm not really pushing it as it's arguably slightly less readable):
| const auto curUIFontFamily = SettingsGroup("UI").value("Fonts/family", "").toString(); | |
| const auto curUIFontFamily = SettingsGroup("UI").get("Fonts/family", QString()); |
| // timelineFont | ||
| const auto curTimelineFontFamily = SettingsGroup("UI").value("Fonts/timeline_family", "").toString(); | ||
| timelineFontFamily->insertItem(0, tr("system default")); | ||
| if (curTimelineFontFamily == "") { |
There was a problem hiding this comment.
| if (curTimelineFontFamily == "") { | |
| if (curTimelineFontFamily.isEmpty()) { |
I hope Qt Designer is not banned.
Closes #553.