Refactor Animations to use [BindableProperty] partial properties#3013
Conversation
Refactored BaseAnimation and FadeAnimation to define bindable properties using the [BindableProperty] attribute and partial properties. Removed manual BindableProperty fields and wrappers, specifying default values and value creators via attributes and static methods. Updated property summaries for clarity and marked BaseAnimation as partial. This simplifies and modernizes the property implementation.
There was a problem hiding this comment.
Hey James! This PR has uncovered a bug in the source generator that we need to fix before merging.
The generated code (below) tries to cast ((BaseAnimation<TAnimatable>)bindable, however, <TAnimatable> is unknown to the separate file static class __BaseAnimationBindablePropertyInitHelpers. This is causing a compiler error.
This demonstrates that we need to come up with a solution to enable BindablePropertySourceGenerator to work with partial property initializers inside generic classes.
Generated Code (with error annotated)
// <auto-generated>
// See: CommunityToolkit.Maui.SourceGenerators.Internal.BindablePropertyAttributeSourceGenerator
#pragma warning disable
#nullable enable
namespace CommunityToolkit.Maui.Animations;
public partial class BaseAnimation<TAnimatable>
{
/// <summary>
/// Backing BindableProperty for the <see cref = "Length"/> property.
/// </summary>
public static readonly global::Microsoft.Maui.Controls.BindableProperty LengthProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("Length", typeof(uint), typeof(CommunityToolkit.Maui.Animations.BaseAnimation<TAnimatable>), null, (Microsoft.Maui.Controls.BindingMode)2, null, null, null, null, LengthCreateDefaultValue);
public partial uint Length
{
get => (uint)GetValue(LengthProperty);
set => SetValue(LengthProperty, value);
}
/// <summary>
/// Backing BindableProperty for the <see cref = "Easing"/> property.
/// </summary>
public static readonly global::Microsoft.Maui.Controls.BindableProperty EasingProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("Easing", typeof(Microsoft.Maui.Easing), typeof(CommunityToolkit.Maui.Animations.BaseAnimation<TAnimatable>), null, (Microsoft.Maui.Controls.BindingMode)2, null, null, null, null, __BaseAnimationBindablePropertyInitHelpers.CreateDefaultEasing);
public partial Microsoft.Maui.Easing Easing
{
get => __BaseAnimationBindablePropertyInitHelpers.IsInitializingEasing ? field : (Microsoft.Maui.Easing)GetValue(EasingProperty);
set => SetValue(EasingProperty, value);
}
}
file static class __BaseAnimationBindablePropertyInitHelpers
{
public static bool IsInitializingEasing = false;
public static object CreateDefaultEasing(global::Microsoft.Maui.Controls.BindableObject bindable)
{
IsInitializingEasing = true;
var defaultValue = ((BaseAnimation<TAnimatable>)bindable).Easing; // <-- This line is causing the compiler error because `__BaseAnimationBindablePropertyInitHelpers` does not know about <TAnimatable>
IsInitializingEasing = false;
return defaultValue;
}
}…alizers and add test for it
…onstructor parameter
TheCodeTraveler
left a comment
There was a problem hiding this comment.
Thanks James! I had the same idea to make __BindablePropertyInitHelpers a nested class.
I don't love that we have two implementations now, one that generates a nested class and one that generates a non-nested file class depending on whether the class is generic, because it increases our maintenance costs going forward. But I'm cool keeping it this way for now. If we discover another bug as we continue to implement [BindableProperty] internally, we may clean it up to have it always implement a nested class.
Description of Change
Refactored
BaseAnimationandFadeAnimationto define bindable properties using the[BindableProperty]attribute and partial properties. Removed manualBindablePropertyfields and wrappers, specifying default values and value creators via attributes and static methods. Updated property summaries for clarity and markedBaseAnimationas partial. This simplifies and modernizes the property implementation.Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information