Xamarin.forms: [Bug] Xamarin Forms Styles - Support multiple target types

[CherryBu]

I have recently updated to very latest Xamarin forms pre release 4.2 version. One notable breaking change I have encounter is - Say I have the following Style:

In previous versions, same target "Label" was supported for both Span and Labels. Like - this was working before:

Same style targeted for Label was support on Span as well. However now in new version it doesn't.

My Question is: Can we support Both Label and Span with same style? Can we not target same style for both? Like I tried the following but it doesn't compile:

Please help me. I can copy paste the style and make 2 different styles however; if there's a some other way?

• Version with issue:
• Last known good version:
• IDE:
• Platform Target Frameworks:
  
  
  
  • iOS:
  
  
  • Android:
  
  
  • UWP:
  
  
• Android Support Library Version:
• Nuget Packages:
• Affected Devices:

Reproduction Link

https://stackoverflow.com/questions/57083760/xamarin-forms-styles-support-multiple-target-types

[jsuarezruiz]

I have created a small sample where I can effectively reproduce the problem.
Test6907.zip

After reviewing code and PRs, I think I have found the change and where is the problem: https://github.com/xamarin/Xamarin.Forms/pull/6677/files#diff-3168ac6369eedd3489bbe6dba62d5c36R43

Let's review it.

[andreinitescu]

@jsuarezruiz Hi Javier, thanks for looking into this.

Similar to the code on StackOverflow, in your repro, you set the built-in BodyStyle which has TargetType=Label to a Span:

                    <Span Text="default, "
                          Style="{StaticResource BodyStyle}">

This worked before #6677 because there was no check for types which allowed applying styles to completely unrelated targets, just because the property existed in both the Style target and the target, for example you could apply a Page style to a Button!

            <Style x:Key="PageStyle1"
                   TargetType="Page">
                <Setter Property="Padding"
                        Value="10" />
            </Style>

            <Button Style="{StaticResource PageStyle1}" />

I understand that the new behavior can be seen as 'breaking' but if we want #6677 as it is, the developer code was just running wild.

Now, it all depends how we want to handle this. The idea of being able to use a Style with a completely different target than the target which the Style is applied to, can be interesting (similar to how CSS works), but it's kind of weird in Xamarin.Forms context because of the mandatory TargetType attribute on Style. It doesn't make sense to have a TargetType unrelated to the target you apply the Style to.

[andreinitescu]

Here's another option: https://github.com/andreinitescu/Xamarin.Forms/commit/bbd34613017042d4c7c2f80507f88a581ff2258f

