A suggestion on a new API design

[halipourian]

First of all I must say I can't thank you enough for all the time and effort that your MahApps projects had saved me in my projects.
I've got a suggestion on API design of this project. The main idea came to my mind by looking at how we use Brush, Brushes classes with Foreground property on a control. Through years I've learned that using generic classes along with xaml is not good idea... It will make headaches for the developer down the road... Like having you defined different styles with exactly same xaml code for different TKind in PackIconBase... The following classes are just for demonstration of the idea so it will be easier to understand.

Design

The first class stores the icon data (It's like Brush class or even Color class, read on and you'll find out why). It can even contain other properties like the preferred Brush to draw it with, which can be ignored when it is null (In IconPresenter control template) and the Foreground of IconPresenter would be used instead.

[ContentProperty("PathData")]
public class IconData : Freezable
{
    public static IconData Empty;

    static IconData()
    {
        Empty = new IconData();
    }

    public IconData()
    {
    }

    public IconData(string pathData)
    {
        PathData = pathData;
    }

    private static readonly DependencyProperty PathDataProperty = DependencyProperty.Register(
        nameof(PathData),
        typeof(string),
        typeof(IconData),
        new PropertyMetadata(null));

    [Bindable(true), Category("Appearance")]
    public string PathData
    {
        get => (string) GetValue(PathDataProperty);
        set => SetValue(PathDataProperty, value);
    }
        
    protected override Freezable CreateInstanceCore()
    {
        return new IconData();
    }
}

The next class is an example of our libraries of icons (like Brushes class but for Modern icons).

public static class ModernIcons
{
    private static readonly object CupcakeKey = new object();
    private static readonly object BeerKey = new object();

    private static readonly IDictionary<object, IconData> Icons = new Dictionary<object, IconData>
    {
        {CupcakeKey, new IconData("M50.67,22.17A3.16,3.16 0 0,1 53.83,25.33C53.83,25.91 53.68,26.45 53.41,26.92H53.83C55.58,26.92 57,28.33 57,30.08A3.17,3.17 0 0,1 53.83,33.25H55.42C57.17,33.25 58.58,34.67 58.58,36.42A3.16,3.16 0 0,1 55.42,39.58H23.75C22,39.58 20.58,38.17 20.58,36.42A3.17,3.17 0 0,1 23.75,33.25H25.33C23.58,33.25 22.17,31.83 22.17,30.08A3.16,3.16 0 0,1 25.33,26.92H25.76C25.5,26.45 25.33,25.91 25.33,25.33C25.33,23.58 26.75,22.17 28.5,22.17C30.08,22.17 33.25,19 39.58,17.42C45.92,22.17 49.08,22.17 50.67,22.17M23.75,41.17H55.42L50.67,57H28.5L23.75,41.17Z")},
        {BeerKey, new IconData("M24,20H49L49.08,26.92C53.46,26.92 57,30.46 57,34.83V42.75A7.92,7.92 0 0,1 49.08,50.67L49,59H24V20M49,46C50.75,46 53,43.75 53,42V36C53,34.25 50.75,31 49,31V46M27,23V25H46V23H27M46,56V54H27V56H46M30.5,28C29.63,28 29,29.13 29,30V49C29,49.87 29.63,51 30.5,51C31.37,51 32,49.87 32,49V30C32,29.13 31.37,28 30.5,28M36.5,28C35.63,28 35,29.13 35,30V49C35,49.87 35.63,51 36.5,51C37.37,51 38,49.96 38,49.08V30C38,29.13 37.37,28 36.5,28M42.5,28C41.63,28 41,29.13 41,30V49C41,49.87 41.63,51 42.5,51C43.37,51 44,49.87 44,49V30C44,29.13 43.37,28 42.5,28M23.75,17.23C25,17 31.67,13.67 33.25,15.25C34.83,16.83 36.42,16.83 38,16.83C39.58,16.83 41.17,15.25 44.33,15.25C45.92,15.25 49.08,16.04 49.08,17.63C49,21 47.9,18.81 47.9,18.81C47.9,18.81 47.1,17.23 43.94,17.23C40.77,17.23 39.58,18.81 38,18.81C36.42,18.81 33.65,17.23 30.5,17.23C27.31,17.23 26.88,18.83 24.94,18.42C23,18 21,20 21.77,22C23,24 22.17,25.28 22.17,27.5C22.17,27.92 21.75,27.88 21.38,25.94C21,24 19,23.17 19,21.58C19,20 19.17,19.83 20.58,18.42C22,17 22.5,17.46 23.75,17.23Z") }
    };

    public static IconData Cupcake => Icons[CupcakeKey];
    public static IconData Beer => Icons[BeerKey];
}

