Skip to content

Remove BindablePropertyAttribute.DefaultValue#2995

Merged
TheCodeTraveler merged 47 commits intomainfrom
Remove-`BindablePropertyAttribute.DefaultValue`
Dec 12, 2025
Merged

Remove BindablePropertyAttribute.DefaultValue#2995
TheCodeTraveler merged 47 commits intomainfrom
Remove-`BindablePropertyAttribute.DefaultValue`

Conversation

@TheCodeTraveler
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler commented Dec 8, 2025

Description of Change

This PR removes the DefaultValue property from [BindableProperty] for the following reasons now that it is not longer required thanks to #2987:

  1. Setting the DefaultValue has been the largest source of pain and workarounds in BindablePropertySourceGenerator
  2. The DefaultValue property only supported compile-time constants, while partial property initializer can use static readonly values
    • For example, [BindableProperty(DefaultValue = Colors.Transparent)] would generate a compiler error because Colors.Transparent is public static readonly
  3. Improve the developer experience
    • It's a bit confusing to give developers two ways to initialize / set the default value of the bindable property
    • If we keep BindablePropertyAttribute.DefaultValue, we would need to write an additional Analyzer to generate a compiler error when we detect a developer is using both DefaultValue and the partial property initializer

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

Additional information

TouchBehavior Refactoring

I refactored TouchBehavior's properties to be null by default so that we can determine whether or not a developer as set each property.

I was forced into this refactor because the underlying GestureManager was relying on BindableProeprety.IsSet to determine whether or not a developer had set a value. This logic only works with BindableProperty.DefaultValue. Now that partial property initializers use DefaultValueCreator, BindableProperty.IsSet always returned false until .NET MAUI only calls DefaultValueCreator when the BindableProperty is first accessed (e.g. BindableProperty.GetValue(DefaultOpacityProperty).

This refactor of TouchBehavior should not cause any breaking changes for developers, as noted by the fact that I did not need to modify any of the following code:

  • TouchBehaviorPage
  • TouchBehaviorTests
    • (other than testing the Default Values of its properties)

ImageTouchBehavior Refactoring

I also refactored ImageTouchBehavior (same reasons as TouchBehavior).

This refactor required adding an OnPropertyChanged method for DefaultImageSource, HoveredImageSource and PressedImageSource to support the scenario where the ImageSource? was set to null when the image was active.

This refactor of TouchBehavior should not cause any breaking changes for developers, as noted by the fact that I did not need to modify any of the following code:

  • ImageTouchBehaviorPage
  • ImageTouchBehaviorTests
    • I added additional Assertions to ImageTouchBehaviorTests .VerifyImageSourceStateMachine
    • I created ImageTouchBehaviorTests .VerifyImageSourceStateMachineWhenImageSourceSetToNullWhilstActive
    • I modified the DefaultValues test

stephenquan and others added 29 commits December 5, 2025 18:34
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…fault_GeneratedCodeDefaultsToUseDefaultValueCreatorMethod`
@ne0rrmatrix
Copy link
Member

Do we want to add tests for most common system types that we will see used? Like DateTime and others? Or add them as we update our sample app?

@TheCodeTraveler TheCodeTraveler marked this pull request as ready for review December 11, 2025 23:16
@TheCodeTraveler TheCodeTraveler added the breaking change This label is used for PRs that include a breaking change label Dec 11, 2025
@TheCodeTraveler
Copy link
Collaborator Author

Do we want to add tests for most common system types that we will see used? Like DateTime and others?

@ne0rrmatrix Done! I've added these to this Unit Test: GenerateBindableProperty_WithComplexDefaultValues_GeneratesCorrectCode

ne0rrmatrix
ne0rrmatrix previously approved these changes Dec 12, 2025
Copy link
Member

@ne0rrmatrix ne0rrmatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was amazing. Learned a few new things reading this PR. It is exactly as discussed in this months meeting and is exactly what we are looking for. Very good job. I have just that one question about how we are handling touch behavior null issue that I mentioned. If you feel it is a non issue I think we can go ahead and merge this. I am excited to see this one get added!

It fixes the one issue I was struggling with and solves a few others with regards to default values.

@TheCodeTraveler TheCodeTraveler merged commit cc2038f into main Dec 12, 2025
7 of 10 checks passed
@TheCodeTraveler TheCodeTraveler deleted the Remove-`BindablePropertyAttribute.DefaultValue` branch December 12, 2025 21:04
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

breaking change This label is used for PRs that include a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants