Skip to content
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

RelativeSource Binding Does Not Work For PlatformBehaviors (MCT Touch Behavior) #24313

Open
Axemasta opened this issue Aug 19, 2024 · 13 comments
Labels
area-xaml XAML, CSS, Triggers, Behaviors platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@Axemasta
Copy link
Contributor

Description

Earlier this year the maui community toolkit shipped the TouchBehavior which was a port of the popular TouchEffect, an issue noticed in development and by the community once released was that RelativeSource bindings cause the following exception to throw:

System.InvalidOperationException: Operation is not valid due to the current state of the object.

Our workaround for this problem was to use x:References to grab the binding context you needed, ie from the root page. This did leave users using shared data templates unsupported, however it supported most cases. Recently in #23711 it has been reported that compiled bindings will break the way our users are referencing their binding context. This is a major issue for the MCT, but possibly other PlatformBehaviors that use binding.

I understand that I am reporting a bug from another library, however I cannot understand what the TouchBehavior is doing wrong and now seeing the workaround will be breaking in .NET 9, i thought it was time to report it and investigate where the issue lies. If the issue is on the MCT side please let us know why this happens so we can understand and prevent this issue in the future.

I have a fully coded sample app demonstrating the issue.

Steps to Reproduce

See reproduction app for clerarer details:

  • Install MCT to project
  • Add TouchBehavior to a DataTemplate
  • For the Command, use a RelativeSource

Expected:
The relative binding applies and the command executes based on the touch behaviors interactions.

Actual:
The following crash occurs:

System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at Microsoft.Maui.Controls.Binding.ApplyRelativeSourceBinding(BindableObject targetObject, BindableProperty targetProperty, SetterSpecificity specificity)
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
   at Foundation.NSAsyncSynchronizationContextDispatcher.Apply() in /Users/builder/azdo/_work/1/s/xamarin-macios/src/Foundation/NSAction.cs:line 176
--- End of stack trace from previous location ---
   at ObjCRuntime.Runtime.ThrowException(IntPtr gchandle) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:line 2594
   at UIKit.UIApplication.UIApplicationMain(Int32 argc, String[] argv, IntPtr principalClassName, IntPtr delegateClassName) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:line 60
   at UIKit.UIApplication.Main(String[] args, Type principalClass, Type delegateClass) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:line 94
   at TouchBehaviorRelativeBinding.Program.Main(String[] args) in /Users/axemasta/Documents/Dev/GitHub/TouchBehaviorRelativeSource/TouchBehaviorRelativeBinding/Platforms/iOS/Program.cs:line 13
2024-08-19 14:53:25.848207+0100 TouchBehaviorRelativeBinding[31391:15072770] Unhandled managed exception: Operation is not valid due to the current state of the object. (System.InvalidOperationException)
   at Microsoft.Maui.Controls.Binding.ApplyRelativeSourceBinding(BindableObject targetObject, BindableProperty targetProperty, SetterSpecificity specificity)
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
   at Foundation.NSAsyncSynchronizationContextDispatcher.Apply() in /Users/builder/azdo/_work/1/s/xamarin-macios/src/Foundation/NSAction.cs:line 176
--- End of stack trace from previous location ---
   at ObjCRuntime.Runtime.ThrowException(IntPtr gchandle) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/ObjCRuntime/Runtime.cs:line 2594
   at UIKit.UIApplication.UIApplicationMain(Int32 argc, String[] argv, IntPtr principalClassName, IntPtr delegateClassName) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:line 60
   at UIKit.UIApplication.Main(String[] args, Type principalClass, Type delegateClass) in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:line 94
   at TouchBehaviorRelativeBinding.Program.Main(String[] args) in /Users/axemasta/Documents/Dev/GitHub/TouchBehaviorRelativeSource/TouchBehaviorRelativeBinding/Platforms/iOS/Program.cs:line 13

Last known working version is unknown for maui. I tested the TouchEffect in forms and relative source bindings did work then.

Link to public reproduction project repository

https://github.com/Axemasta/TouchBehaviorRelativeSource

Version with bug

8.0.80 SR8

Is this a regression from previous behavior?

No, this is something new

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows, macOS, Other (Tizen, Linux, etc. not supported by Microsoft directly)

Affected platform versions

All

Did you find any workaround?

Use x:Reference and hook onto a parent element, however this will break in .NET 9 along with compiled bindings

Relevant log output

No response

@Axemasta Axemasta added the t/bug Something isn't working label Aug 19, 2024
Copy link
Contributor

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

@Axemasta
Copy link
Contributor Author

@simonrozsival Hey I've opened a new issue seperate from #23711, there is a documented example of the crash. If theres anything I'm doing wrong wiring this up please let me know, I'm fairly sure it's correctly setup!

@kevinxufei kevinxufei added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Aug 20, 2024
@kevinxufei
Copy link

I can repro this issue at Android/iOS platforms on the latest 17.12.0 Preview 1.0(8.0.80 & 8.0.72).

@samhouts samhouts added the area-xaml XAML, CSS, Triggers, Behaviors label Aug 27, 2024
@LennoxP90
Copy link

This has been a fairly large pain point here as the base MAUI doesn't have a long press gesture and I desperately need one.

@simonrozsival
Copy link
Member

@Axemasta I finally looked into the issue, sorry for the delay.

The code throws because TouchBehavior does not extend Element:

image

@StephaneDelcroix is this behavior expected? Both the Behavior and Element extend BindableObject so it seems that Behavior should be an acceptable binding target.

@simonrozsival
Copy link
Member

simonrozsival commented Sep 12, 2024

OK, I think I can answer my own question: Behavior doesn't have a reference to its parent so we can't find its ancestors. I don't think this can change, although the exception might be better + maybe we shouldn't throw in async void. I'll try to open a PR to at least improve the debugging experience.

The best workaround that produces a binding that can be compiled I can think of at the moment is this:

public partial class MyPage : ContentPage
{
    public MyPage(ExampleViewModel exampleViewModel)
    {
        InitializeComponent();
        
        BindingContext = exampleViewModel;
    }

    public ExampleViewModel ViewModel => (ExampleViewModel)BindingContext;
}
<DataTemplate x:DataType="models:DisplayItem">
    <Grid ColumnDefinitions="Auto,*,Auto" RowDefinitions="*,*">
        <Grid.Behaviors>
            <mct:TouchBehavior DefaultOpacity="1" 
                                PressedOpacity="0.8"
                                PressedBackgroundColor="LightGray" 
                                DefaultBackgroundColor="White"
                                Command="{Binding ViewModel.ItemSelectedCommand, Source={x:Reference Root}, x:DataType=viewmodels:ExampleViewModel}"
                                CommandParameter="{Binding .}"/>
        </Grid.Behaviors>
        <!-- ... -->
    </Grid>
</DataTemplate>

Alternatively, it is possible to force XamlC to skip compilation of that binding and ignore the warning it will produce:

 Command="{Binding BindingContext.ItemSelectedCommand, Source={x:Reference Root}, x:DataType=x:Null}"

EDIT: I see there's a BasePage<TViewModel> in the CommunityToolkit samples that would nicely solve the problem without a need to change the XAML by shadowing the BindingContext:

class MyPage : ContentPage
{
    protected MyPage(ExampleViewModel vm)
    {
        base.BindingContext = vm;
    }

    public new ExampleViewModel BindingContext => (ExampleViewModel)base.BindingContext;
}

@Axemasta
Copy link
Contributor Author

@simonrozsival thanks for that explanation this is very useful. I have a PR open, but in a stagnant place regarding AppThemeBindings for PlatformBehavior not working and its exactly the same underlying issue that the behavior inherits BO but gets cast as an Element.

