Rxui5 validation #225

Closed
wants to merge 11 commits into
from

Projects

None yet

3 participants

@wendazhou
Member

Took a stab at implemeting a validation framework for ReactiveUI as per discussed in #217

In essence, it implements two mechanisms:

  • On one hand, we have IBindingErrorProvider, that provides a stream of errors for a given object. An implementation exist for both IDataErrorInfo and INotifyDataErrorInfo, and more can be added (similarly to the extensibility of the binding mechanism).
  • On the other hand, we have the IBindingDisplayProvider, that displays the current error on the view. An implementation is provided for WPF that basically hacks System.ComponentModel.Validation to display the error. In the future, we should look to move towards a more general system for all the xaml frameworks. This is also extensible and pluggable.

The validation framework then takes care of wiring these two components up, so that everything is handled correctly (I hope). The syntax looks as follows:

public class MyView : Window, IViewFor<ViewModel>
{
  public MyView()
  {
    InitializeComponent();

    this.Bind(ViewModel,  x => x.SomeProperty, v => v.MyTextBox.Text);
    this.DisplayValidationFor(x => x.MyTextBox.Text) // enables validation
  }
}

and a sample can be found at https://github.com/wendazhou/ReactiveUI/tree/rxui5-validation-sample

It compiles on most platforms (could not test WP8), but will not work on any except WPF as there is no implementation of IBindingDisplayProvider on both.

Also, the code makes it very clear moving to portable libraries #217 will be very beneficial, and figuring out a way to sort out the RxApp and introducing some sort of dependency injection would be really great (as per #219).

There is one known issue for the moment: in order to be able to find all the bindings on a given view, I register a IPropertyBindingHook that keeps track of all the bindings ever created... Not only does this probably leak, it is not very efficient, and hard to configure (we need it on by default so that it registers all bindings, but then if the validation is not used, it consumes memory purposelessly).

So, in conclusion, I'd like to gather some feedback on the design, and adress the following:

  • Provide implementations of IBindingDisplayProvider on all platforms
  • Figure out how to disable the registration hook if user is not using the framework
  • Test the whole thing, but will be easier with better DI
  • reduce the huge mess of #if by using portable libraries and better DI

@jlaanstra I saw that you rebuilt ReactiveValidatedObject for for INotifyErrorInfo, do you have any input on that?

In any case, thanks a lot @xpaulbettsx for the great work!

@jlaanstra
Member

I'll check this out and see if we can somehow combine our work to get at something awesome. Hopefully this week or in the weekend.

@wendazhou
Member

I don't know if you did anything more that what you pushed, but if not, it should just work when you merge, as I have implemented an error provider for INotifyDataErrorInfo.

@paulcbetts
Member

This is totally killer. @wendazhou, I've added you to the Owners team on ReactiveUI, you should move this directly to a branch on reactiveui/reactiveui. Here's how to do the Git Magic™:

git remote add upstream https://github.com/reactiveui/ReactiveUI.git
git push -u upstream rxui5-validation
@paulcbetts
Member

It compiles on most platforms (could not test WP8), but will not work on any except WPF as there is no implementation of IBindingDisplayProvider on both.

I'm okay with that - this is similar to the UserError framework, where we don't provide the UX (though having a default UX for WPF is good)

this.DisplayValidationFor(x => x.MyTextBox.Text) // enables validation

I like this syntax, and I also like that it's in the View code-behind. Would it be better if you enabled it via the ViewModel instead?

this.DisplayValidationFor(x => x.SomeProperty); // enables validation

Also, the code makes it very clear moving to portable libraries #217 will be very beneficial, and figuring out a way to sort out the RxApp and introducing some sort of dependency injection would be really great (as per #219).

Definitely - right now anything that uses Service Location in tests randomly fails because the state of Service Location is constantly being mutated.

@wendazhou
Member

I like this syntax, and I also like that it's in the View code-behind. Would it be better if you enabled it via the ViewModel instead?

This is actually a very interesting idea, we could handshake with ReactiveObject and have it store all the active bindings instead of using the global hook I have right now. It just seemed more natural at the time for it to be on the view, but thinking right now, it is a totally symmetric (in terms of view vs view-model) feature. I'll take a look in a few hours.

Concerning the UX, it shouldn't be hard to provide some default adorner system for all Xaml-based platforms, I just need to understand how adorners actually work.

@wendazhou wendazhou referenced this pull request Mar 28, 2013
Closed

Rxui5 validation #227

0 of 2 tasks complete
@wendazhou
Member

Closing this thread, now tracked in #227.

@wendazhou wendazhou closed this Mar 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment