Make font size configurable #1872

Kissaki · 2015-11-02T00:49:27Z

Add configuration options for the font of Log, Input and Tree.

Before to after:

Before	After

Fixes #1850

Kissaki · 2015-11-02T19:29:32Z

The mentioned issue is solved with the recent commit.

Various sizes in use:

Kissaki · 2015-11-02T20:19:59Z

I do not know where previously our default font came from.
Now the default, empty QFont gives a different default.
This default, for me on Win7x64, is smaller - too small; MS Shell Dlg2 8pt.

Anyone has an idea where the previous default came from?

QApplication maintains a system/theme font which serves as a default for all widgets.

I guess the default fonts in set in the constructor Settings::Settings should be determined by MainWindow.font().
Not sure about initialization order though. It seems g.mw is not set correctly on initial Settings construction.

mkrautz · 2015-11-26T23:34:44Z

Seems fine to me, but:

I think having to select both the font and the size seems a little heavyweight. I had initially thought this would only change the font size. But I guess it's not easy to decouple.
We need to figure out what the default font for the log is.
I suppose we could query it via the application's stylesheet somehow?

bendem · 2015-11-28T19:24:20Z

This patch works as expected. It's kinda weird that the timestamps are not affected by the selected font size for the logs tho.

Also, UI wise, I'd put the Change All before the 3 separate ones. People will generally want to keep these options in sync and by the time they get to that button, they'll already have changed the 3 settings.

Krzmbrzl · 2020-01-25T18:47:01Z

@Kissaki what's the status on this PR? The last comment appears to be indicating that everything is working as expected. Why hasn't this been merged?

Kissaki · 2020-01-26T15:32:28Z

@Krzmbrzl I can only tell you what I see here. It looks like it has not been reviewed, and the comment by mkrautz raised questions which are not answered (who also assigned me again).

bendem added some more suggestions which have not been applied or answered.

Does that answer your question? It has not been merged because nobody merged it.

Krzmbrzl · 2020-01-26T15:53:31Z

Alright then let's get this thing rounded up so we can merge it :)

For the default font I'd use the approach suggested in https://stackoverflow.com/questions/11011238/how-do-you-get-system-default-font-settings-in-qt#12179548
That way I think we also don't have to care about initialization order of the MainWindow and the settings.

I think having to select both the font and the size seems a little heavyweight

I think this is great. It allows for more flexibility. And if the user only wants to change the font size, I assume this is easily possible, isn't it?

It's kinda weird that the timestamps are not affected by the selected font size for the logs tho.

I think this should indeed be addressed (if it is still the case)

I'd put the Change All before the 3 separate ones. People will generally want to keep these options in sync and by the time they get to that button, they'll already have changed the 3 settings.

Fair point. I think we don't lose anything by doing this and it'd have the mentioned advantage.

I'll add my actual code review in the next couple of days (if I should forget it, please remind me) :)

Krzmbrzl · 2020-01-27T17:15:02Z

src/mumble/LookConfig.cpp

+	updateFontInfo(s);
+}
+
+void LookConfig::updateFontInfo(const Settings &r) {


I think r is a bad choice for a variable representing a setting. I'd also prefer a more verbose name. Something like settings I think would be ideal.

Krzmbrzl · 2020-01-27T17:15:58Z

src/mumble/LookConfig.cpp

+}
+
+void LookConfig::updateFontInfo() {
+	updateFontInfo(s);


What's s? I think there should be a comment here explaining what this call is supposed to do

Krzmbrzl · 2020-01-27T17:16:59Z

src/mumble/LookConfig.cpp

@@ -171,6 +187,8 @@ void LookConfig::load(const Settings &r) {
 		}
 	}

+	updateFontInfo(r);


What is this doing? It appears somewhat lonely in the code, so I think there should be a comment

Krzmbrzl · 2020-01-27T17:17:37Z

src/mumble/LookConfig.cpp

@@ -261,3 +279,39 @@ void LookConfig::themeDirectoryChanged() {
 		reloadThemes(themeData.value<ThemeInfo::StyleInfo>());
 	}
 }
+
+void LookConfig::changeFont(QFont & font) {


Just a style-thing: There's an additional blank after the &

Krzmbrzl · 2020-01-27T17:19:35Z

src/mumble/LookConfig.h

+		void updateFontInfo(QLabel *l, const QFont &f);
+		void updateFontInfo();
+		void updateFontInfo(const Settings &r);
+		void changeFont(QFont & font);
+	protected slots:
+		void on_qpbFontLog_clicked();
+		void on_qpbFontInput_clicked();
+		void on_qpbFontTree_clicked();
+		void on_qpbFontAll_clicked();


Missing documentation. A function header (Doxygen) explaining what these functions are intended for and what the parameters are for would be nice

Krzmbrzl · 2020-01-27T17:21:10Z

src/mumble/Settings.h

+	QFont qfFontLog;
+	QFont qfFontInput;
+	QFont qfFontTree;


Documentation: A small Doxygen comment stating what these fonts are intended to be used for would improve readability

Implement mumble-voip#1850 Add configuration options for the font of Log, Input and Tree.

Self user and channel, as well as linked channels have adjusted font properties. Up to now this was based on the main window font. Now it correctly uses the configured font.

FireZtreaM · 2022-11-27T14:46:40Z

Is there any updates on this? Still want this feature!

Krzmbrzl · 2022-11-27T16:07:27Z

Nope - if there was any activity on this, it'd be here

Kissaki added client ui labels Nov 2, 2015

Kissaki assigned Kissaki and unassigned Kissaki Nov 2, 2015

mkrautz assigned Kissaki Nov 26, 2015

davidebeatrici force-pushed the master branch from 55a6687 to 501651b Compare December 24, 2016 22:25

Krzmbrzl requested changes Jan 27, 2020

View reviewed changes

toby63 mentioned this pull request Mar 23, 2020

Revamped UI #3608

Open

Krzmbrzl mentioned this pull request Jun 20, 2020

Make Font-size configurable #1850

Open

Kissaki mentioned this pull request Oct 30, 2020

Made font size for TalkingUI entries configurable #4168

Merged

Kissaki marked this pull request as draft October 30, 2020 20:41

Kissaki force-pushed the f/fontconfig branch from 8e896b0 to b1a1d41 Compare October 30, 2020 20:47

Kissaki added 4 commits October 30, 2020 21:48

Move toolbar options to separate group

30e01cc

Make font size configurable

60aebcc

Implement mumble-voip#1850 Add configuration options for the font of Log, Input and Tree.

Use configured font for visual adjustments in Tree

918229e

Self user and channel, as well as linked channels have adjusted font properties. Up to now this was based on the main window font. Now it correctly uses the configured font.

Update font info on restore default settings

6d880d3

Kissaki force-pushed the f/fontconfig branch from b1a1d41 to 6d880d3 Compare October 30, 2020 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make font size configurable #1872

Make font size configurable #1872

Kissaki commented Nov 2, 2015 •

edited

Kissaki commented Nov 2, 2015

Kissaki commented Nov 2, 2015

mkrautz commented Nov 26, 2015

bendem commented Nov 28, 2015 •

edited by Kissaki

Krzmbrzl commented Jan 25, 2020

Kissaki commented Jan 26, 2020 •

edited

Krzmbrzl commented Jan 26, 2020

Krzmbrzl Jan 27, 2020

Krzmbrzl Jan 27, 2020

Krzmbrzl Jan 27, 2020

Krzmbrzl Jan 27, 2020

Krzmbrzl Jan 27, 2020

Krzmbrzl Jan 27, 2020

FireZtreaM commented Nov 27, 2022

Krzmbrzl commented Nov 27, 2022

Make font size configurable #1872

Are you sure you want to change the base?

Make font size configurable #1872

Conversation

Kissaki commented Nov 2, 2015 • edited

Kissaki commented Nov 2, 2015

Kissaki commented Nov 2, 2015

mkrautz commented Nov 26, 2015

bendem commented Nov 28, 2015 • edited by Kissaki

Krzmbrzl commented Jan 25, 2020

Kissaki commented Jan 26, 2020 • edited

Krzmbrzl commented Jan 26, 2020

Krzmbrzl Jan 27, 2020

Choose a reason for hiding this comment

Krzmbrzl Jan 27, 2020

Choose a reason for hiding this comment

Krzmbrzl Jan 27, 2020

Choose a reason for hiding this comment

Krzmbrzl Jan 27, 2020

Choose a reason for hiding this comment

Krzmbrzl Jan 27, 2020

Choose a reason for hiding this comment

Krzmbrzl Jan 27, 2020

Choose a reason for hiding this comment

FireZtreaM commented Nov 27, 2022

Krzmbrzl commented Nov 27, 2022

Kissaki commented Nov 2, 2015 •

edited

bendem commented Nov 28, 2015 •

edited by Kissaki

Kissaki commented Jan 26, 2020 •

edited