qt: Implement application wide font management #3555

xdustinface · 2020-06-26T14:09:36Z

This PR ist part of a series of +-25 PRs related to UI redesigns. Its ancestor is #3554, its successor is #3556. I did not screenshot every single PR and its changes, instead i made "walk through all screen" videos with the result of this PR series and also with the 0.15 UI. If there are any concrete screenshots wanted, just let me know. To build with the full set of changes you can build from the branch xdustinface:pr-ui-redesign which always contains all changes.

-> Walk through 0.15
-> Walk through Redesign

I tried to give the commits enough text to make things obvious without a lot description for each PR. Also here, if you want more description for this specific PR, let me know.

About this PR

This PR adds custom font helpers in GUIUtil and makes Montserrat (a font from the "Dash Style Guide") the default font for dash themes.

All PRs are based on its ancestor. Click here to see the changes of this PR only

UdjinM6 · 2020-06-28T13:23:45Z

Needs rebase + see #3578 for some font related issues

PastaPastaPasta

can you run git diff -U0 HEAD~13.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v (that 13 will probably change after you rebase) and then take a look at those changes. Some of them you might not need / want to take

UdjinM6 · 2020-06-28T17:51:25Z

Tested this a bit. It looks like -font-* cmd-line options have very limited effect atm, I guess that's going to be the case up until #3567. Also, I agree with @strophy that system default font should be the default one in the app, not the Monserrat one which could be an optional one but there is no way to select one or another until #3568.

I think it might probably make sense to postpone this and/or maybe group all font related PRs (i.e. this one, #3567 + defining defaultFontFamily = GUIUtil::FontFamily::SystemDefault and font selection parts of #3568) into one single PR.

xdustinface · 2020-06-30T19:43:50Z

@PastaPastaPasta thanks 👍 good to know that we have a script for that 😄 i applied some of the changes.

@UdjinM6 Rebased on develop + rebased/merged all the related commits into this PR and added -font-family to round this PR up. 0 -> SystemDefault is the default now there.

@UdjinM6 @strophy I used Montserrat because its defined in Dash Style Guide (page 14) to be used as main typeface for most web and print. I assumed we are not too far away from that with the core wallet ;)

Montserrat is our main typeface in the weights shown.
Use this typeface for most web and print communications (titles, body text, etc).

Anyway, i get your points of some people rather want the native thing as default. Though, i have another point here because i prefer the non-native/dash-specific default approach rather. I just prefer the cleaner/modern look of the Montserrat which stands out over the system fonts and i also can't confirm from my side that "its hard to read" it. Maybe thats some windows thing or so, idk. So i would really want to have Montserrat as default haha... But at the end thats just personal preference and thats why i propose to add a "Setup your preferred settings" dialog. The idea is just to have a simple dialog which looks similar to the Appearance tab and it will pop up after starting the application if there are no font related settings yet. Means this would pop up in the following cases:

Very first run of DashCore (could also be part of the "Intro" dialog)
First run of DashCore with the UI redesign
When running with -resetguisettings

This way you can choose your own defaults for all of the font/theme related settings. Just like you can if you for example start with macOS (maybe Windows too? idk) configuration or so. Would either add this to the Appearance tab PR or just have a separate one for it not sure yet.

strophy · 2020-06-30T22:28:08Z

Suba's interpretation of the style guide here (under "Visual") indicates Montserrat should only be used for headings, Open Sans should be used for body text. This is what I have followed when laying out e.g. our PDF legal briefs.

Fonts are such a subjective issue, and I totally support giving users a choice. The experience depends so much on user's platform as well. But the change from system default as default to Montserrat as default feels like it presumes to know what the users wants - for me personally the characters are far too wide for it to be a good choice for body text or data in tables. The kerning issue is probably also turning me off. So I think it's a good compromise to offer the user a choice during first install, maybe hidden behind an "Advanced Options" button or something (since the user should really be focusing on where to store the blockchain data and wallet during first install).

xdustinface · 2020-07-01T06:13:47Z

Suba's interpretation of the style guide here (under "Visual") indicates Montserrat should only be used for headings, Open Sans should be used for body text. This is what I have followed when laying out e.g. our PDF legal briefs.

for me personally the characters are far too wide for it to be a good choice for body text or data in tables.

Well as you can see in my cite of the pdf guide, it says Montserrat is good for body texts, this is what i have followed. Seems like we have some inconsistency issue then with our style guides. Also i presented Suba the UI changes and even asked him what he thinks about the font choice, size and weight. He didn't complain about it at that point ;) So yeah, some people seem to like it, some not and thats totally fine.

But the change from system default as default to Montserrat as default feels like it presumes to know what the users wants

Check my last comment, it's not longer default now with the latest changes to this PR. So, should be good?

The kerning issue is probably also turning me off.

Just for me to understand this, doesn't it just come up if you change the font from Montserrat to SystemDefault?

So I think it's a good compromise to offer the user a choice during first install, maybe hidden behind an "Advanced Options" button or something (since the user should really be focusing on where to store the blockchain data and wallet during first install).

No worries, it will not distract the user from choosing where to store the data since that will be done already then. Could be an "Advanced Options" button or what i had in mind actually is just a second page for the initial setup.

strophy · 2020-07-02T01:00:10Z

Yup, we have definitely found a discrepancy in the style guide ;) I'm fine with the changes as you describe them and I think we can go ahead with this PR. Regarding the kerning, I'm not sure this is a new problem, it occurs with both fonts and I think it occurs with older versions as well. So we can probably continue discussion on these Windows-with-mixed-normal/HiDPI-panel-only problems in #3578

UdjinM6 · 2020-07-02T01:06:43Z

Few suggestions, pls see https://github.com/UdjinM6/dash/commits/pr3555

xdustinface · 2020-07-02T14:29:37Z

@strophy sounds good 👍

@UdjinM6 Thanks, nice suggestions 🙂 picked them all!

UdjinM6

Slightly tested ACK

PastaPastaPasta

some things

src/qt/dash.cpp

src/qt/guiutil.cpp

src/qt/askpassphrasedialog.cpp

PastaPastaPasta · 2020-07-06T17:11:26Z

Generally looks good, can you squash some of these commits? 25 commits is quite a bit to merge, but this PR seems too big to merge as one commit

Includes the following methods - GUIUtil::loadFonts to load the fonts into the application's font database - GUIUtil::setApplicationFont to set the application font depending on the active theme - GUIUtil::setFont to set a specific font variation for a vector of widgets (Montserrat for dash themes, system default for traditional theme). - GUIUtil::getFont to get a specific montserrat variation as QFont (Montserrat for dash themes, system default for traditional theme). - GUIUtil::getFontWeightNormal to get the font weight to be used for normal text - GUIUtil::getFontWeightBold to get the font weight to be used for bolder text - GUIUtil::getFontNormal to get the default normal weighted QFont - GUIUtil::getFontBold to get the default bold weighted QFont There is a Qt bug on macOS (at least with macOS 10.15.4 and Qt 5.7.1) which leads to not mapping the font weights correctly to the different font types so that they can be selected by the appropriate methods to set the font weight. As result there is now way to properly select the different font styles from stylesheet or with QFont::setWeight() for the font Montserrat. There are only two different weights selectable over the full range of possible weight values. One very bold one and the "ExtraLight" version. As workaround its possible to select the fonts by using the font style string (QFont::setStyleName) as the Montserrat font becomes loaded as one family with one "font style" for each font file. The font wrapper added by this commit are taking care of this behaviour for all operating systems so there should be no other usage of QFont outside of GUIUtil and no more font changes in stylesheets.

This is because font related changes will from now on only be made either in c++ with `GUIUtil::setFont` for font weight/style changes or in css for size and color changes with `font-size` or `color`.

The two parent commits did remove all some font-weight changes from css and ui files. This commit reverts them.

-font-weight-normal allows to set the weight of normal texts in the UI -font-weight-bold allows to set the weight of bold texts in the UI -font-scale allows to scale the application font size up/down

This also keeps track of changed font-sizes and updates them depending on the settings if GUIUtil::updateFonts() gets called.

Allows to choose between the fonts: 0 - SystemDefault (default) 1 - Montserrat

xdustinface · 2020-07-06T21:48:16Z

Ok squashed all review/fix stuff into the related commits, combined some of the original ones and added one small change with 172c283. All good now?

UdjinM6

Slightly tested ACK

PastaPastaPasta

On ubuntu right now it looks pretty bad... maybe this is due to using system font?

Generally it looks like stuff still works. light ACK, code review utACK

qt: Implement application wide font management

This was referenced Jun 26, 2020

qt: Disable macOS system focus rectangles for dash themes #3556

Merged

qt: Redesign of the main toolbar #3554

