aurelia/validation

validateOnChangeOrelseBlur

pvalsecc opened this issue · 23 comments

I'm submitting a feature request

  • Library Version:
    1.2.3

Current behavior:

There are four types of validation binding behaviors, but none I find useful in a UX point of view.

Expected/desired behavior:

If a field is not set, not modified or not in error, I like the validateOnBlur mode. That way, the user doesn't get distracting errors while typing. Then he gets an error when he goes to the next field.

But, a field that is already in error I'd rather use the validateOnChangemode which would make the error disappear as soon as the field gets correct. Then on blur, the mode could switch back to something to validateOnBlur.

Basically, that would be a validateOnChangeOrelseBlur mode. I didn't find a way to plug that mode into the library.

I've been thinking about something similar to this. I prefer delaying the first validation of all form fields until submit (this is why). But I agree that once a validation error has been rendered, the validation trigger should change so that the user can get rid of the error quickly. Whether that trigger should be blur, change or input is up for debate so I suggest leaving that configurable.

So I suggest that each validation controller should come with two trigger settings: a standard trigger, and one for fields with rendered errors. These settings should be overridable per controller, and the defaults configurable for the entire plugin, just as is the case with the current single trigger.

What do you think @bigopon?

I don't have a clear picture of how the two triggers per controller should translate to binding behaviors. I suspect the current model of having various named behaviors won't hold. Perhaps something like this:

<input value="myVar & validate:'manual':'blur'">

Would anyone like to give their thoughts on this? @bigopon @EisenbergEffect @fkleuver

This certainly makes sense from a UX perspective. We're open to taking a PR to add this but I'm not sure that our core team has the bandwidth to work on this now. @Sayan751 may have some thoughts on this since he's working on validation for Aurelia 2. Whatever new feature we add here, we'll definitely want to also have there.

As a matter of fact today I tested a prototype implementation for this. Although the testing for this is not yet quite complete, I can at least share my thoughts on this.

From my perspective, there is not much point in supporting two triggers, one initial and one changed later. In my prototype, I removed all the clutter of configuration and simply did this.

  • The new validate binding behavior calls the validation controller on 2 events: on change and on blur (or rather on focusout).
  • The on change trigger does not take effect until the property associated with the binding is validated once, either by manual validation or event-triggered validation (by blur/focusout). If the property is updated, the binding sets a isDirty flag to true.
  • The event-triggered validation validates the property iff it is dirty.

From the primary glance, this strategy seems to be working in my project quite well, although it is not fully unit tested. IMO this is a fairly straightforward strategy for the desired UX behavior, and thus need no initial and secondary configurable trigger.

@EisenbergEffect @RomkeVdMeulen What do you think about this approach?

Definitely would love to hear if this would enable the UX needed by @RomkeVdMeulen and @pvalsecc

