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

Layered font image source #9934

Closed
wants to merge 2 commits into from

Conversation

GalaxiaGuy
Copy link
Contributor

@GalaxiaGuy GalaxiaGuy commented Mar 11, 2020

Description of Change

The original FontIconSource spec (#3189) included a layered option. This is an implementation for iOS, Android and macOS. My plan is to add the other platforms, but I wanted confirmation this was still likely to be accepted before continuing.

API Changes

[ContentProperty("Layers")]
public class LayeredFontImageSource : ImageSource
{
	public IList<FontImageSource> Layers { get; set; }
	public Color Color { get; set; }
	public string FontFamily { get; set; }
	public string Glyph { get; set; }
	public double Size { get; set; }
}

-->

None

Platforms Affected

  • Core/XAML (all platforms)
  • iOS
  • macOS

Before/After Screenshots

Simulator Screen Shot - iPad Air (3rd generation) - 2020-03-11 at 13 08 34

Testing Procedure

Some layered icons have been added to the Font ImageSource gallery

Open questions

I've repurposed FontImageSourceHandler to handle layered and non-layered to share code easier. This does mean that the non-layered case allocates a List. Would it be better to keep them separate.

I initially thought FontFamily and Size were highly likely to be shared between layers, so adding them made sense. Color seemed less likely, but possible. Glyph seems unlikely, but allowing it to be shared wasn't any harder.

Should LayeredFontImageSource inherit from FontImageSource? It doesn't really make sense conceptually and would imply you could have a layer of layers. On the other hand it would be handy fallback if platforms don't get a proper implementation.

There currently isn't a reliable way to detect whether FontImageSource.Size has been set explicitly, or is the default. So if the user has LayeredFontImageSource.Size set to something other than 30, and then wants to override it with 30, that won't be handled correctly.

I could possibly do UWP.

On the subject of UWP, I'm not sure what LoadIconSourceAsync and LoadIconElementAsync should do.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@GalaxiaGuy
Copy link
Contributor Author

I've added an implementation for macOS. There is some odd clipping happening (that was also happening before) that I haven't been able to fix.

I've also changed the macOS implementation to support screen scaling.

@GalaxiaGuy
Copy link
Contributor Author

I've now added Android.
Screenshot_1585486172

@GalaxiaGuy GalaxiaGuy changed the title Layered font image source [WIP] Layered font image source Apr 5, 2020
@GalaxiaGuy
Copy link
Contributor Author

I've had issues getting Forms to compile on Windows, so I'm going to have to leave it at Android, iOS and macOS.

The UWP implementation should be straightforward.

Oliver Brown and others added 2 commits May 22, 2020 14:27
Squashed commits:
[e156bb7] Add macOS implementation.
[ccfd1f0] Remove superfluous property.
[245ed2a] Handle default size better.
[753e7f5] Fix tabs
[88c7fc9] Clean some things up. (+1 squashed commit)
Squashed commits:
[5bd9105] Add layered font image source with limplementation for iOS.
@samhouts samhouts requested review from hartez and rmarinho May 28, 2020 01:17
@samhouts samhouts changed the base branch from master to main June 26, 2020 00:06
@samhouts samhouts added this to In Review in .NET MAUI Backlog Jul 30, 2020
@samhouts samhouts removed this from In Review in vCurrent (4.8.0) Jul 30, 2020
@samhouts samhouts added API-change Heads-up to reviewers that this PR may contain an API change t/enhancement ➕ a/fonticon labels Aug 4, 2020
@jfversluis
Copy link
Member

Hey there @GalaxiaGuy, thank you so much for all your time and effort on this. It really is appreciated. For whatever reason this was left too long and now we're unable to still add this to Forms. If this is still something you'd love to see in .NET MAUI, please open a spec there so we can discuss and hopefully salvage this code and use it there.

Again, thank you!

@jfversluis jfversluis closed this Oct 8, 2021
.NET MAUI Backlog automation moved this from In Review to Closed Oct 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants