microsoft / microsoft-ui-xaml

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

Proposal: Make Command Properties assignable to System.Windows.Input.ICommand #2240

Open christiannagel opened 4 years ago

christiannagel commented 4 years ago

For .NET, please use System.Windows.Input.ICommand for Command properties. This interface is included with .NET Standard.

Assigning a Command to a shared view-model library using System.Windows.Input.ICommand results in the compilation error "Invalid binding assignment: Cannot directly bind type System.Windows.Input.ICommand to Microsoft.UI.Xaml.Input.ICommand..."

Using System.Windows.Input.ICommand makes it possible to create .NET Standard shared libraries containing view-models, and reuse them across WPF, UWP, Xamarin, Prism... This is not possible with WinUI.

With UWP this is documented (from https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.primitives.buttonbase.command):

The ICommand interface comes from a different source depending on whether the data source is implemented in C++ or for Microsoft .NET. C++ implements Windows::UI::Xaml::Input::ICommand. Microsoft .NET implements System.Windows.Input.ICommand. Both interfaces have the same template. Which interface you call from your app code is analogous: use Windows::UI::Xaml::Input::ICommand for C++ code and System.Windows.Input.ICommand for Microsoft .NET code.

Wouldn't this be possible with WinUI as well? This would make it easy to reuse view-models created earlier with WinUI.

Thanks, Christian

adambarlow commented 4 years ago

I would not be opposed to this. @llongley what would be required here to achieve this?

ranjeshj commented 4 years ago

@MikeHillberg @stevenbrix Is this something that needs to be fixed in the interop layer ?

stevenbrix commented 4 years ago

Yeah this is a known issue. I'd close this as a dupe of #1981

christiannagel commented 4 years ago

this is now working with preview 1 👍 Closing the issue

tomasfabian commented 4 years ago

It works for the Desktop Win32 version, but not for the UWP version WinUI 3 Preview1.

<Button Command="{x:Bind ViewModel.AddNew}"/>

I got the following error in XAML:

Severity Code Description Project File Line Suppression State Error Invalid binding assignment : Cannot directly bind type 'System.Windows.Input.ICommand' to 'Microsoft.UI.Xaml.Input.ICommand'. Use a cast, converter or function binding to change the type

On the other hand this can be compiled:

<Button Command="{Binding ViewModel.AddNew}"/>
MikeHillberg commented 4 years ago

There's a couple of things going on there. One is that an app running in UWP is using .Net Native, which doesn't understand WinUI3's ICommand (it's in the Microsoft namespace rather than the Windows namespace). When running as a Desktop app it's using .Net5, which (because of cs/winrt) understands how to project as the System.Windows.Input version.

The other thing there is that {x:Bind} is a language feature that's evaluated during build, whereas {Binding} is a runtime feature.

tomasfabian commented 4 years ago

@MikeHillberg thx

The other thing there is that {x:Bind} is a language feature that's evaluated during build, whereas {Binding} is a runtime feature.

I understood this, but what is the conclusion? Should I use conditional compilation in a common library to choose the correct ICommand?

The runtime feature {Binding} was able to call System.Windows.Input.ICommand.Execute() in an UWP WinUI 3 app during runtime in some cases.

MikeHillberg commented 4 years ago

As a workaround, could you implement both ICommands on the value? (System.ComponentModel and Microsoft.UI.Xaml)

christiannagel commented 4 years ago

In my case the library is a .NET Standard library that's also used with Xamarin, WPF, UWP, WinUI. As a workaround using C# preprocessor directives and building different binaries should be possible.

tomasfabian commented 4 years ago

Sure we can do that, but its beginning to be quite messy. We already have a lot of C# preprocessor directives and conditional compilation in our codebase for mobile, desktop and different target frameworks: #NETFRAMEWORK, #NETCOREAPP etc, etc. Bringing yet another package dependency in the csproj (with Condition)...

@MikeHillberg what is inside System.ComponentModel? I haven't found ICommand there. Did you mean System.Windows.Input?

tomasfabian commented 4 years ago

I ended with this workaround:

#if !NETCOREAPP && !NETFRAMEWORK
    private Joker.WinUI3.Shared.Commands.RelayCommand addNew;

    public Microsoft.UI.Xaml.Input.ICommand AddNew => addNew ?? (addNew = new Joker.WinUI3.Shared.Commands.RelayCommand(OnAddNew, OnCanAddNew));
#else
    private Prism.Commands.DelegateCommand addNew;

    public ICommand AddNew => addNew ?? (addNew = new DelegateCommand(OnAddNew, OnCanAddNew));
#endif

I had to create yet another ICommand implementation since Prism's DelegateCommand, ReactiveUI's ReactiveCommand, our company's command etc. are not compatible with Microsoft.UI.Xaml.Input.ICommand.

Shouldn't this issue be reopened/reconsidered please?

Is there a WinUI_UWP symbol for conditional compilation? I haven't found it.

Thank you very much for your understanding

christiannagel commented 4 years ago

@thomasfabian, I think this should be reopened and just did. Using C# preprocessor directives should just be a temporary workaround.

MikeHillberg commented 4 years ago

what is inside System.ComponentModel? I haven't found ICommand there. Did you mean System.Windows.Input?

Yes, sorry, I meant System.Windows.Input (I got confused with INotifyPropertyChanged).

amitpal1979 commented 2 years ago

Hi, this issue still exists in released version of WinUI. It seems to be working in Win32 apps but it is not working in UWP apps. Please suggest whether this will be fixed in upcoming releases.