And the last one is the control which hosts the icon.

public class IconPresenter : Control
{
    private static readonly DependencyProperty DataProperty = DependencyProperty.Register(
        nameof(Data),
        typeof(IconData),
        typeof(Icon),
        new PropertyMetadata(null));

    [Bindable(true), Category("Appearance")]
    public IconData Data
    {
        get => (IconData) GetValue(DataProperty);
        set => SetValue(DataProperty, value);
    }
}

Usage

<icons:IconPresenter Data="{x:Static icons:ModernIcons.Cupcake}" />

In my initial search I couldn't find a way to implement some kind of converter in designer to use strings instead of '{x:Static icons:ModernIcons.Cupcake}' just like we use 'Red' in a foreground property. But even though I could find one, I would definitely not find a way to have intellisense for that.

The developer can even have their own icons defined and use it in the same manner by StaticResource.

<controls:IconData x:Key="CustomIcon">M21,16V14L13,9V3.5A1.5,1.5 0 0,0 11.5,2A1.5,1.5 0 0,0 10,3.5V9L2,14V16L10,13.5V19L8,20.5V22L11.5,21L15,22V20.5L13,19V13.5L21,16Z</controls:IconData>

And then

<icons:IconPresenter Data="{StaticResource CustomIcon}" />

Implementation

We can define an XSD file and a couple of corresponding XML files for each icon pack. Then have a T4 text template to generate classes. The resulting solution structure will be much cleaner and easier to maintain.

NuGet

We would have MahApps.Metro.IconPacks.Core which contains the IconData and IconPresenter classes. And a separate package for each icon pack like MahApps.Metro.IconPacks.Modern which contain classes like ModernIcons. And another package MahApps.Metro.IconPacks which have dependency on all of the mentioned packages.

Browser

For browser application compatibility we can use attributes on static properties of the icon pack classes (For name, description, tags etc.)

Advantages

1. Cleaner and simpler resulting solution structure which is easier to maintain
2. It is more resource dictionary friendly in expandability aspect.
3. We have just one IconPresenter class across application to have its default style set.
4. Developers can easily define preferred brushes for icons (They are not a simple shape).
5. Developers can use their custom icons along with our predefined icons in the same manner.
6. If developers wants to organize their used icons in an application, They can have their own icons repository class defined. And this way they can easily switch between icons for an specific purpose.

public static class ApplicationIcons
{
    private static readonly object AppIconKey = new object();
    private static readonly object NewKey = new object();

    private static readonly IDictionary<object, IconData> Icons = new Dictionary<object, IconData>
    {
        {AppIconKey, ModernIcons.Cupcake},
        {NewKey, MaterialIcons.PlusCircleOutline}
    };

    public static IconData AppIcon => Icons[AppIconKey];
    public static IconData New => Icons[NewKey];
}

Further discussion

We can even forget about the ModernIcons and similar classes and add resource dictionaries for different icon packs and add another property named BasedOn on IconData to help the developer have an alternative solution for the ApplicationIcons class discussed above (If you prefer xaml based solutions for UI problems). But I like it more in C# manner, you know, it's a library and other people shouldn't be able to modify it easily. But this doesn't mean having BasedOn property is wrong. And of course we should handle possible loops in BasedOn parents. Or maybe, we can have another class named Icon which is responsible for storing things like Fill brush along with IconData (I like this idea more).

Summary

So suggested classes and their responsibilities are as following:

1. IconData: Storing path data of the icon
2. MaterialIcons: Storing IconData of the known icons in each icon pack
3. IconPresenter: A control which shows the icon
4. Icon**: Storing other info about icon like Fill brush along with IconData. An IconData must be easily convertable to an Icon. If defined IconPresenter would use this instead of IconData. So it's kinda like Brush class and IconData is kinda like Color class.

** Additionally suggested, but it would be nice to have something like this.

And at the end I must say fixing FlowDirection in the default style (as it is now) for hosting controls is a bad idea. Sometimes we want the icon to adapt the hosting window's FlowDirection... For example an arrow for the GO button. By overriding its FlowDirection in the default style it doesn't inherit its value from its parent container anymore so it has to be maintained manually and that is definitely a problem. By using suggested IconData (Or Icon) class we can overcome this problem too, but again, I strongly advise against it to have it hard-coded inside the library (One can use the alternative suggested approach in ApplicationIcons class for their specific requirement)

I'm open to any kind of discussion/suggestion about this idea, and in ways which it is not practical. And of course I'm willing to help in implementing it because I don't think it takes more than couple of weekends.

[michael-hawker]

May be able to use CustomResource or a UWP Markup Extension to aid lookup too.