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

[Feature] Allow ObservableValidator to force re-evaluation of validation for a property #3742

Closed
pixsperdavid opened this issue Feb 8, 2021 · 5 comments · Fixed by #3562
Closed
Assignees
Labels
feature request 📬 A request for new changes to improve functionality in progress 🚧 In-PR 🚀 mvvm-toolkit 🧰 Issues/PRs for the Microsoft.Toolkit.Mvvm package
Milestone

Comments

@pixsperdavid
Copy link

Describe the problem this feature would solve

I have a simple, common situation which is very hard to resolve with the current ObservableValidator implementation. I have a set of number boxes setting TCP port values for various services. Their validation rule is set to ensure that the value of each must be unique (cannot be the same as any other value). Due to this, it is possible that a value which failed validation due to not being unique, could then become valid as the result of another value changing. Therefore upon any value in the set changing, the validation rules for all values must be re-evaulated. However, ObservableValidator currently offers no way to force re-evaluate the validation of a property unless it's value changes.

Describe the solution

Add a 'RevalidateProperty' method to ObservableValidator. This simply invokes the existing private 'ValidateProperty' method. I have currently done this in my project and the solution works well, albeit I have to call the method via reflection. The new method would be used as follows:

public int ExampleValue1
{
    get => _exampleValue1;
    set
    {
        if (SetProperty(ref _exampleValue1, value, true))
            RevalidateProperty(ExampleValue2, nameof(ExampleValue2));
    }
}

Describe alternatives you've considered

A possible alternative is to change the value of the property requiring re-validation to a temporary value and then change it back to the original value, however this feels very hacky and doesn't accurately express the intention. It also has the possibility of triggering unintentional/undesirable side-effects as it would not be a 'real' value change.

@pixsperdavid pixsperdavid added the feature request 📬 A request for new changes to improve functionality label Feb 8, 2021
@ghost
Copy link

ghost commented Feb 8, 2021

Hello, 'impsnldavid! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

@Kyaa-dost
Copy link
Contributor

@impsnldavid Thanks for highlighting this feature. It seems like an interesting scenario. Let me open this up for discussion and see if any community member can provide more insight on this.

@Kyaa-dost
Copy link
Contributor

@Sergio0694 any insight on this scenario? Is this anyway relevant to #3729?

@Sergio0694
Copy link
Member

Sergio0694 commented Feb 8, 2021

I believe this issue has already been addressed with the changes I made in 54f48b4, as the same feature has been requested in #3665. I've included a full breakdown of the updated API surface of ObservableValidator (for the upcoming Preview 5) here. In particular, the ValidateProperty method is now protected, so it can be used directly by viewmodels inheriting from ObservableValidator. As an example, the snippet provided in the first post in this issue would become something like this:

public int ExampleValue1
{
    get => _exampleValue1;
    set
    {
        if (SetProperty(ref _exampleValue1, value, true))
            ValidateProperty(ExampleValue2, nameof(ExampleValue2));
    }
}

@impsnldavid thank you for posting the feedback though, this is definitely a fair feature request! 😄

@Sergio0694 Sergio0694 linked a pull request Feb 8, 2021 that will close this issue
7 tasks
@Sergio0694 Sergio0694 changed the title [Feature] Allow ObservableValidator to force re-evalation of validation for a property [Feature] Allow ObservableValidator to force re-evaluation of validation for a property Feb 8, 2021
@Sergio0694 Sergio0694 added In-PR 🚀 mvvm-toolkit 🧰 Issues/PRs for the Microsoft.Toolkit.Mvvm package and removed open discussion ☎️ labels Feb 8, 2021
@Sergio0694 Sergio0694 added this to the 7.0 milestone Feb 8, 2021
@Sergio0694 Sergio0694 self-assigned this Feb 8, 2021
@ghost ghost added the in progress 🚧 label Feb 8, 2021
@pixsperdavid
Copy link
Author

@Sergio0694 Thanks, looks great!

@ghost ghost closed this as completed in #3562 Feb 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 14, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature request 📬 A request for new changes to improve functionality in progress 🚧 In-PR 🚀 mvvm-toolkit 🧰 Issues/PRs for the Microsoft.Toolkit.Mvvm package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants