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

Support live option for observable objects #540

Open
chilversc opened this issue Feb 10, 2015 · 9 comments
Open

Support live option for observable objects #540

chilversc opened this issue Feb 10, 2015 · 9 comments

Comments

@chilversc
Copy link

I have some properties that contain 0 or 1 items that either start as null or can change their instance in the future. These properties are not included in the validation even when live is true.

Currently I'm using the following as a work around:

this.valueForValidation = ko.observableArray([null]);
this.value = ko.computed({
    read: function() { return this.valueForValidation()[0]; },
    write: function(value) { this.valueForValidation([value]); },
    owner: this
});
@crissdev
Copy link
Member

Working fiddle http://jsfiddle.net/6wp03awL/

@chilversc
Copy link
Author

Sorry, my work around was a bit confusing there. The actual property is not an array in this case.

e.g. the model I want is something like: http://jsfiddle.net/efpoa2Lg/1/

ko.validation.init({messagesOnModified: false, decorateInputElement: true, decorateElementOnModified: false}, true);

function AddressViewModel()
{
    this.line1 = ko.observable().extend({required: true});
    this.line2 = ko.observable().extend({required: true});
}

function ViewModel() {
    this.email = ko.observable().extend({required: true, email: true});
    this.address = ko.observable();

    this.addAddress = function() {
        this.address(new AddressViewModel());
    }.bind(this);

    this.removeAddress = function() {
        this.address(null);
    }.bind(this);
}


ko.applyBindings(ko.validatedObservable(new ViewModel(), {deep: true, observable: true, live: true}));

Because address is not an array, validatedObservable only ever adds the first object to its group, even if that object's instance later changes. Using an array that only ever contains 0 or 1 items provides a work around for the behaviour I want, and a computed observable then presents that array as if it were a single object.

@crissdev
Copy link
Member

The problem is the group is not updated when any of the properties on the ViewModel changes. There are some solutions for this issue:

  • use observable: false in grouping options and ask for errors when needed.
  • invoke internal method _updateState to force the underlying group to refresh. (The internal method was introduced in this commit 3dbd84d)

I've updated your fiddle to use the second approach since the first solution is pretty straightforward http://jsfiddle.net/vd2o9q1a/

@chilversc
Copy link
Author

Could be a work around for now, but I don't really like relying on private methods like that. As a quick test I removed the utils.isObservableArray check and the validation seemed to behave correctly.

Though I was wondering about handling of deep object graphs as a whole. For instance I wanted to have references back to the parent object but not follow those references with deep: true. I can equally see a case such as a selected item where you'd only want to validate the observable, but not recurse in to the observable.

I was thinking of adding live and deep to the validatable extension to allow control of this on a per observable basis.

So then you could have:

this.address = ko.observable().extend({validatable: {deep: true, live: true}});
this.selectedItem = ko.observable().extend({validatable: {deep: false}});

The live option only makes sense when deep is true for objects. So when deep is false on an object live can be ignored.

This wouldn't be a breaking change as when deep === undefined the existing behaviour of using the options passed to group would apply. This change would only take effect when deep === true or deep === false.

@crissdev
Copy link
Member

The internal method is just a way to update the group and it was added to help validatedObservable remain consistent. Of course, using a private API is highly discouraged but you have the freedom of choosing the option that suits best to your needs. Enabling grouping options for individual observables adds complexity and confusion - honestly I don't see myself typing that, but that's just my two cents.

@chilversc
Copy link
Author

I can't see any way to avoid individual grouping options though if you want ko.validatedObservable(model, {deep: true}) but then want to exclude certain observables such as references back to the parent, or observables containing objects that are outside the current context.

The only alternative is to use ko.validation.group and re-implement the recursive code. Which is fairly complex when you include the cycle detection and live option.

My thought was you would only need to specify the grouping options on the individual observables when they're different from the default options to allow opt-in/opt-out. To reduce the typing the syntax could be this.address = ko.observable().extend({deep: true, live: true}) or for opt-out this.selectedItem = ko.observable().extend({deep: false}).

I've assumed there is a reason live only applies to arrays by default and not objects so was thinking objects should be opt-in only for live. If not, then perhaps another option could be added to grouping for live objects?

While calling _updateState does work it can be awkward to get the correct context when the validatable isn't at the root. In my case the address observable turns up in 2 validation groups, one for the specific step, and one as a property of the main screen model.

@crissdev
Copy link
Member

Linking #478 and #224 to this.

@crissdev
Copy link
Member

crissdev commented Mar 2, 2015

@chilversc Changing the behaviour of live and deep options is not very likely to happen. What we can do instead is to add some ignore option (for the grouping options) to specify which objects / references should not be part of the group. An alternative would be to specify an option for each observable #224.

@chilversc
Copy link
Author

That was what I was thinking, but unlike #224 which only allows ignoring specific observables I thought of adding the same live/deep options as extenders.

The logic would then be; check if the observable has an explicit live/deep setting if so use the explicit value (even if live is applied to a normal observable rather than an array). Otherwise use the live/deep setting from the options as per the normal rules (i.e. live from options only applies to arrays).

This would allow setting the deep/live options on a per observable basis.

function Nested() {
    this.value = ko.observable().extend({number: true});
}

function Model() {
    // deep from options, live from options only applies to arrays
    this.objectDeepNotLive = ko.observable(new Nested());

    // deep from options, explicit override for live
    this.objectDeepAndLive = ko.observable(new Nested()).extend({live: true});

    // explicit override for deep, live from options only applies to arrays
    this.objectNotDeepNotLive = ko.observable(new Nested()).extend({deep: false});


    // deep from options, live from options
    this.arrayDeepAndLive = ko.observableArray();

    // explicit override for deep and live 
    this.arrayNotDeepNotLive = ko.observableArray().extend({live: false, deep: false});

    // ignored completely (as per #224), live and deep options are irrelevant
    this.ignored = ko.observable().extend({ignoreInValidationGroup: true});

    return ko.validatedObservable(this, {deep: true, live: true});
}

Equally you could specify deep/true as false in the options and explicit override them on specific observables to true.

Another consideration here would be to add ignoreInValidationGroup to the options and allow overriding it to false on specific observables to allow for opt-in instead of opt-out.

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

No branches or pull requests

2 participants