reactiveui / ReactiveUI.Validation

Validation helpers for ReactiveUI-based apps.
https://reactiveui.net/docs/handbook/user-input-validation
MIT License
235 stars 24 forks source link

[BUG] ValidationContex is not updated properly. #95

Closed ScarletKuro closed 3 years ago

ScarletKuro commented 3 years ago

Describe the bug When I change the property value in the ViewModel with the ReactiveValidationObject<> the validation works wrongly on TextBox with NotifyOnValidationError=True, ValidatesOnDataErrors=True. When I change TextBox property directly via keyboard its working perfectly. When I started to debug, I noticed that the ValidationContext has not up to date information.

Steps To Reproduce

I created a repository with the bug https://github.com/ScarletKuro/ValidationContextBug It has ReactiveValidationObjectBugViewModel with the bug, and the ClassicValidationViewModel that implements IDataErrorInfo and works correctly, just to make sure the bug is not related with something else. I know that ReactiveValidationObject implements the INotifyDataErrorInfo, but it still should work the same, because if I make IDataErrorInfo to work with ValidationContex it gets the same buggy behaviour.

Provide the steps to reproduce the behavior: Simple code ViewModel:

public class ReactiveValidationObjectBugViewModel : ReactiveValidationObject<ReactiveValidationObjectBugViewModel>
{
        [Reactive]
        public string CustomerCode { get; set; } = string.Empty;
        public ReactiveCommand<Unit, string> Numeric0Command { get; }
        public ReactiveCommand<Unit, string> Clear { get; }

        public ReactiveValidationObjectBugViewModel() : base(Scheduler.Immediate)
        {
            Numeric0Command = ReactiveCommand.Create(() => CustomerCode += 0);
            Clear = ReactiveCommand.Create(() => CustomerCode = string.Empty);

            this.ValidationRule(viewModel => viewModel.CustomerCode,
                customerCode => !string.IsNullOrEmpty(customerCode), "You need to enter customer code");
        }
}

View xaml: <TextBox Text="{Binding CustomerCode, UpdateSourceTrigger=PropertyChanged, NotifyOnValidationError=True, ValidatesOnDataErrors=True, Mode=TwoWay}"/>

Current buggy behavior As you can notice, when we have a value the Text is red. When the TextBox is empty its not red. But when I type more then it gets updated.

Expected behavior

Environment

worldbeater commented 3 years ago

Thanks for reporting this! Curious if BindValidation is not showing validation errors in this case If yes, then probably the issue is in this class https://github.com/reactiveui/ReactiveUI.Validation/blob/main/src/ReactiveUI.Validation/Helpers/ReactiveValidationObject.cs

GitHub
reactiveui/ReactiveUI.Validation
Validation helpers for ReactiveUI based solutions, functioning in reactive way. - reactiveui/ReactiveUI.Validation
ScarletKuro commented 3 years ago

Yes, just tried with the BindValidation and it works correctly. I updated the repo with the BindVlidation example.

ScarletKuro commented 3 years ago

But I'm not sure that the problem is ReactiveValidationObject. Feels like it's some synchronization problem. In my observations, if we take IDataErrorInfo and make it rely on ValidationContext with some simple code like this

public string? this[string columnName]
{
    get
    {
        var temp = ValidationContext.Validations
            .OfType<IPropertyValidationComponent<ClassicValidationViewModel>>()
            .Where(validation => !validation.IsValid && validation.ContainsPropertyName(columnName))
            .Select(validation => validation.Text?.ToSingleLine()).FirstOrDefault();
        return temp;
    }
}

We get the same problem as with ReactiveValidationObject that uses INotifyDataErrorInfo During the debug, when we change CustomerCode in viewmodel lets say to empty and the IDataErrorInfo is firing, we see that the ValidationContext.Validations.IsValid is true

mickut commented 3 years ago

I encountered this too, and I think it indeed has to do with INotifyDataErrorInfo implementation in ReactiveValidationObject. AFAIK the WPF framework expects the validation change events to explicitly contain the changing property name as a parameter of DataErrorsChangedEventArgs, but now that event is triggered with a string.Empty only when ValidationContext.ValidationStatusChange occurs.

It should trigger the ErrorsChanged -event with affected main property whenever any validator error changes, not just the state. That way two validators (e.g. not empty, and must meet some format requirement) can be indicated and triggered correctly.

A workaround could be to add a protected RaiseErrorChanged(Expression<...> viewModelProperty) to the ReactiveValidationObject so you could listen to the error message changes from a specific ValidationRule and trigger a new ErrorChanged event with the property name.

worldbeater commented 3 years ago

Looking into this now. Most likely we are hitting https://stackoverflow.com/a/24837028/11351183

Stack Overflow
UI not calling INotifyDataErrorInfo.GetErrors()
I have a model implementing both INotifyPropertyChanged and INotifyDataErrorInfo. The Property changed event fires when ever I have a property modified, but for some reason when I raise the Error e...
worldbeater commented 3 years ago

ReactiveUI.Validation with the fix is now shipped to NuGet as ^1.6.4.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.