Add helper to load font#448
Conversation
|
Concept ACK. |
| return !selection.at(0).data(role).toString().isEmpty(); | ||
| } | ||
|
|
||
| void loadFont(const QString& file_name) |
There was a problem hiding this comment.
Why not return id for generality?
From Qt docs:
An ID is returned that can be used to remove the font again with
removeApplicationFont()or to retrieve the list of family names contained in the font.
There was a problem hiding this comment.
The helper is meant for our use case: load font; ensure it is valid; remains loaded while the application runs.
Happy to add the return value, but at the moment it is discarded at the call site.
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
Concept ACK
This PR introduces a new loadFont function, which along with adding the font, also asserts that it has been properly added to the QFontDatabase. I agree with the changes in the PR because this function eliminates any chance of font not being correctly added to the Application.
promag
force-pushed
the
2021-10-font-helper
branch
from
44f948f to
98f423d
Compare
promag
force-pushed
the
2021-10-font-helper
branch
from
98f423d to
d54ec27
Compare
stratospher
left a comment
stratospher
left a comment
There was a problem hiding this comment.
Tested ACK d54ec27. Refactoring the code and defining loadFont() in src/qt/guiutil.cpp reduces redundant imports of the QFontDatabase and is a better design.
Fonts in the application remain consistent in both the PR and master.
| Master | PR |
|---|---|
 |
 |
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
ACK d54ec27
Changes since my last review:
- The added function has been renamed from
loadFont->LoadFontaccording to the name convention - QStringLiteral macro is now used to ensure the input string is allocated statically.
- const keyword is used for the id variable to indicate that it is not modified.
- The fileName has been corrected to file_name in the comment.
|
Why do we want to abort if it fails?? |
5 participants
Originally submitted as bitcoin-core/gui-qml#49.