Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Custom/Embedded fonts #6013

Merged
merged 36 commits into from
Dec 16, 2019
Merged

Custom/Embedded fonts #6013

merged 36 commits into from
Dec 16, 2019

Conversation

Clancey
Copy link
Contributor

@Clancey Clancey commented Apr 25, 2019

Description of Change

This pull request unifies the way Custom Fonts are specified cross platform.

API Changes

Added:
New classes

  • ExportFontAttribute
  • EmbeddedFont
  • FontFile
  • FontRegistrar
  • IEmbeddedFontLoader
  • EmbeddedFontLoader (iOS, Android, and UWP)

Platforms Affected

  • Core/XAML (all platforms)
  • iOS
  • Android
  • UWP

Behavioral/Visual Changes

This is Backwards compatible. Users can continue to use fonts as they want. However it now allows us to embed fonts as Resources. This also allows us to declare custom fonts in a uniformed way.

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

"Lobster-Regular.ttf#Lobster-Regular" now works as just "Lobster-Regular"
UWP now supports `PTM55FT#PTMono`
"Lobster-Regular.ttf#Lobster-Regular" now works as just "Lobster-Regular"
UWP now supports `PTM55FT#PTMono`
@samhouts
Copy link
Member

@Clancey Any chance you could fix the conflicts? Thanks!

@jfversluis jfversluis mentioned this pull request Nov 5, 2019
2 tasks
var fontStream = GetEmbeddedResourceStream(foundFont.assembly, font);

var type = Registrar.Registered.GetHandlerType(typeof(EmbeddedFont));
var fontHandler = (IEmbeddedFontLoader)Activator.CreateInstance(type);
Copy link
Contributor

Choose a reason for hiding this comment

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

was noticing you're using the Activator here... for consistency I believe this should be using the DependencyResolver/DependencyService in Xamarin.Forms... also if someone for some insane reason decided to provide their own IEmbeddedFontLoader this would give them the ability to use DI...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what benefit this would give us, Right now, it gets the embedded font loader from the registrar. So you can still replace it if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

well I'm the first to admit I would have to ask what on earth someone is doing replacing the FontLoader... that said I just see this as a consistency issue. Anything other types that Xamarin.Forms may require can at the dev's choosing be generated by a DI container... this would explicitly prevent that.

Copy link
Contributor Author

@Clancey Clancey Nov 16, 2019

Choose a reason for hiding this comment

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

I am still wondering why this is considered an inconsistency issue. When you retrieve something from the Registrar, we always created it using activator. I am wondering when/where that changed? The the registrar is DI.. You can already swap things out whenever you want. You can register your own FontLoader, then it will load yours. Still failing to see an issue. This is the same thing as saying we should move all Renderers to DI instead of the registrar.

Copy link
Contributor

Choose a reason for hiding this comment

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

The consistency has to do with the refactoring that was done around the DependencyResolver to add support for those who want their DI Container to be the source of truth for resolving types. The intent was that the only place in the code base that Activator.CreateType should called should be in the DependencyResolver itself for the 99% of the time where people aren't using the hook.

@samhouts samhouts added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 20, 2019
@samhouts samhouts modified the milestones: 4.4.0, 4.5.0 Nov 22, 2019
@samhouts samhouts removed the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 22, 2019
@samhouts
Copy link
Member

@PureWeen The failing test is PriorityRegistrarTests.BasicTest. Would you mind checking out what we did to break that? Thanks!!

