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

Make BackgroundKit take ICommand and not concrete Xamarin.Forms.Command implementation #67

Closed
toverux opened this issue Mar 6, 2020 · 3 comments · Fixed by #70
Closed
Assignees
Milestone

Comments

@toverux
Copy link

toverux commented Mar 6, 2020

Hello,

The library exposes a few command properties, I'll take the example of MaterialContentView.ClickedCommand.

This one holds a Xamarin.Forms.Command instance while typically it is recommended to use System.Windows.Input.ICommand instead. Even Xamarin.Forms uses it, see Button.Command for example.

I personally use ReactiveUI's ReactiveCommand to fulfill my needs. This implementation implements ICommand but does not inherit from Xamarin.Forms.Command, leading to this`kind of binding errors:

Binding: ReactiveUI.ReactiveCommand`2[Aterno.Binding.DisplayableRoom,System.Reactive.Unit] can not be converted to type 'Xamarin.Forms.Command'

Now I use a value converter to convert my commands on the fly:

public class ReactiveToXamarinCommandConverter : IValueConverter {
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture) {
        var command = (ICommand) value;

        return new Command(param => command.Execute(param));
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture) {
        throw new NotImplementedException();
    }
}

If you don't rely on this specific implementation of ICommand, could you use this base interface instead?

@ChasakisD
Copy link
Owner

Thank you for your feedback!

It's my mistake actually using Xamarin.Forms.Command instead of System.Windows.Input.ICommand. Gonna change it asap!

@ChasakisD
Copy link
Owner

@toverux Version 2.0.7 published to nuget. It will be live soon.

@toverux
Copy link
Author

toverux commented Mar 28, 2020

Works as expected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants