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: Improved IDisposable Handling, Possibility to Change Validation Rules Dynamically #115

Merged
merged 19 commits into from
Oct 13, 2020

Conversation

worldbeater
Copy link
Collaborator

@worldbeater worldbeater commented Oct 12, 2020

What kind of change does this PR introduce?

This PR allows IValidationComponents to be detached from a ValidationContext.

What is the current behavior?

As described in #71, currently there is no way to detach an IValidationComponent if it is attached to a ValidationContext. Although it is possible to write WhenAnyValue-based ValidationRules that respect the observable and mutable state of a validated view model, sometimes it may be easier to just remove and dispose of a specific IValidationComponent.

Currently there is no way to dispose of a BasePropertyValidation and of a ModelObservableValidation, see https://github.com/reactiveui/ReactiveUI.Validation/blob/main/src/ReactiveUI.Validation/Extensions/ValidatableViewModelExtensions.cs#L58 If we call the ValidationRule() extension method, a new instance of a disposable IValidationComponent is created and attached to the ValidationContext. Notably, the disposable validation component is never disposed of. Also, currently, there is no way to dispose of the validation component manually, the IValidationComponent interface doesn't implement IDisposable, and the ValidationHelper doesn't dispose of the underlying IValidationComponent.

What is the new behavior?

Now, if we dispose of the ValidationHelper that is returned by a call to ValidationRule, it will dispose of an underlying BasePropertyValidation or a ModelObservableValidation automatically. Also, it will detach the IValidationComponent from the ValidationContext before disposal. Also, it is now possible to detach any validation components manually from the validation context. We are now accessing the ReadOnlyObservableCollection<IValidationComponent> in a reactive fashion both in our INotifyDataErrorInfo implementation, and in the BindValidation extension method. This allows putting ValidationRules into a WhenActivated block e.g.

this.WhenActivated(disposables =>
{
    this.ValidationRule(
        x => x.UserName,
        name => !string.IsNullOrWhiteSpace(name),
        "UserName shouldn't be null or white space.")
        .DisposeWith(_compositeDisposable);
});

What might this PR break?

Hopefully nothing.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

@codecov
Copy link

codecov bot commented Oct 12, 2020

Codecov Report

Merging #115 into main will increase coverage by 1.51%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #115      +/-   ##
==========================================
+ Coverage   52.73%   54.25%   +1.51%     
==========================================
  Files          16       16              
  Lines         895      894       -1     
==========================================
+ Hits          472      485      +13     
+ Misses        423      409      -14     
Impacted Files Coverage Δ
ValidationBindings/ValidationBinding.cs 79.36% <0.00%> (-3.07%) ⬇️
Extensions/ValidatableViewModelExtensions.cs 52.74% <0.00%> (-1.80%) ⬇️
Helpers/ReactiveValidationObject.cs 97.29% <0.00%> (+0.32%) ⬆️
Contexts/ValidationContext.cs 91.66% <0.00%> (+1.19%) ⬆️
Components/ModelObservableValidationBase.cs 90.00% <0.00%> (+10.00%) ⬆️
Extensions/ValidationContextExtensions.cs 27.45% <0.00%> (+10.30%) ⬆️
Components/BasePropertyValidation.cs 92.85% <0.00%> (+10.71%) ⬆️
Helpers/ValidationHelper.cs 100.00% <0.00%> (+28.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0040f9...7f2a2ba. Read the comment docs.

@worldbeater worldbeater marked this pull request as ready for review October 13, 2020 11:09
@worldbeater worldbeater requested a review from a team October 13, 2020 11:09
@glennawatson glennawatson merged commit 1121e56 into main Oct 13, 2020
@glennawatson glennawatson deleted the proper-disposal branch October 13, 2020 11:18
@reactiveui reactiveui locked as resolved and limited conversation to collaborators Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants