microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.33k stars 675 forks source link

Element binding not working in interactivity behavior of WinUI desktop project #4057

Closed KarthikRajaAKR closed 1 year ago

KarthikRajaAKR commented 3 years ago

Describe the bug I'm trying to use ChangePropertyAction trigger using Microsoft.Xaml.Behaviors.WinUI.Managed package, but trigger not working.

So I have checked the issue by porting XamlBehavior code into my testing project and found that element binding value not updated to trigger.

Steps to reproduce the bug Sample: WinUIDesktopTestProject.zip

Steps to reproduce the behavior:

  1. Run the attached sample and change the first slider value
  2. First Rectangle background should get updated but it's not. ( Slider value >= 50 => Green, else Transparent)
  3. Change slider value of second slider, note second rectangle fill not updated.

Note:

  1. For first rectangle, Interactivity package behavior used
  2. For second rectangle, locally integrated interactivity code used.

Expected behavior Behaiors should trigger value based on condition.

Screenshots NA

Version Info

NuGet package version: Microsoft.WinUI 3.0.0-preview3.201113.0 Microsoft.Xaml.Behaviors.WinUI.Managed - 2.0.3-rc4

Windows app type: UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362) Yes
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT
StephenLPeters commented 3 years ago

@michael-hawker Is this a Winui3 issue, a toolkit issue or perhaps the authors usage issue?

michael-hawker commented 3 years ago

@StephenLPeters nothing to do with the Toolkit, the XAML Behaviors are a different thing. So, it's either an issue in WinUI 3 OR it's an issue with the WinUI 3 port of the Behaviors package (FYI @DVaughan @azchohfi). I just copied the code snippet from the provided sample into XAML Studio and was able to see the proper behavior, so the original UWP logic seems sound.

    ...
    xmlns:interactionCore="using:Microsoft.Xaml.Interactions.Core" 
    xmlns:interaction="using:Microsoft.Xaml.Interactivity">

        <Rectangle x:Name="DataTriggerRectangle" Grid.Row="1" Fill="Violet">
            <interaction:Interaction.Behaviors>
                <interactionCore:DataTriggerBehavior Binding="{Binding Path=Value, ElementName=slider}" ComparisonCondition="GreaterThan" Value="50">
                    <interactionCore:ChangePropertyAction TargetObject="{Binding ElementName=DataTriggerRectangle}" PropertyName="Fill" Value="Green"/>
                </interactionCore:DataTriggerBehavior>

                <interactionCore:DataTriggerBehavior Binding="{Binding Path=Value, ElementName=slider}" ComparisonCondition="LessThanOrEqual" Value="50">
                    <interactionCore:ChangePropertyAction TargetObject="{Binding ElementName=DataTriggerRectangle}" PropertyName="Fill" Value="Transparent"/>
                </interactionCore:DataTriggerBehavior>
            </interaction:Interaction.Behaviors>
        </Rectangle>
        <Slider x:Name="slider" Grid.Row="2" Value="1"/>

We do use the behaviors as well in the Toolkit, but not sure how extensively we've tested things using them yet. We leverage it more in our upcoming 7.0 release which is still being worked on.

@KarthikRajaAKR what changes did you make when you copied the code? Did you grab from their main branch or the branch they had setup for WinUI already?

KarthikRajaAKR commented 3 years ago

Hi @michael-hawker, I just copied code from the main branch and modified namespaces for WinUI from UWP.

StephenLPeters commented 3 years ago

@michael-hawker I'm not sure I understand this issue, could you help me direct it to people who could answer better? @MikeHillberg or @Austin-Lamb .

michael-hawker commented 3 years ago

@StephenLPeters so if the only code change is the fact that something that works in UWP now doesn't in WinUI 3 this sounds like it's exposed an issue in the WinUI 3 preview. I think this may be is related to this DevOps issue? @alwu-msft looks like the same root cause?

@azchohfi was your report sparked from this issue/discussion here? This'll effect our Animation story in the Toolkit as well, as we do similar patterns with attached behaviors and binding in our Sample App examples for XamlReader.

azchohfi commented 3 years ago

I believe it is the Binding. @KarthikRajaAKR, could you try using x:Bind, and see if the issue persists?

I've reported this issue already, and apparently its a WinUI3 issue, and no @michael-hawker I didn't see this issue until now, but they seem like the same.

KarthikRajaAKR commented 3 years ago

@azchohfi, thanks for the suggestion but x:bind also not working.

chrisglein commented 3 years ago

It's hard to evaluate this as there's a stack of various previews involved (both WinUI and XamlBehaviors). @KarthikRajaAKR have you attempted to reduce the binding problem in question without XamlBehaviors so we can verify if this is a WinUI3 issue or a WinUI3+XamlBehaviors issue?

There's been a fair number of bug fixes between preview3 and preview4. @DVaughan have we had a compatibility check between XamlBehaviors and preview4?

ghost commented 3 years ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

michael-hawker commented 3 years ago

@azchohfi, thanks for the suggestion but x:bind also not working.

FYI @alwu-msft thought this may have been the same issue as the DevOps one I linked to tracking, but in that one you said x:Bind should work which here it appears not to?

evelynwu-msft commented 3 years ago

@michael-hawker If replacing {Binding} with {x:Bind} fails as well, then it's unlikely to be the same root cause as the DevOps issue since x:Bind doesn't rely on FrameworkElement.FindName (the moral equivalent of what an ElementName Binding does).

Noemata commented 3 years ago

@michael-hawker , given this: https://github.com/microsoft/XamlBehaviors/issues/213

It doesn't look like XAML Behaviors has had much testing against WinUI 3. I consider the EventTriggerBehavior/InvokeCommandAction essential and a must have in order to be able to port even trivial UWP applications to WinUI.

Noemata commented 3 years ago

While I'm here, could we please add this to XAML Behaviors: https://github.com/Noemata/SimpleMVVM/blob/master/SimpleMVVM/SimpleMVVM/Behaviors/EventToCommandBehavior.cs

I've seen other Microsoft devs use this within their commercial UWP App store projects, so it's only fair to include this in XAML Behaviors for everyone to benefit.

michael-hawker commented 3 years ago

@Noemata you should open new issues for requests about adding a specific Behavior on the Behavior repo itself and not on a different bug thread in the WinUI repo.

@chrisglein the sample provided @KarthikRajaAKR contains a copy of the behaviors code within the project itself, so you can test just on top of WinUI itself.

@KarthikRajaAKR it wasn't clear from me in the report (and I can't seem to build the project at the moment) about if both scenarios failed or the local copy somehow worked? Can you clarify some of the details on the two scenarios, expected results, and screenshots in your bug report? Thanks!

KarthikRajaAKR commented 3 years ago

Hi @michael-hawker, Both scenarios are not working, I integrated the code just to check the issue. In both scenarios, I suspect that binding is the issue.

Noemata commented 3 years ago
    <StackPanel x:Name="Stack" Orientation="Horizontal" HorizontalAlignment="Center" VerticalAlignment="Center">
        <i:Interaction.Behaviors>
            <ic:EventTriggerBehavior EventName="Loaded">
                <ic:InvokeCommandAction Command="{x:Bind StackPanelLoadedCommand}" CommandParameter="{Binding ElementName=Stack}"/>
            </ic:EventTriggerBehavior>
        </i:Interaction.Behaviors>

        <Button x:Name="myButton" Click="myButton_Click">Click Me</Button>
    </StackPanel>

I traced through the XAML behaviors code with a WinUI project. Here's a screenshot of the problematic code:

image

CommandParameter should not be null.

@michael-hawker , it's quite disappointing to see this sort of flaw slip through on WinUI 3 Preview 4. This should be caught by automated testing before it gets out the door!

Noemata commented 3 years ago

Looks like the binding expression -> CommandParameter="{Binding ElementName=Stack}" is broken in WinUI 3 Preview 4.

Noemata commented 3 years ago

x:Bind works -> CommandParameter="{x:Bind Stack}

So it looks like a WinUI 3 Preview 4 bug.

evelynwu-msft commented 3 years ago

This will be fixed in an upcoming preview of WinUI 3.

Noemata commented 3 years ago

I did have a couple other binding expressions fail when running through a series of XAML Behaviors tests. I didn't bother running those against the XAML Behaviors source code since it looked like there may have also been a XAML compiler problem, like here:

https://github.com/microsoft/microsoft-ui-xaml/issues/4092

Running against the XAML Behaviors unit tests should reveal any remaining issues, I hope ...

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.