-
Notifications
You must be signed in to change notification settings - Fork 83
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
Incorrect target type for RelativeAncestor attached properties #106
Comments
Does this cause relativeAncestor not to work? "DataContext="{Binding DataContext.(mainDetails:MasterDetailViewModel.CasaDetailsViewModel), RelativeSource={RelativeSource Mode=FindAncestor,AncestorType={x:Type mainDetails:MasterDetailView}}}"" DataContext="{Binding DataContext.CasaDetailsViewModel, RelativeSource={RelativeSource Mode=FindAncestor,AncestorType={x:Type mainDetails:MasterDetailView}}}" Previously worked, but when I tried upgrading to source generator it no longer worked, as it just couldn't find the viewmodel, no error messages or anything, just a crash whenever I opened it due to property being null |
@oscarjaergren your example appears to be WPF based, this issue is about the UWP polyfill we have for RelativeSource Mode=FindAncestor in the Windows Community Toolkit and is unrelated to whatever issue you may have had. If there's some difference between code you had before with the MVVM Toolkit and code after switching to the Source Generator version, you should open a discussion/issue in the .NET Community Toolkit repo here: https://aka.ms/toolkit/dotnet |
Thanks @Sergio0694, brought this over to the new repo and will fix in #105. Note, However, you're correct, since we generally want to wait until the element is loaded, we should make Though technically, we could still call the visual tree helpers on a DependencyObject, I don't think there's going to be many of those cases though, so think fine to exclude? |
…Extensions.RelativeAncestor to FrameworkElement over DependencyObject Code scoped to FrameworkElement in callback, so doesn't work with just DependencyObject as-is.
…Extensions.RelativeAncestor to FrameworkElement over DependencyObject Code scoped to FrameworkElement in callback, so doesn't work with just DependencyObject as-is.
Describe the bug
The attached properties for the "relative ancestor" feature are currently targeting the wrong type:
https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/44ad0cc6806c99f1a693c923cb94ea7551718c13/Microsoft.Toolkit.Uwp.UI/Extensions/FrameworkElement/FrameworkElementExtensions.RelativeAncestor.cs#L18-L21
These are defined in
FrameworkElementExtensions
but target aDependencyObject
. This is not correct on its own, but in particular it also makes the APIs technically more error prone, as there is no type guarantee at build-time, and we're doing additional checks at runtime to ensure the input elements are in factFrameworkElement
-s, otherwise we do nothing:https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/44ad0cc6806c99f1a693c923cb94ea7551718c13/Microsoft.Toolkit.Uwp.UI/Extensions/FrameworkElement/FrameworkElementExtensions.RelativeAncestor.cs#L62-L65
Given that these APIs only make sense for
FrameworkElement
instances, and that they're defined withinFrameworkElementExtensions
too, we should update the signature and have them only targetFrameworkElement
objects. I should note: this would not be a source breaking change when the API was being used correctly, but it would be a binary breaking change.The text was updated successfully, but these errors were encountered: