Return FontImageSource from BaseIconExtension by default

[TiberiusDRAIG]

BaseIconExtension.ProvideValue(IServiceProvider) throws an exception if it is unable to determine the expected return type, which, on the face of things, makes sense. There are, however, some legitimate cases where the type isn't able to be determined using the current mechanism; for example:

<OnPlatform x:TypeArguments="ImageSource">
    <On Platform="MacCatalyst" Value="{cupertino:Icon Icon=Gear}"/>
    <On Platform="WinUI" Value="{segoeFluent:Icon Icon=Settings}"/>
</OnPlatform>

In this example, the extension method throws; by changing it to optimistically return a FontImageSource in this scenario, the OnPlatform stuff can easily be made to work.

My thinking behind this suggestion is that if the extension is going to throw an exception and explode anyway, it might as well pick a sensible default (in this case I think an ImageSource fits that bill) and let it explode later on. It saves chasing cases like this too, when I'd guess most consumers are expecting an ImageSource anyway.

Cool library anyway! Very helpful 👍

[AathifMahir]

BaseIconExtension.ProvideValue(IServiceProvider) throws an exception if it is unable to determine the expected return type, which, on the face of things, makes sense. There are, however, some legitimate cases where the type isn't able to be determined using the current mechanism; for example:

<OnPlatform x:TypeArguments="ImageSource">
    <On Platform="MacCatalyst" Value="{cupertino:Icon Icon=Gear}"/>
    <On Platform="WinUI" Value="{segoeFluent:Icon Icon=Settings}"/>
</OnPlatform>

In this example, the extension method throws; by changing it to optimistically return a FontImageSource in this scenario, the OnPlatform stuff can easily be made to work.

My thinking behind this suggestion is that if the extension is going to throw an exception and explode anyway, it might as well pick a sensible default (in this case I think an ImageSource fits that bill) and let it explode later on. It saves chasing cases like this too, when I'd guess most consumers are expecting an ImageSource anyway.

Cool library anyway! Very helpful 👍

Thanks for reporting this issue. Will fix or provide viable solution to this asap.

I'm currently in the process of rewriting library with v2, will test this scenario on the v2 as well.

[AathifMahir]

@TiberiusDRAIG How do you priortize this issue, I'm looking to fix and enhance this on v2, Do you think this should be atmost priority and should be available before v2, most probably on v1.2.2

[TiberiusDRAIG]

Entirely up to you - it's arguably a breaking change (default behavioural change) so I'd imagine it makes more sense to include in V2. In my case I've simply made my own version of the base extension with the new behaviour, so it's not blocking us at all 👍

[AathifMahir]

Entirely up to you - it's arguably a breaking change (default behavioural change) so I'd imagine it makes more sense to include in V2. In my case I've simply made my own version of the base extension with the new behaviour, so it's not blocking us at all 👍

Quick Question, As you mentioned that you made solution with extension, did that only applies to Image Control or Applies to Other Controls as well,

I'm currently prototyping with solution but getting the OnPlatform TypeArguments does need to go through lots of Reflection Gymnastics, Therefore looking into reliable solution on this

[TiberiusDRAIG]

Ahh, I probably wasn't clear enough; I've removed the NotSupportedException and instead, when the return type can't be determined, it just returns the FontImageSource - the thinking was that an ImageSource is a sensible default return type, and if it's not the right type it'll, at worst, result in an exception later on, which is what happens now when the type can't be determined anyway.

So, to be clear, I'm not detecting the TypeArguments at all - if the return type can't be determined, I'm just defaulting to returning FontImageSource instead of throwing an exception.

[AathifMahir]

Ahh, I probably wasn't clear enough; I've removed the NotSupportedException and instead, when the return type can't be determined, it just returns the FontImageSource - the thinking was that an ImageSource is a sensible default return type, and if it's not the right type it'll, at worst, result in an exception later on, which is what happens now when the type can't be determined anyway.

So, to be clear, I'm not detecting the TypeArguments at all - if the return type can't be determined, I'm just defaulting to returning FontImageSource instead of throwing an exception.

Sure, that's explains it. Thanks man.

I guess, we might need to support other controls too on this. I'm working on couple of prototypes to solve this, I hope we can implement viable solution that works on controls that already works with Current version Markup Extension.

[AathifMahir]

i'm going to close this issue since this issue is surpassed by #74