I reverted to not checking for type compatibility (how it was before #6677) and instead check when exception rises within Style.Apply()

However, I'm not so sure about this. I think #6677 is still better. Note that BasedOn actually does check for type compatibility, so I think so should TargetType, which what #6677 does

[maxim-saplin]

Can we have TargetType validation working at least for descendant types, not the exact match as it is now?
E.g. I have created a custom label class by deriving from Label and have single set of label styles in my dictionary. With 4.2 I have the app broken and need to duplicate all Label styles both in resource dictionary and across all XAML files - doesn't make sense.

[IoTFier]

I am facing same issue.
In my XAML I am using Label with FormattedString that provides multiple elements.

@samhouts , @jsuarezruiz , @andreinitescu -
1> is there a document that list the style types that are checked / not backward compatible like the one in this issue ?
2> will it be possible to get all these debug time errors in the build process instead of weeding it out one at a time during debug ?

Thank you

[ianvink]

This is really really bad for us. Control makers like Grail use the Styles across elements extensively. 4.2 completely breaks the entire ecosystem. We suddenly can not upgrade as core components will not upgrade. https://grialkit.com @samhouts

[PureWeen]

@ianvink what's the language of the exception you are seeing with GrialKit? This exception should only be raised if you're applying a Style with a TargetType that completely doesn't match the element being applied to

For example

 <ContentPage.Resources>
        <ResourceDictionary>
            <Style x:Key="labelRedStyle" TargetType="Label">
                <Setter Property="TextColor" Value="Red" />
            </Style>
        </ResourceDictionary>
    </ContentPage.Resources>
    <StackLayout>
        <local:CustomLabel Style="{StaticResource labelRedStyle}"  Text="I work fine"  />
        <Button Style="{StaticResource labelRedStyle}"  Text="I will crash"  />
    </StackLayout>

So if you have styles applied to Label and a derived type of CustomLabel then you can still apply the Label Style to the Custom Label

[samhouts]

I am facing same issue.
In my XAML I am using Label with FormattedString that provides multiple elements.

@samhouts Samantha Houts FTE , @jsuarezruiz Javier Suárez Ruiz FTE , @andreinitescu -
1> is there a document that list the style types that are checked / not backward compatible like the one in this issue ?
2> will it be possible to get all these debug time errors in the build process instead of weeding it out one at a time during debug ?

Thank you

We had planned to implement https://github.com/xamarin/Xamarin.Forms/issues/6689 to give compile-time warnings/errors.

[ianvink]

The way it works is you provide, for example, their TabControl with a style of targettype TabControl. Then internally they create Grids for each Tab and apply that style. Boom, because the Tab's style TargetType is not Grid. They do this all over the place. We'd need to completely drop GrailKit from our app to make it work. That would be not a very nice day given how foundational controls like GrailKit and Telerik are to us.

The fluidness of Styles has been around so long that, though it may not be good for [[insert reason here]], it became fundamental to the way a great number of apps, and the ecosystem of controls, are built.

[IoTFier]

I am facing same issue.
In my XAML I am using Label with FormattedString that provides multiple elements.
@samhouts Samantha Houts FTE , @jsuarezruiz Javier Suárez Ruiz FTE , @andreinitescu -
1> is there a document that list the style types that are checked / not backward compatible like the one in this issue ?
2> will it be possible to get all these debug time errors in the build process instead of weeding it out one at a time during debug ?
Thank you

We had planned to implement #6689 to give compile-time warnings/errors.

Hi @samhouts , I am using syncfusion, grialKit and some 3rd party controls, plugins.
With the explanation of @ianvink in above message, I am bit skeptical on upgrading my App to 4.2 - especially it is not showing all errors at compile time and I don't know what else is going to break based on our 3rd party dependencies.

Any help / guidelines for us to address it smoothly ?
Thank you

[samhouts]

We're looking at options now, and whatever we plan to do to resolve this issue, we'll have a release available within the next couple of weeks. If you're apprehensive about upgrading at this time, I would wait until the first service release is available. Thanks!

[ianvink]

@PureWeen The error is the same, caused by applying a Style TargetType of ABC control but using it on EDF control. Grail, and other control makers, use this design pattern extensively. Grail, for example, applies the TabControl Style to internally created Grids. Consumers of their TabControl can't opt-out, as cross-control styles are foundational now. It just crashes.

The eco system has evolved to depend on being able to fluidly use Styles, like CSS in the web world, without rigidity. 4.2 breaks the eco-system of Styles at a fundamental level.

Emotionally Supportive Note: I love Xamarin Forms and the people who dedicate so much to it.

[samhouts]

@ianvink We've discussed this issue with Grial, and they are releasing an update that will resolve the exceptions.

We're looking at adding the compile-time errors now to make the conversion for other apps and controls easier.

We're also reaching out to other vendors to see if this has been reported to them.

Thanks for your patience!

[IoTFier]

@ianvink We've discussed this issue with Grial, and they are releasing an update that will resolve the exceptions.

We're looking at adding the compile-time errors now to make the conversion for other apps and controls easier.

We're also reaching out to other vendors to see if this has been reported to them.

Thanks for your patience!

Hi @samhouts , thank you for looking out for us.
Can you share with us the list of vendors and any other open source providers e.g. [https://github.com/rotorgames/Rg.Plugins.Popup] , FFImageloading , mFractor that you are reaching out to ? If applicable, we could add to that list any of our dependencies, if missing.

Compile-time errors will be really great if also complemented with suggestion on what may be alternative fixes.
In my example - a style defined for label is resulting in an error as incompatible with element target type xamarin.forms.span while I am using Label with FormattedString and Span elements.
I have no clue why that should be failing in XF 4.2 when it has been working up until 4.1

Also, if compilation were to result in failing for one issue at a time, then, dev cycle will need to factor in a lot more time for opening and closing of a github issue to address many instances like that.

I wish there was a part of XF regression that could validate all new pre and GA releases against say top 10-20 OSS and/or commercial vendors/partners repos, controls used by Xamarin Forms developer community from startup to enterprises.

Thoughts?
Thanks

[ianvink]

Grail 2.7.7 addresses this issue and works fine.

[IoTFier]

Hi @samhouts , any ETA for this issue ?

Thank you

[samhouts]

I think the intention of a strict TargetType was good, but I'm struggling to find any benefit for it other than consistency with BasedOn.

We've determined that the compile-time checks are probably not the best course of action, as they will require significant changes and will only catch a portion of the potential problems.

Upon further discussion, we determined that the best course of action is to remove the exception from TargetType and replace it with a warning instead. As a developer, I'd like to be able to create a style and have it possibly apply to multiple target types as long as they share similar properties. For example, I should be able to use the same style to set the FontColor on a Label and a Button, even if the TargetType is set just to Button. If I accidentally try to use this style on a StackLayout, which has no such property, I'd like to have it just noop and log a warning for me. This would make the behavior a bit more friendly, like CSS styles.

Are there cases where the exception provides value that I'm missing? If so, please let me know.

@andreinitescu (or anyone else), if you're interested in submitting a follow-up PR that converts the exceptions to warnings as described above, let me know. Otherwise, we'll plan to pick it up later this week.

Thanks, everyone!

[IoTFier]

I think the intention of a strict TargetType was good, but I'm struggling to find any benefit for it other than consistency with BasedOn.

We've determined that the compile-time checks are probably not the best course of action, as they will require significant changes and will only catch a portion of the potential problems.

Upon further discussion, we determined that the best course of action is to remove the exception from TargetType and replace it with a warning instead. As a developer, I'd like to be able to create a style and have it possibly apply to multiple target types as long as they share similar properties. For example, I should be able to use the same style to set the FontColor on a Label and a Button, even if the TargetType is set just to Button. If I accidentally try to use this style on a StackLayout, which has no such property, I'd like to have it just noop and log a warning for me. This would make the behavior a bit more friendly, like CSS styles.

Are there cases where the exception provides value that I'm missing? If so, please let me know.

@andreinitescu (or anyone else), if you're interested in submitting a follow-up PR that converts the exceptions to warnings as described above, let me know. Otherwise, we'll plan to pick it up later this week.

Thanks, everyone!

@samhouts - I really appreciate sharing these details. A set of warnings is a great idea.
But, to reiterate my request - can we get transparency into the list of vendors / products and any other open source providers e.g. [https://github.com/rotorgames/Rg.Plugins.Popup] , FFImageloading , mFractor that are used in doing regression for XF releases?

Looking forward to the next update of XF 4.2 having the string TargetType converted to warnings.

Thank you

[IoTFier]

@samhouts , @andreinitescu , @CherryBu , @jsuarezruiz and @PureWeen ,

I just installed the service release 3 of XF 4.2; and everything run smooth like silk.

Thank you all for your help.

P.S. Please consider my request in sharing info on the list of vendors / products and any other open source providers e.g. [https://github.com/rotorgames/Rg.Plugins.Popup] , FFImageloading , mFractor that are used in doing regression for XF releases?