I would like to propose that this behavior should be made the default behavior that comes with changeOrBlur and subsequently with changeOrFocusout trigger (see #543). As this is a sort of breaking change, it can be released via a major version update.

I have prepared something testable. The changes are here: https://github.com/Sayan751/validation/tree/issues/543-focusout-trigger.

You can give it a try to check if it suffices your requirement.

@Sayan751 I'd say that the changes you've implemented solve @pvalsecc's original request, and makes for a better UX than the current version.

However, I'm not convinced that this is the overall best possible UX. It's all a matter of perspective of course. I'm particularly focused on accessibility, and making the validation easy to understand for people with fewer cognitive resources. I'm worried that revalidating on every change could cause error messages to disappear and re-appear, causing confusion.

My colleagues and I have been giving this a lot of thought. The best scenario that we could come up with was this:

  • No errors appear when you first fill out the form. Only once you submit and there are still errors, will those errors be shown.
  • If you change the value of any field with an error message, that error message disappears. Note that his doesn't mean the input is immediately revalidated: the error is simple unrendered.
  • When you re-submit the corrected form, it is revalidated and any new or recurring errors are once again shown.

I think that this behaviour most closely resembles how people are naturally inclined to enter and correct input. It resembles the way I compose prose: write, read back, revise, read back.

Now that I've written all this out, I recognize that this is a departure from the current behaviour in more ways than one. I'm not exactly sure how I would make this scenario possible while maintaining backward-compatibility. I'm curious to hear your thoughts on this, @Sayan751 and @EisenbergEffect .

@RomkeVdMeulen Thank you sharing your thoughts on this. It helps me understand the different approaches people consider for validation.

You have correctly said that

It's all a matter of perspective of course.

I don't want to force my outlook on you. I just want to mention that I feel differently on some parts of your approach.

If you change the value of any field with an error message, that error message disappears.

I feel that if a field is validated once to be invalid, and when I start typing/changing the field the error message disappears, I would take that this as a hint that the value is now correct. Therefore, when I resubmit the form and get a validation error for the same field, I would find that frustrating.

When you re-submit the corrected form, it is revalidated and any new or recurring errors are once again shown.

If the errors are only shown when the form is submitted, in my opinion it increases the feedback time. Moreover, that way the user will be presented with a list of errors all of a sudden. And that is not a desirable UX from my perspective.

Having said that, the workflow you described, actually can be implemented with the current state of the plugin.

  • Use manual trigger, and validate your form on submit
  • Once a property is changed reset the error using the ValidationController API:
controller.reset({ object: person, propertyName: 'firstName' });

Therefore, when I resubmit the form and get a validation error for the same field, I would find that frustrating.

This is a good point. On the other hand, I can also imagine some people would find it distracting if the UI updates while they're trying to focus on correcting the field. I also expect that a second validation error after the user has made a correction would be very uncommon, but that depends on the kind of input of course.

If the errors are only shown when the form is submitted, in my opinion it increases the feedback time.

You focus on feedback time, and I can certainly understand that. I used to think similarly regarding validation feedback. What changed my opinion is this research: Why Users Make More Errors with Instant Inline Validation. The key point there was:

This finding can be explained by understanding the different modes users enter when completing a form. Users enter completion mode when they initiate a form. They’re focused on filling out each field regardless of making mistakes. After filling out each field, they switch to revision mode and are then focused on correcting errors.
Inline validation forces users to switch from completion to revision mode constantly. Switching modes back and forth splits and interrupts the user’s focus. A split and interrupted focus increases their cognitive load, causing more errors that lead to longer completion times.

Which is why I'm now more inclined to do at least the very first validation pass only on submit.

Having said that, the workflow you described, actually can be implemented with the current state of the plugin.

You're right, I hadn't considered that. Perhaps I should simply implement my own input components that behave like this instead of trying to get it supported out of the box in this plugin.

That reminds me: there's one more edge-case to consider. If you have multiple fields and the validation of one depends on the other, validating to early can actually show an error when a user simply hasn't finished yet. I ran into this when making a form for creating a DNS record which was split up into name, type and value. The validation rules for the name and value of a DNS record depend on the type.

But like I said: this is an edge-case and perhaps here the developer should simple add validateManually anyway.

I had to hack around a lot to get this to work in a generic form input component that doesn't know which property is being validated. I came up with this:

onInput() {
  for (const binding of (<Map<Binding, any>> (<any> this.validationController).bindings).keys()) {
    if ((<any> binding).target === this.inputElement) {
      this.validationController.resetBinding(binding);
    }
  }
}

Since the bindings are supposed to be private on the controller, I'd love it if we could include part of the above behavior as a public method of the validation controller. Something like resetBindingsByTarget(target: any). Does that sound like a good idea?

I've created #545 to add resetBindingsByTarget and validateBindingsByTarget.

@RomkeVdMeulen It might be cheaper every way to inherit the ValidateBindingBehavior, and override the bind method. That way you don't have to iterate over the bindings for every change.

Hmm, interesting. I'll give that a try next thursday.

@EisenbergEffect I think it is difficult to standardize all UX behavior in one implementation. That will be overly complex to maintain and possibly error prone. Therefore my proposal is to accept community contribution for such new behavior in form of new BB implementation. That should keep things simple and separate.

I agree with the original issue, and think the default impl. of the changeOrEVENT BB should be changed. However, I am ready to create a new impl. for this. Only problem I have is naming this new thingy. If you think we should go that way then please let me know.

@Sayan751 Are you saying to add a new BB from the community AND update our own BB with new behavior? or are those one and the same thing?

@EisenbergEffect Firstly, I would like to see our changeOrEVENT validation binding behaviors get updated with the new behavior (refer #544). That should close the original issue.

Secondly, referring to our earlier conversation on this topic, it is difficult to support all possible UX behavior in one implementation. Therefore, I suggest that if the need arise to support more behaviors out of the box, community contribution for a new BB, implementing the new UX behavior, can be accepted without changing the default implementation. As long as every BB is clearly documented, this should work.

@Sayan751 I suggest that if additional BBs are going to be encouraged we make some of the code currently in ValidateBindingBehaviorBase a bit easier to reuse. Lines 16-32 for example would probably be necessary for any custom validation BB.

Thanks to @Sayan751's suggestion I found that making a custom BB for my own validation trigger is pretty simple. I came up with this:

import {Optional,TaskQueue} from "aurelia-framework";
import {autoinject} from "aurelia-property-injection";
import {getTargetDOMElement, ValidationController} from "aurelia-validation";

/**
 * Indicated the bound field should be revalidated when the user has entered and
 * then later revised the input.
 */
export class ValidateOnRevisionBindingBehavior {

	@autoinject
	private taskQueue: TaskQueue;

	bind(binding: any, source: any, rulesOrController?: ValidationController | any, rules?: any) {
		// identify the target element.
		const target = getTargetDOMElement(binding, source);

		// locate the controller.
		let controller: ValidationController;
		if (rulesOrController instanceof ValidationController) {
			controller = rulesOrController;
		} else {
			controller = source.container.get(Optional.of(ValidationController));
			rules = rulesOrController;
		}
		if (controller === null) {
			throw new Error("A ValidationController has not been registered.");
		}

		controller.registerBinding(binding, target, rules);
		binding.validationController = controller;

		binding.validateChangeHandler = () => {
			if (!binding.valueEntered) {
				binding.valueEntered = true;
			} else {
				this.taskQueue.queueMicroTask(() => controller.validateBinding(binding));
			}
		};
		binding.validateTarget = target;
		target.addEventListener("change", binding.validateChangeHandler);
	}

	unbind(binding: any) {
		if (binding.validateChangeHandler) {
			binding.validateTarget.removeEventListener("change", binding.validateChangeHandler);
			binding.validateChangeHandler = null;
			binding.validateTarget = null;
			binding.valueEntered = null;
		}
		binding.validationController.unregisterBinding(binding);
		binding.validationController = null;
	}
}

Like I said: the first few lines in bind() are copied from ValidateBindingBehaviorBase and it would be helpful if those were in some reusable form.

@RomkeVdMeulen Very nice :) I agree with you that the Base implementation should be part of the public API.

@Sayan751 Let's move ahead with your plan. That all sounds great to me.

Really nice to see the community working together with us and arriving at a solution that works for everyone 😄