else
{
if (attribute.ShouldRegister())
Registered.Register(attribute.HandlerType, attribute.TargetType, attribute.SupportedVisuals);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Registered.Register(attribute.HandlerType, attribute.TargetType, attribute.SupportedVisuals);
Registered.Register(attribute.HandlerType, attribute.TargetType, attribute.SupportedVisuals, attribute.Priority);

@dansiegel
Copy link
Contributor

@samhouts just curious what's going on with this... would really love to be using this...

@samhouts
Copy link
Member

@dansiegel Assuming the build goes well, this will be merged into master for release in 4.5.0.

@samhouts samhouts merged commit 190dda8 into xamarin:master Dec 16, 2019
@dansiegel
Copy link
Contributor

Fantastic.. so glad to see this merged! thanks @samhouts

@geojorg
Copy link

geojorg commented Jan 14, 2020

I just try to run this new feature. But I can't see the fonts rendering.
In the AssemblyInfo.cs I add this
[assembly: ExportFont("Ubuntu-Regular.ttf",Alias ="Ubuntu")]

Then on my XAML using:
FontFamily="Ubuntu-Regular" or FontFamily="Ubuntu" or FontFamily="Ubuntu-Regular.ttf" i can't render the font.
The font is set as Embedded Resource.

Also on the release note:
https://docs.microsoft.com/en-us/xamarin/xamarin-forms/release-notes/4.5/4.5.0-pre1
it uses [assembly: RegisterFont("CuteFont-Regular.ttf")] but registerFont namespace does not exist.
Does this change the way you handle a Resource Dictionary?

@Clancey Clancey deleted the CustomFonts branch January 14, 2020 04:05
@Clancey
Copy link
Contributor Author

Clancey commented Jan 14, 2020

@geojorg #9194

@jimberg98
Copy link

jimberg98 commented Jan 20, 2020

Does this new feature do anything to deal with FontAttributes on Android? If, for example, you register fonts fontfamily-regular.ttf, fontfamily-bold.ttf, fontfamily-italic.ttf, and fontfamily-bolditalic.ttf, is there away to use just "fontfamily" as the name and then if you set FontAttributes to "Bold", will it select the correct font automatically? I've been trying to find a solution for this.

When you register a font resource, is there a way to tie it directly to specific FontAttributes? Are there any plans to add support for font weights?

I made this modification to Xamarin.Forms.Platform.Android.FontExtensions as a proof of concept that works great, and am wondering if there is anything that will work like this in the new version?

	static Typeface CreateTypeface(Tuple<string, FontAttributes> key)
	{
		Typeface result;
		var fontFamily = key.Item1;
		var fontAttribute = key.Item2;

		if (String.IsNullOrWhiteSpace(fontFamily))
		{
			var style = ToTypefaceStyle(fontAttribute);
			result = Typeface.Create(Typeface.Default, style);
		}
		else if (IsAssetFontFamily(fontFamily))
		{
			var fontAssetName = FontNameToFontFile(fontFamily);

			if (fontAttribute != default) 
				fontAssetName = AdjustFontAssetNameForFontAttributes(fontAssetName, fontAttribute);

			result = Typeface.CreateFromAsset(AApplication.Context.Assets, fontAssetName);
		}
		else
		{
			var style = ToTypefaceStyle(fontAttribute);
			result = Typeface.Create(fontFamily, style);
		}

		return result;
	}

	static string AdjustFontAssetNameForFontAttributes(string assetFileName, FontAttributes attributes)
	{
		if (!assetFileName.Contains("regular", StringComparison.OrdinalIgnoreCase)) return assetFileName;

		string style;
		switch (attributes)
		{
			default:
				return assetFileName;
			case (FontAttributes)((int)FontAttributes.Bold | (int)FontAttributes.Italic):
				style = "bolditalic";
				break;
			case FontAttributes.Bold:
				style = "bold";
				break;
			case FontAttributes.Italic:
				style = "italic";
				break;
		}

		var assetList = AApplication.Context.Assets.List("");
		if(assetList.Length==0) return assetFileName;
		var match = assetList.SingleOrDefault(fn => fn.Equals(fn.Replace("regular", style),StringComparison.OrdinalIgnoreCase));
		return !string.IsNullOrEmpty(match) ? match : assetFileName;
	}

Thanks, and if this is the wrong place, my apologies.

@jrahma
Copy link

jrahma commented Feb 12, 2020

It's not working for me when I try IconFont

Here for example:

<SyncfusionTabView:SfTabView x:Name="MainPageTabView" Grid.Row="0" DisplayMode="Image" TabHeaderPosition="Bottom" EnableSwiping="False" VisibleHeaderCount="5" SelectionChanged="MainPageTabView_SelectionChanged" TabItemTapped="MainPageTabView_TabItemTapped">
    <SyncfusionTabView:SfTabItem x:Name="tabHome" Title="الرئيسية" FontIconFontFamily="Taqyeem" IconFont="H" FontIconFontColor="Silver" />
    <SyncfusionTabView:SfTabItem x:Name="tabBrowse" Title="التقارير" FontIconFontFamily="Taqyeem" IconFont="R" FontIconFontColor="Silver" />
    <SyncfusionTabView:SfTabItem x:Name="tabBarCode" Title="التقييم" FontIconFontFamily="Taqyeem" IconFont="T" FontIconFontColor="Red" />
    <SyncfusionTabView:SfTabItem x:Name="tabSettings" Title="الإعدادات" FontIconFontFamily="Taqyeem" IconFont="S" FontIconFontColor="Silver" />
    <SyncfusionTabView:SfTabItem x:Name="tabAbout" Title="التطبيق" FontIconFontFamily="Taqyeem" IconFont="A" FontIconFontColor="Silver" />
</SyncfusionTabView:SfTabView>

I am setting IconFont for Syncfusion TabView

I added the Taqyeem.ttf to my shared project and set the Build to Embedded Resources

Then In the Shared resources also I added the following line to the AssemblyInfo.cs:

[assembly: ExportFont("Taqyeem.ttf")]
but I ma not getting the Images

Although I try the old way for custom fonts I get the images shown properly

@dansiegel
Copy link
Contributor

@jrahma there are some known issues... see #9194

@jrahma jrahma mentioned this pull request Feb 12, 2020
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API-change Heads-up to reviewers that this PR may contain an API change approved Has two approvals, no pending reviews, and no changes requested Core p/Android p/iOS 🍎 p/UWP roadmap t/enhancement ➕
Projects
None yet
Development

Successfully merging this pull request may close these issues.