-
Notifications
You must be signed in to change notification settings - Fork 120
Add trim-safe replacements for ChangePropertyAction and EventTriggerBehavior #309
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
base: main
Are you sure you want to change the base?
Conversation
…havior to be based upon trim-safe base, add various common event-based behaviors
| new PropertyMetadata(null)); | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the dependency property to change. This is not a dependency property, due to framework restrictions. |
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.
All dependency properties for this class break when one of the DPs has DependencyProperty itself as its type. Meta-DPs are reserved to the framework itself apparently...
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.
🤔 Interesting, I know sometimes not having it as a DP could cause VS to bubble up other warnings when binding to things, but since it'd only ever be a OneTime binding sort of scenario, I can't imagine it being an issue.
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.
Advanced binding scenarios which would trigger that warning probably requires you to make your own DP-of-DP, which you aren't able to, so I think this should be fine. As you've mentioned, this is rarely ever changed at runtime.
|
@michael-hawker any update on this? would love to get it merged |
|
@sylveon sorry for the delay, it may be a bit, but I too would like to get it merged. Would be good to get @Sergio0694 sign-off in the meantime. But I'll raise it up. |
|
@Sergio0694 have you been able to look at this? |
|
@michael-hawker @Sergio0694 any update? |
|
@Sergio0694 Can you sign off on this while you're online and screwing around on Discord chat? Thanks buddy. ❤️ |
#255 added trim annotation for various commonly used behaviors and actions. However, UWP .NET 9 requires NAOT (and by extension trimming-safe code), which meant that users migrating from UWP .NET Native to UWP .NET 9 where simply given "this behavior is not trim-safe" with no useful actionable info in the warnings. This is not helpful.
This PR adds a few behaviors and actions:
ChangeDependencyPropertyAction, a trim-safe alternative toChangePropertyActionwhich specifically targets dependency properties. This should cover many use cases thatChangePropertyActionpreviously covered (in our app, we where able to replace all uses with this). Use like the following:EventTriggerBehaviorBase<T>, a trim-safe version ofEventTriggerBehavior. Not useful by itself, it needs to be derived to add event registration and unregistration code. I've added a few subclasses covering the most common cases in our app, that hopefully are frequently used elsewhere as well. Additionally, I refactored the reflection-basedEventTriggerBehaviorto derive fromEventTriggerBehaviorBase<T>, as an example of more complex subclasses (and to reduce code duplication). Use like the following:These additions are backwards compatible with .NET Native, to allow progressive migration towards trim-safe patterns in an existing .NET Native UWP app, allowing easier upgrade to UWP .NET 9 or WinUI 3 down the line.
A few drive-by changes where made to improve code quality as well.