ubuntu-touch-coreapps-reviewers team mailing list archive

[Stefano Verzegnassi <stefano92.100@xxxxxxxxx>]

Review: Needs Information

The MP looks good, although I'm not sure on the default and the maximum size for the font.
I've left two inline comments.

Also, it might be nice to handle the font size in terms of 100th in the settings page (i.e. from 4% to 500% - assuming the current range is ok).

Diff comments:

> 
> === modified file 'src/app/qml/TerminalSettings.qml'
> --- src/app/qml/TerminalSettings.qml	2015-12-19 05:14:39 +0000
> +++ src/app/qml/TerminalSettings.qml	2016-02-06 16:34:02 +0000
> @@ -11,9 +11,9 @@
>      property alias showKeyboardBar: innerSettings.showKeyboardBar
>      property alias showKeyboardButton: innerSettings.showKeyboardButton
>  
> -    readonly property int defaultFontSize: units.gu(0.4)
> -    readonly property int minFontSize: 2
> -    readonly property int maxFontSize: 32
> +    readonly property int defaultFontSize: 13

https://developer.ubuntu.com/api/qml/current/UbuntuUserInterfaceToolkit.resolution-independence/

By default, "medium" font size is 14px on desktop.
Multiplied a factor of 13/10 would result 18px.

Personally, I would expect to use 1.0x medium size as default.

> +    readonly property int minFontSize: 4
> +    readonly property int maxFontSize: 50

Isn't it too high?

Ubuntu Mono font has a 2:1 ratio.
The width of the font with fontSize=50 would be 14*5/2 = 35px
In a standard console window (80x24) that would mean a width of 2800px. I can't think at any monitor with such resolution. :P

>  
>      property alias jsonVisibleProfiles: innerSettings.jsonVisibleProfiles
>  


-- 
https://code.launchpad.net/~nikwen/ubuntu-terminal-app/font-size-fix/+merge/285272
Your team Ubuntu Terminal Developers is subscribed to branch lp:ubuntu-terminal-app.