I've heard from the MCT team that Effect is going away in the long term, do we need to revisit this as a point of discussion because in XF I saw Behavior & Effect as basically the same thing however Effect seems to be more reliable than Behavior, as you run into all of these frictions with Behavior since it doesn't have a reference to an Element (effect does). Does behavior need updating to have this? Will effect be deprectaed or should we (MCT) cut our losses and move our behavior back to an effect for the short term?

@StephaneDelcroix
Copy link
Contributor

Behaviour shouldn't have ever been BindableObject. I'm guilty of that. They're meant to be shared wo they can't have parents. The problem is that the documentation for Behaviour shows the usage of Behaviour as a BindableObject

@DDHSchmidt
Copy link

Any progress on this?
We're currently using lots of <DataTemplate> tags in seperate xaml files, since we have different ones for phone, tablet, desktop display and even different ones per OS. They're part of the global ResourceDictionary and can be referenced by their keys.

The "golden" rule (if I understood this thread correctly or your suggestion from the other issue @simonrozsival ) would now be to always use x:Reference instead of RelativeSource, right?

Well, unless I'm mistaken that would mean to pollute our page xaml with additional 500+ lines of our DataTemplate xamls. or how am I gonna use the Reference unless it's within the same file, right?

I'm not exactly thrilled by that thought 🫤

@LennoxP90
Copy link

LennoxP90 commented Oct 10, 2024

I ended up re-architecting my templates and did it in the backends

 protected override void OnBindingContextChanged()
 {
   base.OnBindingContextChanged();

   MainGrid.Behaviors.Clear();

   if( BindingContext is ConversationModel conversation )
   {
     MainGrid.Behaviors.Add( new TouchBehavior()
     {
       Command = MainViewModel.ConversationsView.PressedCommand,
       CommandParameter = conversation,
       LongPressCommand = MainViewModel.ConversationsView.LongPressCommand,
       LongPressCommandParameter = conversation
     } );
   }
 }

this only works if your viewmodels are globally scoped

@simonrozsival
Copy link
Member

simonrozsival commented Oct 11, 2024

@DDHSchmidt No, you shouldn't need to make any changes to your code. You can safely use RelativeSource bindings. This issue is specific to the MCT touch behavior which currently cannot be used with RelativeSource bindings (in both .NET 8 and the upcoming .NET 9). If you have code that is using RelativeSource and it is working right now, it will continue working as is even when updating to .NET 9 for example.

In the discussion in the other issue that you linked I was initially suggesting using RelativeSource instead of x:Reference, not the other way round if I remember correctly. Later on I described ways to continue using x:Reference for customers who were not able to switch to RelativeSource or in the cases like the MCT touch behaviors which currently cannot work with RelativeSource.

Do you have any specific example of code that is causing you problems or that you think might be affected? I can certainly have a look and we can discuss it in more detail. I would suggest that you open a new issue though. Thanks!

@DDHSchmidt
Copy link

We do have code that is working right now since we recreated some of TouchBehavior's benefits via Effects in our own code.
But for maintenance sake I'd be very happy if we could get rid of that crutch and use TouchBehavior instead.

@Axemasta
Copy link
Contributor Author

We do have code that is working right now since we recreated some of TouchBehavior's benefits via Effects in our own code. But for maintenance sake I'd be very happy if we could get rid of that crutch and use TouchBehavior instead.

Currently the TouchBehavior is in a really bad place because of the maui bugs regarding. Unfortunately the maui team won't accept the PR's that fix these issues because they don't believe Behavior should inherit BindableObject so future wise its a bit mixed. I'm planning on un archiving my Maui.TouchEffect port and releasing on nuget, which itself was supposed to be a stop gap between XCT's TouchEffect and MCT's TouchBehavior. Currently I'm working on a port of TouchBehavior to an Effect but the api break is crazy because effects can't be used in the same way as behaviors.

At some point I need to write up a discussion on this repo about Behaviors & Effects and what the long term plan is because currently TouchBehavior is unfixable 🫤

@jsuarezruiz jsuarezruiz added this to the Backlog milestone Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-xaml XAML, CSS, Triggers, Behaviors platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants