Added the ability to reset validation state.

[kylefarris]

The functionality was added as an option to the module so that it is the default if the user chooses for it to be. It can also be ran with a new method resetValidation or via a parameter to the clear and reset methods.

This is useful when the same form is being reused without having to refresh the page or re-render the entire view.

The original functionality is retained as to not break backward-compatibility. This feature, in all cases, is purely optional. I updated the changelog portion of the README to reflect a minor update as their should be no breaking changes but it is not simply a bug patch.

[cdaringe]

hey @kylefarris, just thinking out loud here, and off the cuff. shouldn't reset also reset the validation state? otherwise, if you didn't want to reset the validation state, you'd just use clear. that may not be how it's implemented ATM, but seems possibly more clear? what do you think?

[kylefarris]

@cdaringe I would personally prefer that but it could cause some people's apps to act in unexpected ways if they, for some reason, expect it to work as it currently does. With this PR, they can easily add in the feature by adding and setting the clearValidationOnReset option to true.

If they want to reset an individual field back to original state and clear validation state, they can run fieldVarHere.reset(true). OR to completely clear the field and clear validation state, they can run fieldVarHere.clear(true). Or they can just reset or clear fields as they have been without resetting validation state by calling the methods without the param (unless they have the clearValidationOnReset option set to true).

In other words, if we just made it so that reset did what clearValidation() does, users would lose the ability to decide to clear validation state--even if we both agree it makes much more sense to always do this (at least on reset).

My 2 cents.

[cdaringe]

gosh, i'm sorry, i'm still having a hard time understanding all of what this PR is attempting to acheive. apologies for the inconvenience, but can you baby step me through it? I'm pretty sure i'm just a little slow today 🐢

[kylefarris]

Hmm... No problem man. So, let's assume this single field form (plus submit button)...

var FormView = require('ampersand-form-view');
var InputView = require('ampersand-input-view');
var Users = require('./collections/users');
var users_collection = new Users();

var name_field = new InputView({
    label: 'Name',
    name: 'name',
    required: true,
    tests: [
        function(val) {
            if (val.length <= 1) return "The name must be more than 1 character!"
        }
    ]
});

var form = new FormView({
    el: this.queryByHook('place-on-page'),
    submitCallback: (form_data) => {
        users_collection.create({name: form_data.name}, {wait:true, success: (m,r,o) => {
            form.reset();
        });
    },
    fields: () => {
        return [
            name_field,
            new InputView({
                name: 'submit',
                label: 'Submit',
                type: 'submit'
            })
        ];
    }
});

Now, if someone were to fill out the name field with, say, "K" and then press the Submit button, it would prevent the form from being submitted and place the invalidClass on the field/label (maybe make the border of the field red or something) and place the error message next to the field. This is great. The user corrects it to "Kyle" and, as they type, this plugin changes the class on the field/label to the validClass (yay! green outlines and stuff!). So far so good. This is what the module has been doing forever.

Okay, so now the user presses the submit button again after fixing their mistake and it posts the field to the back-end, or whatever, and it clears the form using ampersand-form-view'sreset method so that it can be used again for something else.

form.reset();

And... here is the problem... the invalidClass and 'required' message are now applied to the name field even though it's empty and the form has been reset. This is because the field is required, was filled out, and is now empty again. If this were to happen in normal circumstances (not re-using the same form again), that would be the correct behavior. But, what if we wanted to basically fully reset the form, including the validation stuff, so that we can use the same form over and over again without re-rendering its parent view or refreshing the page?

That's where the PR comes into play.

Now we have several options on how to handle the scenario where we want our form to be in the exact state it started in...

Option 1: Completely clear the form and validation classes by passing true to the clear method This might be the simplest and most effective/useful option IMO. This will completely clear all the fields (regardless of what their initial values were when the form was initially rendered on the page) and the validation classes (and messages) would be removed..

form.clear(true);

Option 2: Do a full reset of the form by passing true to the reset method and clear validation classes This is very similar to option 1 but will just reset the form back to it's original state. So, if name field had a value of 'Bob' by default, it would now be reset back to 'Bob' (as opposed to cleared entirely) and the validation classes (and messages) would be removed.

form.reset(true);

Option 3: clear/reset specific fields of your choosing This is nice if you only need to reset 1 or two fields on a form with, say, dozens...

name_field.clear(true);
// ... or ...
name_field.reset(true);

Option 4: Have the clear and reset method also clear validation by default using a module option If you want this functionality by default for all times when you reset/clear a field (either directly or via the parent form's reset method), you can just set it in the field's options...

var name_field = new InputView({
    label: 'Name',
    name: 'name',
    required: true,
    clearValidationOnReset: true, // <-- THIS
    tests: [
        function(val) {
            if (val.length <= 1) return "The name must be more than 1 character!"
        }
    ]
});

var form = new FormView({
    fields: [name_field]
});
form.reset(); // <-- no need for TRUE to be passed here

Option 5: Just reset the validation (don't clear or reset the fields) This is useful if you just want to clear the validation classes for whatever reason. Not sure there's too many use-cases for this, but, I'm sure someone will think of one. Doesn't hurt to add it.

name_field.resetValidation()

I hope that makes it a bit clearer.

[cdaringe]

@kylefarris, amazing writeup. thanks so much!

with this new understanding, i'm on board. i'm going to speak through my thought process below. i'm sorry to keep dragging out the review, but:

• are you still convinced reset need the ability? select-view clears it by default. however, to counter that, clearing by default doesn't represent a true reset if validation was present on init. so 👍
• API. my personal pref would be for the API to be more akin to field.clear({ validate: false }). more specifically, an object based argument, to ++communication/readability. it's definitely not the status quo (shame on me!) but if this will be rippling back up to form and possibly into other fields, it may be a good opportunity to make it great 💪

[kylefarris]

No problem at all--I totally welcome (and appreciate) a thoughtful discussion on any PR. It's what makes open source software so much better than anything else.

That said, on your first point, I do think reset needs the option for it (even if its rarely used) purely for aesthetics and to meet what the end-user expects in such a scenario.

---

Contrived User Update Form:

var name_field = new InputView({
    label: 'Name',
    name: 'name',
    value: 'Bob'
    tests: [
        function(val) {
            if (val.length <= 1) return "The name must be more than 1 character!"
        }
    ]
});

var form = new FormView({
    el: this.query('#update_user_form'),
    fields: [name_field]
});

1. User highlights the default text in the name field containing valid value 'Bob' and continues to type replacement value, 'Robert'.
2. Form field flickers from red (invalid) to green as the character count climbs above 1.

3. User decides that they've changed their mind--they like the name 'Bob' and so they hit the 'Reset' button on the form which is tied to an event which triggers it's parent form's reset method.

   this.query('#update_user_form').onreset = function(e) {
      form.reset();
   });

4. Field is reverted back to 'Bob' but the form is now green unlike how it was when they initially loaded the page which may lead the end-user to think that something didn't work properly. Or they might just think it's weird/ugly and move on.

It might be a somewhat rare scenario but it would be there should a developer need such functionality.

---

As for point 2, I completely agree with you. I tend to prefer that style of parameterization in javascript as well. It does, however, make merging with defaults slightly more of a pain, maybe. Maybe not... there's several ways to go about it.

Which do you prefer?

1. Override directly in default object

function foo(params) {
    params = params || {};
    var options = {
        validate: params.validate || true,
        foo: (params.foo && typeof params.foo === 'object' && Object.keys(params.foo).length > 0) ? params.foo : {
            bar: 'baz'
        })
    };
}

2. Merge parameter object with a defaults object

function foo(params) {
    var options = {
        validate: true,
        foo: {
            bar: 'baz'
        }
    }; 
    if (params && typeof params === 'object' && Object.keys(params).length > 0)
        options = Object.assign({}, options, params);
}

---

My thoughts on it...

Option 1 would be more direct, faster, would be more-controllable, and would work on all browsers. It might get a little hairy for multi-level deep parameter objects (rare), though.

Option 2 might scale a little better (from a "code-cleanliness" standpoint) for items with a large number of options but would also be a bit slower and wouldn't work with browsers that don't support Object.assign (IE, of course). We could get around that by using lodash.assign but that's just another chunk of code to include (its probably already included through other modules, but, still..). Another potentially bad thing (although minor if things are coded properly) is that additional options could be merged into the default that we don't want or support.

[cdaringe]

_.merge is a good option to address collisions. _.defaults could be used as well. whatever is most fitting for what you observe in the codebase. _.assign is fine if parameters are flat-ish. call the ball, based on what you see