-
Notifications
You must be signed in to change notification settings - Fork 313
Choose monospaced font in C++ code rather in *.ui
file
#87
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
Conversation
@jonasschnelli Should this be considered a bugfix? |
Unsure about this. |
How is it so? I think both |
I'd say *.ui is view, *.cpp is controller (the non-automatic generated ones). |
Right. But Qt programming usually assumes "the view and the controller objects are combined" (https://doc.qt.io/qt-5/model-view-programming.html). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, one change requested
src/qt/overviewpage.cpp
Outdated
|
||
QFont f = GUIUtil::fixedPitchFont(); | ||
f.setWeight(QFont::Bold); | ||
f.setPointSize(10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not override this for no reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason is to make things explicitly rather than implicitly. Does setPointSize
call has any downsides?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the downside is that it is explicit and overrides the user's preference (which may not be 10pt).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Updated.
Now I'm able to change the font size with the qt5ct
utility on Linux Mint 20 :)
This change makes macOS choose the correct monospaced font.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding |
Closed in favor of #207 which should be less controversial. |
This change makes macOS choose the correct monospaced font.
fanquake noted (bitcoin/bitcoin#16432 (comment)):
... because it is not monospaced.
This is an alternative to #79 as that PR has a Concept NACK from luke-jr.
I'm personally still lean to embedding font as it makes the GUI more stable.