Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui: Update default font to Inter-Regular and console font to Inconsolata #1908

Merged
merged 4 commits into from
Oct 4, 2020

Conversation

opsinphark
Copy link
Contributor

@opsinphark opsinphark commented Oct 3, 2020

First run at importing assets from https://github.com/madmaxpayne/Gridcoin-wallet-redesign.

Adds Inter as the default font and Inconsolata as the RPC console font
The font SF Pro cannot be used according to Apple's legal policy. It has been replaced with Inter which uses the Open Font License.
More information about Inter can be found here:
https://fonts.google.com/specimen/Inter?query=inter

More information about Inconsolata can be found here:
https://fonts.google.com/specimen/Inconsolata?query=Inconsolata

For DPI scaling issues we might have to move to using % for font-size instead of px or pt.
px pixels (1px = 1/96th of 1in) (fixed width)
pt points (1pt = 1/72 of 1in) (fixed width)
% (sets to a percent of the parent element)

Both of these changes will make the font universal over all platforms as they are compiled into the binaries themselves. Hopefully this will provide some display consistency.

Note:
The stylesheets need to be reworked entirely at some point. These commits should satisfy the scope of this PR.

Screenshots:
grc-inter-debugconsole
grc-inter-peerstab
grc-inter-transactions

The Inconsolata font keeps the fixed width aspect for the RPC console:
grc-rpcconsole-inconsolata

Copy link
Member

@jamescowens jamescowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are missing native_stylesheet.qss, right?

I like moving towards the new assets.

src/qt/rpcconsole.cpp Outdated Show resolved Hide resolved
src/qt/res/stylesheets/dark_stylesheet.qss Outdated Show resolved Hide resolved
@opsinphark
Copy link
Contributor Author

I think you are missing native_stylesheet.qss, right?

I like moving towards the new assets.

@opsinphark
Copy link
Contributor Author

I think you are missing native_stylesheet.qss, right?
I like moving towards the new assets.

The native_stylesheet.qss is actively referenced anywhere in the code, it's merely there to provide a starting basis for a new stylesheet. It is beyond outdated and probably needs to be updated using the light or dark stylesheet.

@jamescowens
Copy link
Member

Are you sure about that? There is a native selection in the style drop down in the GUI, and it is definitely different than light.

@jamescowens
Copy link
Member

Further, native is actually better than light IMHO.

@jamescowens
Copy link
Member

The stylesheets are a mess quite frankly, and cycy and I didn't have time to fully fix them for the Fern release. If you want to take a shot at really getting them straight, that's great. Let's do it in a different PR than this though. Let's constrain this one to fix the main font + the console.

@jamescowens
Copy link
Member

Compiling your PR for testing now...

@opsinphark
Copy link
Contributor Author

opsinphark commented Oct 4, 2020

The stylesheets are a mess quite frankly, and cycy and I didn't have time to fully fix them for the Fern release. If you want to take a shot at really getting them straight, that's great. Let's do it in a different PR than this though. Let's constrain this one to fix the main font + the console.

Your right, it didn't see it loading at QT_ENABLE, it has bare minimum effects though. I will add the RPC font into the native stylesheet.

Yes, they are a mess. The styles are combined both in the C++ and stylesheets, so some elements might override the other.

After adding the RPC font to the native stylesheet all that is left is finding the right size for the RPC console icon.
Working on this now.

@jamescowens
Copy link
Member

I just tried this to no effect...

ui->messagesWidget->document()->setDefaultStyleSheet(
            "table { }"
            "td.time { font-family: Inconsolata; color: #808080; valign: bottom;} "
            "td.message { font-family: Inconsolata; valign: bottom; } "
            "td.cmd-request { color: #006060; } "
            "td.cmd-error { color: red; } "
            "b { color: #006060; } "
            );

@opsinphark
Copy link
Contributor Author

opsinphark commented Oct 4, 2020

I just tried this to no effect...

ui->messagesWidget->document()->setDefaultStyleSheet(
            "table { }"
            "td.time { font-family: Inconsolata; color: #808080; valign: bottom;} "
            "td.message { font-family: Inconsolata; valign: bottom; } "
            "td.cmd-request { color: #006060; } "
            "td.cmd-error { color: red; } "
            "b { color: #006060; } "
            );

I've added the RPC console font to the native stylesheet. This should fix all the issues beyond the icon size in the console.

SCRATCH THAT.
The native stylesheet does not work properly as it's missing several elements. It needs to be entirely reworked to include the elements from the 'light' & 'dark' stylesheets.

Will continue to work on this tomorrow. I've spotted a few other spots that have scaling issues. It's majorly frustrating to have three stylesheets that do not have the same elements.

src/qt/rpcconsole.cpp Outdated Show resolved Hide resolved
@jamescowens
Copy link
Member

Ok. I got the right font to show up in the console...
image

@jamescowens
Copy link
Member

I had to do

QFontDatabase::addApplicationFont(":/fonts/inconsolata-regular");
QFont messagesFont("inconsolata", 10);

ui->lineEdit->setFont(messagesFont);

ui->messagesWidget->document()->setDefaultFont(messagesFont);
ui->messagesWidget->document()->setDefaultStyleSheet(
            "table { }"
            "td.time { color: #808080; valign: bottom;} "
            "td.message { valign: bottom; } "
            "td.cmd-request { color: #006060; } "
            "td.cmd-error { color: red; } "
            "b { color: #006060; } "
            );

