-
Notifications
You must be signed in to change notification settings - Fork 476
Refactor Views to use [BindableProperties] partial properties
#3017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Views to use [BindableProperties] partial properties
#3017
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks James!
Could you also please create a [ClassName]Defaults.shared.cs class for each of these Views in CommunityToolkit.Maui.Core/Defaults/ and also add a Unit Test for each called EnsureDefaults that verifies the default value for each property in the class equals the expected default value?
Here's an example that I added for FadeAnimation in the recent PR:
Maui/src/CommunityToolkit.Maui.Core/Primitives/Defaults/FadeAnimationDefaults.shared.cs
Lines 3 to 7 in f5b68d3
| static class FadeAnimationDefaults | |
| { | |
| public const uint Length = 300u; | |
| public const double Opacity = 0.3; | |
| } |
| [Fact] | |
| public void EnsureDefaults() | |
| { | |
| // Arrange | |
| var animation = new FadeAnimation(); | |
| // Act // Assert | |
| Assert.Equal(BaseAnimationDefaults.Easing, animation.Easing); | |
| Assert.Equal(FadeAnimationDefaults.Length, animation.Length); | |
| Assert.Equal(FadeAnimationDefaults.Opacity, animation.Opacity); | |
| } |
We should've created the [ClassName]Defaults.shared.cs and implemented these unit tests when originally creating every control in CommunityToolkit.Maui. But now that we're going back and editing every control to implement [BindableProperty], I've found that this is a great time to clean up our codebase and implement these!
I will update the classes as requested. This seems like a logical next step. Getting excited with all the changes. This has been fun to do so far :) |
TheCodeTraveler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks James!
Description of Change
Refactored
Viewsto 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. This simplifies and modernizes the property implementation.PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information