aurelia / validatejs

Enables expressive validation using decorators and/or a fluent API.
MIT License
22 stars 23 forks source link

"Bootstrap" renderer doesn't remove previous errors when re-validating. #39

Closed apawsey closed 8 years ago

apawsey commented 8 years ago

Given a property with both a required, and email validator, if I leave the form blank and execute validate (using fluent api so have to execute it manually at the moment), it correctly adds the message that "Email can't be blank" (by adding a span element to the form-group).

However if I now fill the field in, but not with a valid email address, and then validate again, the field is displayed with both errors (can't be blank, and not a valid email), even though the first is now no longer applicable.

See resultant HTML below:

<div class="form-group label-floating form-group-lg has-error">
    <label class="control-label" for="register-email">Email</label>
    <input class="form-control au-target" id="register-email" type="text" value.bind="email & validate" required="" autofocus="" au-target-id="19">
    <span class="material-input"></span><span class="help-block">Email can't be blank</span><span class="help-block">Email is not a valid email</span>
</div>
plwalters commented 8 years ago

I think for this we just need to clear out previous messages when re-rendering, even if new messages exist to prevent a duplicate. I think I saw this before with the exclusion rule when the message was different each time.

apawsey commented 8 years ago

Yeah from what I can see, you only clear the errors if there are no errors at that point in time. I assume it should be clear errors, then post new ones if they exist. Any chance you'd like me to submit a PR for this? I'd like to start getting involved somewhere.

plwalters commented 8 years ago

That would be great! If you haven't already just make sure to sign the CLA when you submit the PR (and make sure the commit messages match the format we use for changelog generation such as fix(messages): always clear error messages on validate)

apawsey commented 8 years ago

Busy putting together my PR... :)

I have a question on one of the issues with the current renderer - just wondering the most appropriate 'Aurelia' way to resolve it.

Currently when the renderer removes the elements representing errors (ie containing the error messages), it matches them against any node with the class 'help-block', but that doesn't differentiate against elements that may have pre-existed the renderer adding new elements... if that makes sense.

Basically we don't know if an element with the class 'help-block' was created by us or not. It seems elsewhere in aurelia, you add marker classes to differentiate elements that aurelia created, manipulated. Should we do the same here? Like add "au-validation-error" or something? Otherwise what other suggestions are there to mark the element as essentially belonging to the renderer?

Thanks.

EisenbergEffect commented 8 years ago

You could add a data- attribute, class or special au- attribute to differentiate. Any would be fine I think. It's an interesting scenario. Thanks for bringing it up.

plwalters commented 8 years ago

@apawsey Let's go with au-validation for now and if we need to clean it up later it should be easy since it's not something anyone outside the lib should be relying on. Then the selector can simply be .help-block.au-validation

Sound good?

apawsey commented 8 years ago

All good thanks, will be done with this later today.

Will start docs on custom renderers as well.