Merged

xdustinface force-pushed the pr-ui-2-fonts branch 2 times, most recently from f3c5deb to b2eccfe Compare June 27, 2020 20:22

PastaPastaPasta requested changes Jun 28, 2020

View reviewed changes

xdustinface force-pushed the pr-ui-2-fonts branch 3 times, most recently from 8821df3 to a832540 Compare June 30, 2020 19:34

xdustinface changed the title ~~qt: Add Montserrat as custom application font~~ qt: Implement application wide font management Jun 30, 2020

This was referenced Jun 30, 2020

qt: Move font-size settings from css/code to code only #3567

Closed

Qt redesign issues #3578

Open

UdjinM6 previously approved these changes Jul 3, 2020

View reviewed changes

UdjinM6 added this to the 17 milestone Jul 3, 2020

UdjinM6 requested a review from PastaPastaPasta July 3, 2020 11:08

PastaPastaPasta requested changes Jul 6, 2020

View reviewed changes

xdustinface dismissed UdjinM6’s stale review via 9a1c07a July 6, 2020 12:01

xdustinface added 5 commits July 6, 2020 23:18

qt: Use default montserrat instead of fixed pitch font in dash themes

3f80233

qt: Removed all font related changes from .ui/.css files

27bede7

This is because font related changes will from now on only be made either in c++ with `GUIUtil::setFont` for font weight/style changes or in css for size and color changes with `font-size` or `color`.

qt: Set a proper font weight with GUIUtil::setFont where requried

f94dfbe

The two parent commits did remove all some font-weight changes from css and ui files. This commit reverts them.

qt: Use correct default font for size calculations

68a3250

xdustinface added 9 commits July 6, 2020 23:24

qt: Load custom font in splashscreen

2bbc6b8

qt: Use default font in receive requests and transaction description

2c7f8db

qt: Add -font-weight-normal, -font-weight-bold and -font-scale arguments

19a93e9

-font-weight-normal allows to set the weight of normal texts in the UI -font-weight-bold allows to set the weight of bold texts in the UI -font-scale allows to scale the application font size up/down

qt: Keep track of font updates

c0056c5

qt: Move font-size settings from css to c++.

687a0e6

This also keeps track of changed font-sizes and updates them depending on the settings if GUIUtil::updateFonts() gets called.

qt: Introduce GUIUtil::FontWeight as mapper for QFont::Weight

85c353e

qt: Introduce GUIUtil::FontFamily and make font family adjustable

175c3e0

qt: Add -font-family command line parameter

6e36764

Allows to choose between the fonts: 0 - SystemDefault (default) 1 - Montserrat

qt: Make font related variables private in GUIUtil's namespace

172c283

xdustinface force-pushed the pr-ui-2-fonts branch from 9a1c07a to 172c283 Compare July 6, 2020 21:44

UdjinM6 approved these changes Jul 6, 2020

View reviewed changes

PastaPastaPasta approved these changes Jul 7, 2020

View reviewed changes

UdjinM6 merged commit ab9a8ee into dashpay:develop Jul 7, 2020

xdustinface mentioned this pull request Jul 8, 2020

qt: Call GUIUtil::loadFonts earlier #3593

Merged

xdustinface pushed a commit to xdustinface/dash that referenced this pull request Sep 1, 2020

Merge pull request dashpay#3555 from xdustinface/pr-ui-2-fonts

d296cbd

qt: Implement application wide font management

UdjinM6 modified the milestones: 17, 16 Sep 4, 2020

qt: Implement application wide font management #3555

qt: Implement application wide font management #3555

Uh oh!

Conversation

xdustinface commented Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

About this PR

Uh oh!

UdjinM6 commented Jun 28, 2020

Uh oh!

PastaPastaPasta left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UdjinM6 commented Jun 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xdustinface commented Jun 30, 2020

Uh oh!

strophy commented Jun 30, 2020

Uh oh!

xdustinface commented Jul 1, 2020

Uh oh!

strophy commented Jul 2, 2020

Uh oh!

UdjinM6 commented Jul 2, 2020

Uh oh!

xdustinface commented Jul 2, 2020

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PastaPastaPasta commented Jul 6, 2020

Uh oh!

xdustinface commented Jul 6, 2020

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xdustinface commented Jun 26, 2020 •

edited

Loading

PastaPastaPasta left a comment •

edited

Loading

UdjinM6 commented Jun 28, 2020 •

edited

Loading