Fonts: make fonts part of InfiniTimeTheme#2427
Fonts: make fonts part of InfiniTimeTheme#2427muesli wants to merge 1 commit intoInfiniTimeOrg:mainfrom
Conversation
Added a Fonts namespace to the InfiniTimeTheme, which provides references to the available standard fonts. Instead of directly referencing fonts in various code places, we can now rely on the Theme's settings. This makes it easier to try out different fonts and have the setting applied system-wide.
|
Build size and comparison to main:
|
mark9064
left a comment
mark9064
left a comment
There was a problem hiding this comment.
A long needed refactor :) thanks
| }; | ||
|
|
||
| namespace Fonts { | ||
| static constexpr const lv_font_t* normal = &jetbrains_mono_bold_20; |
There was a problem hiding this comment.
There's the option of something like static constexpr const lv_font_t &normal = jetbrains_mono_bold_20; to keep the type the same. But maybe it's better as ref anyway?
| }; | ||
|
|
||
| namespace Fonts { | ||
| static constexpr const lv_font_t* normal = &jetbrains_mono_bold_20; |
There was a problem hiding this comment.
I feel these should be capitalised (I know that disagrees with the entries above)
I don't think we have a clear standard for these kind of constants
Not something that needs changing to merge. Interested in everyone's opinions on this one, could be totally off base here :)
There was a problem hiding this comment.
I absolutely agree! Even felt weird writing them in lowercase, but I wanted to follow the convention from above.
There was a problem hiding this comment.
@JF002 do you have a defined convention in mind? I don't mind too much either way, but we should probably add it to the coding standard
2 participants
Added a Fonts namespace to the InfiniTimeTheme, which provides references to the available standard fonts. Instead of directly referencing fonts in various code places, we can now rely on the Theme's settings. This makes it easier to try out different fonts and have the setting applied system-wide.