-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[Android] Ensure custom fonts also get weight/slant #1011
Conversation
@@ -79,27 +81,29 @@ public FontManager(IFontRegistrar fontRegistrar, ILogger<FontManager>? logger = | |||
foreach (var folder in folders) | |||
{ | |||
formated = $"{folder}{fontFile.FileNameWithExtension()}#{fontFile.PostScriptName}"; | |||
var result = LoadTypefaceFromAsset(formated); | |||
var result = LoadTypefaceFromAsset(formated, false); |
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.
Warnings at each step of the search are not needed.
this fixed the android tests ,but now we have 3 tests failing on iOS |
Yeah. I found out why... On all platforms in the universe, they can make a "regular" font bold, but iOS just can't. Just updating some tests and will push. |
//TODO: Investigate making this Async | ||
(bool hasFont, string? fontPath) HasFont(string font); | ||
// TODO: this should be async as it involves copying files | ||
string? GetFont(string 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.
There is NO reason to have these two in a tuple - we literally always have false,null and true,notnull. I even had an extension method that converted from a tuple into a single value... I am not sure my original thought process - might have been for 100% backwards compat. But that is not worth it.
bool CanUsePackagedNativeFonts => | ||
#if WINDOWS | ||
true; | ||
#else | ||
false; | ||
#endif |
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.
This is genuinely cause for concern for all the other devs out there. Which dummy wrote this instead of using the partial classes to actually do different logic. Me. 😭
Description of Change
Seems that custom fonts never got the weight and slant applied as well as for the "system" fonts, bold was not applied.
However, all is not perfect because of #1018