Skip to content

Conversation

@kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Mar 2, 2024

Issues Fixed

Fixes #19556

Before After

@kubaflo kubaflo requested a review from a team as a code owner March 2, 2024 16:54
@ghost ghost added the community ✨ Community Contribution label Mar 2, 2024
@ghost
Copy link

ghost commented Mar 2, 2024

Hey there @kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@jsuarezruiz jsuarezruiz added area-fonts Custom fonts and Font related API's platform/android labels Mar 4, 2024
_ = App.WaitForElement("label");

// The test passes if fonts are correctly rendered
VerifyScreenshot();
Copy link
Contributor

Choose a reason for hiding this comment

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

Waiting CI to pass tests and generate the reference snapshot.

Copy link
Member

Choose a reason for hiding this comment

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

@jsuarezruiz how do we get the screenshot? this looks ok to merge now otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Right now, you need to go to the UITest run https://dev.azure.com/xamarin/public/_build/results?buildId=110953&view=ms.vss-test-web.build-test-results-tab&runId=2620968&resultId=100163&paneView=debug
And check the failed test .
Screenshot 2024-03-13 at 16 17 21

Then go to the Attachments and download the image for the test SystemFontsShouldRenderCorrectly
Screenshot 2024-03-13 at 16 18 18

Copy link
Member

Choose a reason for hiding this comment

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

SystemFontsShouldRenderCorrectly

@AlleSchonWeg
Copy link
Contributor

AlleSchonWeg commented Mar 4, 2024

Hi,
thank you for fixing my issue ;)
But this should work also on API < 28. I know you using a Typeface.Create which is only available above 28. In XF it works below 28. So i'm not sure if this is the correct overload.

Edit:
Sorry, i thought you implemented the API check. Below API 28 all works as it should. Inspired from your fix i tried to make it work for other "styles"

		Typeface? CreateTypeface((string? fontFamilyName, FontWeight weight, bool italic) fontData)
		{
			var (fontFamily, weight, italic) = fontData;
			fontFamily ??= string.Empty;
			var style = ToTypefaceStyle(weight, italic);

			var result = Typeface.Default;
			var isDefaultTypeface = false;
			if (!string.IsNullOrWhiteSpace(fontFamily))
			{
				if (LoadDefaultTypeface(fontFamily) is Typeface systemTypeface)
				{
					result = systemTypeface;
					isDefaultTypeface = true;
				}
				else if (GetFromAssets(fontFamily) is Typeface typeface)
					result = typeface;
				else
					result = Typeface.Create(fontFamily, style);
			}

			if (OperatingSystem.IsAndroidVersionAtLeast(28))
				result = Typeface.Create(result, (!isDefaultTypeface || result == null) ? (int)weight : result.Weight, italic);
			else
				result = Typeface.Create(result, style);

			return result;
		}

Thank you

@kubaflo
Copy link
Contributor Author

kubaflo commented Mar 4, 2024

@jonathanpeppers I've added a commit with your suggested changes. Thanks for the tips!

jonathanpeppers
jonathanpeppers previously approved these changes Mar 5, 2024
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

LGTM, if the test & screenshot are working. 👍

@rmarinho
Copy link
Member

/azp run MAUI-UITests-public

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rmarinho
Copy link
Member

/azp run MAUI-UITests-public

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rmarinho rmarinho merged commit a648f08 into dotnet:main Mar 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android Systemfonts (light/black etc.) not working

6 participants