FontIcon markup extension

[Sergio0694]

This PR is part of the group being tracked in #3108.

Also includes changes originally from #3129 and #3130.

PR Type

What kind of change does this PR introduce?

• Feature

What is the new behavior?

Some XAML markup extensions to create FontIcon-s, FontIconSource-s and BitmapIcon-s directly from XAML with no code behind, resulting in much cleaner code (see here).
All additional info can be found in the linked issue.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link: #313
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

Other information

I'm going to need some help in setting the tests up, since they keep failing on my end (despite the fact that the extension in question has been used by a number of apps for years), and I'm wondering whether there are additional steps I missed in cloning/setting up the repository.
If anyone has a minute to double check, I'd appreciate it! 😊

Because of this, I'm only marking this PR as a draft for now.

EDIT: seems like the tests are actually passing in the CI. At this point I really think I'm just missing some configuration steps to get them to work on my end as well 😆

[ghost]

Thanks for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[Sergio0694]

I'm wondering whether it'd be a good idea to also add relaying properties to the other available FontIcon properties as , if we want APIs in the toolkit to be as flexible as possible.
In particular, those would be:

• FontSize
• FontStyle
• FontWeight
• IsTextScaleFactorEnabled
• MirroredWhenRightToLeft

Also, while I'm at it, I have 2 more equivalent markup extensions just like this one, but for FontIconSource (which is common in controls like the WinUI SwipeItems), and for BitmapIcon.

I personally think that having markup extensions for these 3 icon types makes it much easier to set things up in XAML, and it also makes the resulting code cleaner and easier to read.

@michael-hawker Should I open two PRs for those other 2 markup extensions as well? 😄

[michael-hawker]

@Sergio0694 exposing more properties probably isn't a bad idea, if anything FontSize is the most important.

Feel free to submit PRs for the other ones as well. you could add the FontIconSource to this one, or just create separate smaller PRs (easier to review that way). :)

[michael-hawker]

@Sergio0694 just to clarify our UnitTests don't run in our CI yet, I should open an issue to track that. I can try and pull this down and see if I can run the tests.

[michael-hawker]

@Sergio0694 I was able to run your tests locally. Think you had some copy-paste bugs. I didn't notice them on the first look too. I'll point them out as GitHub suggestions.

[Sergio0694]

@michael-hawker Added the two separate draft PRs, without tests for now:

• FontIconSource: FontIconSource markup extension #3129
• BitmapIcon: BitmapIcon markup extension #3130

[Sergio0694]

Ok this last failure is weird, all tests are passing for me locally:

I wonder if this is some .NET Native issue in Release with the unit test projects 🤔

EDIT: yeah it's failing for me in Release as well, it's not finding some types like BitmapIcon. Will try to add some .rd.xml directives to see if I can make that work.

EDIT 2: tried adding these to the .rd.xml file, didn't work. Still investigating.

<Type Name="Windows.UI.Xaml.Controls.FontIcon" Serialize="Required All" />
<Type Name="Windows.UI.Xaml.Controls.BitmapIcon" Serialize="Required All" />
<Type Name="UnitTests.Extensions.MockSwipeItem" Serialize="Required All" />

EDIT 3: also tried with Dynamic="Required All", same error. What's even weirder to me is that all of these types that are apparently being removed, like BitmapIcon, are actually being explicitly used by the code of the unit tests. Shouldn't this alone be enough to ensure they're not removed? 🤔

For instance, here:
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/24d8a1071b3a41b80d9bb009e4055f119981a24b/UnitTests/Extensions/Test_BitmapIconExtensionMarkupExtension.cs#L44
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/24d8a1071b3a41b80d9bb009e4055f119981a24b/UnitTests/Extensions/Test_FontIconExtensionMarkupExtension.cs#L33
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/24d8a1071b3a41b80d9bb009e4055f119981a24b/UnitTests/Extensions/Test_FontIconSourceExtensionMarkupExtension.cs#L31

[michael-hawker]

@azchohfi @MattWhilden any ideas here on why the rd.xml isn't working? Can we try including the whole assembly for the Unit Test project?

[michael-hawker]

@Sergio0694 this is all set now, eh?

For future reference, we had to add dummy copies of the instances in the Unit Test app's XAML file so that the proper XamlTypeInfo would be generated for use by the XamlReader loading code in the tests.

[Sergio0694]

@michael-hawker Yeah this should be good to go as well now! 😊

[Kyaa-dost]

Looking good to me 🚀