@jamescowens
Copy link
Member

The lineEdit is still not the right font...

Copy link
Member

@jamescowens jamescowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the requested changes. Thanks.

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved
src/qt/rpcconsole.cpp Outdated Show resolved Hide resolved
src/qt/rpcconsole.cpp Outdated Show resolved Hide resolved
@jamescowens
Copy link
Member

Ok. I saw your comments on making further changes to the stylesheets.

@jamescowens
Copy link
Member

Some other things to point out while you are at it where the NATIVE stylesheet is better...

Native...
image

Dark or Light...
image

Notice that the labels on the icons are truncated for dark or light, but the menu items are too close together for native. You are going to have to be very careful to ensure you get the best behavior across the board when you straighten these out. Hippocratic Oath applies too... do no harm...

@jamescowens
Copy link
Member

I got it to work correctly with all three styles, and the proportional font is the correct one too...

image

Please see jamescowens@376785c

@opsinphark
Copy link
Contributor Author

So here is where we are at, I think we both might be commenting to fast.
As this PR sits right now, the font is added correctly in bitcoingui.cpp & rpcconsole.cpp. The first issue is the native_stylesheet.qss has several missing elements and does not functional properly. The second issue is that of DPI scaling (which is why your screenshots appear truncated) . I am trying to track down both now.

Light or Dark: (both have the correct default and rpc console fonts)
grc-lightordark

Native: (has the correct default font but will not display the correct rpc font)
grc-native

@jamescowens
Copy link
Member

Alright. Let's see after you make further changes tomorrow. You can look at my branch if it helps at all.

@dblanch256
Copy link

dblanch256 commented Oct 4, 2020 via email

 - Adds the font Inconsolata-Regular to the application database.
   This font is licensed under the Open Font License.
   More information can be found here:
   https://fonts.google.com/specimen/Inconsolata?query=Inconsolata
 - Sets the global default font to 10pt rather than pixels to deal
   with any further DPI scaling issues.
 - Fixes missing elements in native_stylesheet.css that were
   not alllowing the RPC font to display correctly in the RPC
   console.
   (The stylesheets really need to be reworked entirely although
    this satisfies the scope of this PR)
@opsinphark
Copy link
Contributor Author

All stylesheets have been fixed and are displaying both fonts properly.
The stylesheets need to be reworked entirely, although these commits satisfy the scope of this pull request.
If you pulling from my repository for test building purposes all my changes have been squashed and forced updated so you probably will have to use git pull --force to receive all of the changes.
I have statically built and tested for x86_64-pc-linux-gnu. I can provide the binaries if necessary.

@jamescowens
Copy link
Member

@dblanch256 that is an ignorable issue that has been reported several times, thanks. Someone is trying to inject a v2 (block version 11 transaction too early with fields that are out of range in the v10 protocol. The wallet is appropriate rejecting the transactions even though the error message is cryptic. BTW, normally you create a new issue for something like that, rather than mix in to an unrelated one... :)

@jamescowens
Copy link
Member

@opsinphark I will build from your PR here...

@opsinphark
Copy link
Contributor Author

Still noticing with Hi-DPI turned on the light and dark stylesheets still have truncated items.
Trying to track the elements.

@jamescowens
Copy link
Member

The font still wasn't right on my build. Please see this...
image
wrong console font png

@jamescowens
Copy link
Member

I made a couple of changes. It works now (at least the fonts, not the other issues you mentioned above)...

@jamescowens
Copy link
Member

jamescowens commented Oct 4, 2020

image

This is the correct font.

@jamescowens
Copy link
Member

I will PR to your branch...

@jamescowens
Copy link
Member

See opsinphark#1

You have to register both fonts in the bitcoingui.cpp file, because registering it in the rpcconsole.cpp file is too late. The main style sheets have already been processed. Also you need to use the alias names not the actual file names for the resources.

Modifications to PR1908 to get fonts to initialize correctly
@jamescowens
Copy link
Member

BTW, I agree wholeheartedly with you on the DPI scaling. I actually put a DPI ratio to 96 in a few places to scale the icons correctly. We need to adopt a general scaling approach I think. There is also the HiPDI switch or something with the newer Qt. Might want to look into that. Not sure how that works.

Copy link
Member

@jamescowens jamescowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@jamescowens
Copy link
Member

I will go ahead and merge this one, and we can start working on straightening out the rest of the stylesheet problems and HiDPI issues on a new one.

@jamescowens jamescowens changed the title gui: Update default font to Inter-Regular gui: Update default font to Inter-Regular and console font to Inconsolata Oct 4, 2020
@jamescowens jamescowens added this to the Gladys milestone Oct 4, 2020
@jamescowens jamescowens added the gui label Oct 4, 2020
@jamescowens jamescowens merged commit 3b7a596 into gridcoin-community:development Oct 4, 2020
@jamescowens jamescowens linked an issue Oct 4, 2020 that may be closed by this pull request
@jamescowens
Copy link
Member

It was the border: none; settings that was causing the truncation in the toolbar icon labels. Go figure. I think it is a bug. See my PR that is the continuation of the fixup... #1911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console and Icons Don't Scale On High DPI Displays
3 participants