-
-
Notifications
You must be signed in to change notification settings - Fork 128
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
Validation events... #318
Comments
I'm open to adding events to the controller. We even have a decorator we could apply that would automatically add the One question about the use-case part of your comments:
Did you try the |
Yes, I played with that, but it requires the presentation information be duplicated from the renderer into the form for those fields. Re the Event Aggregator, I don't think this is suitable for validation events, as they are not global events. If the same concept could be applied to the scope of the ValidationController, great, but there is no need for the entire application to know when one field has validated. |
I agree, they are not global events. The decorator I linked is a means of reusing the event aggregator logic on a class instance. Any class whose instances need to expose an event api (publish + subscribe) can use the decorator to "mix-in" the functionality. |
Blog for you to check for accuracy. It tests the concept nicely, although (as I mention in the blog) it doesn't expose enough of the operation for all scenarios. |
@jsobell nice- a renderer that renders events. Should this issue remain open- have you been seeing a lot of questions related to validation events? |
No, not many, so let's close it for now. |
I actually ran into this today... I had three "components" which were small forms that represented child models of a parent model. Each component had validation setup to ensure the "value" was a numeric entry the "cat number" was required and less than 25 characters etc. However the editor that has these three components should only be enabled for save when the components are valid. Since I am using validateTrigger= validateTrigger.change, validation was correctly validating the fields when I was changing stuff, but unless I manually invoked validationController.validate() - which I do on initialization only (in a _.defer()) there was no place to hook into the promise (besides that would be a double validation). Instead I used the bindingEngine to listen to collectionObserver changes on the validationController.errors but this really only lets me know when there is a change in the errors of validation. So for example, on initial load, if the validator fires on binding, and there are no errors there is no way to "inform" that everything is "ok". The collectionObserver handler currently emits an event (which can be caught by the editor) that will check the validation state passed to see if all three are valid and if so trigger the buttons to not be disabled. My assumption of course was that forms were inherently invalid, so perhaps switching this to be assumed valid and wait for the validator to fail would be cleaner. Having a "hook" to attach a callback (ala listener pattern) on validation would be helpful here. I suppose an alternative I may explore is rather than raising events out of my components is to have my components have a valid state flag and set that in the collectionObserver handler internally, and then have my editor listen for changes in the component valid states with the normal variableChanged( ) functions. Either way applying a callback would be a lot easier to handle more cleanly... something like
|
@jadrake75 There is this that partially addresses what you mention: |
My current use case: I am validating at the object level, thus must manually call ValidationController.validate(). At that point I can manually publish an event myself. But if one is doing property-only validation, then validation can happen entirely under the hood, and some kind of built-in event or callback mechanism can then be desired. |
@jsobell I have to admit that approach would be cleaner in my case. I never considered it because in my head "renderers" are for rendering stuff (I think this is sort of @dkent600 comment (1) above) - for my code I am willing to use it and eliminate my bindingEngine watching on the errors collection (it would be cleaner) But I think the core issue is, is a workaround like above sufficient (even though mentally non-intuitive at first glance) vs. a cleaner way to hook into the validation without manually invoking it (and thus having a promise you can respond to) |
@jadrake75 No matter what you do you will always have to assign a validation controller to an object, whether through bindings auto-linking them or code doing it explicitly. I'm not sure what there might be as a 'cleaner' way of doing this. We could have events (callbacks) on the controller itself, but that's exactly what the ValidationRenderer is; a predefined set of callback hooks. It's also not practical to have the objects themselves raise events, as an object shouldn't have any additional data related to validation status, and this is why things like change tracking are difficult. Mixing UI validation with object validation is always going to be a compromise, but underlying the whole thing is the fact that the object itself has priority over the UI. Sure, you can have an object with validation on field 'X', and if you forget to make 'X' editable on-screen you can never save the object. My belief is that everything validation-wise has to be object based, and the UI is a secondary assistance layer to make it pleasant for users. If you mess up your UI stuff, the underlying system will still prevent you trying to submit invalid data. As such, anything related to validation extension needs to apply at the model layer, and because we don't extend models for validation, that means it has to be applied to the validation controller. Any ideas you have for ways to improve this stuff, please do post them all in here. I'm not arguing against any of your suggestions, simply trying to explain why some of the stuff has taken the direction it has, and if you have ideas that improve, extend, or find fault in anything as it stands we most definitely want to hear them! |
Possibly, although a second
Obviously,
So if you're doing property-only validation, what is the issue with the example I posted? Does this not get triggered when you validate a property? |
To be clear: I am doing object-level validation. But if one is doing property-level validation, then I would prefer not to ask the user to overload the ValidationRender. I would prefer to see a feature change whereby the ValidationController or some-such entity publish the desired events as a system entirely independent from rendering. I don't think that the validation modules needs to provide a ValidationEventHandler that you referred to. The user can write their own if that is how they would like to handle the published events. But if the ValidationRenderer could be renamed to be more general and be made to truly handle all the types of events, then that seems OK with me. |
That's where I'm a bit confused. The ValidationRenderer is not related to the UI, it's a service hook that can be used for the UI (typically for rendering). How else might you see this being done? Are you saying that you think there should be some other type of events generated by something? Can you give an example? |
Given the name "ValidationRenderer" and all the documentation covering it, it does appear to have been intended to be used in relation to rendering the GUI. It makes sense to me that going forward that intent should be respected, considering that other GUI-related functionality may be desired in the future, and that if this subsystem were also responsible for presenting events to the programmer, then one might start to encounter contention between the two purposes. This is the whole SRP thing. So yes, I was suggesting that a separate subsytem be used to present events to the developer. I'm not suggesting how that would be, just that it makes sense from a SRP point of view. In my mind, a pure event system (such as in what EventAggregator does) makes sense, but I'm not stuck on that by any means. There is also the question of whether ValidationRenderer, in its current state, is actually representing |
I would agree with @dkent600. The intent might not have been to have the ValidationRenderer only be a UI construct but the name implies it. It doesn't help that the main method of it is "render". A cleaner convention would be to have a ValidationResultHandler with a "process" or "evaluate" method. ValidationRenderer could either be an extension of this. My 2cents |
I just wanted to comment about the use of the EventAggregator extension pattern (I implemented it and testing around with it). So now I understand the comment better of @dkent600 about object validation. I am in the same boat. I want to know when the validators on my object are clear (or faulty) as a set not as they are validated. So hooking into the renderer lets me know when "a" validation has changed, but it does not allow me to evaluate the overall validation status of the controller. For example, using a case mentioned earlier, if you had a controller with 5 validation rules on 4 fields assigned after evaluation of the fields (ie. completion of validate()) it would be good to do something about it. However unless you are manually invoking validate( ) there is no way to hook into the promise. Maybe that "is" the solution. Provide a way to provide the callback of the promise success and failure and invoke it with the caller of the validate within the code in the validation module. To further this, in order to do what I needed, I had to pass the controller into the Renderer, which is kind of odd (I am sure this breaks a host of encapsulation rules) but it works.... `
} |
@jadrake75 You mean as mentioned here: #318 (comment) ? Re the nomenclature of the class, if we are to use it for non-UI related actions it would probably be better renamed, although most people will undoubtedly use it purely as a render management service, even to the extent of changing UI feedback on the basis of events. But... you have to bear in mind that the validation list is inherently unsafe and inaccurate, and should not be used as an indication of model validity. It's dangerous to assume that all because you have no errors in the collection that you object is somehow valid. Firstly, object level rules are not evaluated without a full So I suppose the core question here is that if the intention is to use it for model validation, when are you likely to ever want to know the status without validating the whole model? |
Excuse my misunderstanding... so if the validateTrigger is set to "change" you are not calling validate() internally which is analogous to a object model validation? You are only evaluating the rules for the defined model property? (I suppose that makes sense) That makes it a bit trickier since I figured there was a validation cycle on any change, but it sounds like it is only on the properties' change. So it sounds like the only way to evaluate and object model validation state would be via the promise of validate()? |
So even if there was a solution detecting changes to individual fields on an object, you still have all of the issues outlined above. It's impossible to predict every validation requirement, so the best we can do is to support object level fully, and make it as easy as possible to implement the UI validation. So yes, calling |
I think it is valid to think of validation in terms of two major use cases, either property-level or object-level. To answer your question above, "when are you likely to ever want to know the status without validating the whole model", I think it would be when we have no rules that require whole-object validation. This is precisely the use case that is most commonly presented in examples of the use of aurelia validation, especially when talking about the bindings. It is the use case that is most fleshed-out aurelia validation. |
@jsobell Further to your question "when are you likely to ever want to know the status without validating the whole model". I can only think of one use case where one would not need to call validate, where 1) one has no multi-property dependencies, and 2) one is assured to always be starting from a valid object, and 3) one is persisting the object each time a single property is altered with a valid value. (I actually have this case in some places in my application.) I can't think of any other case where one would not have to call validate() before persisting the object. I think this supports my case for making sure that the ValidationRenderer (or whatever we call it) works well in the context of whole-object validation and multi-property rules, which it currently doesn't. Of course, it also needs to work well when evaluating one property at a time, as it is currently primarily optimized to do. |
BTW, the fact that the ValidationRenderer does not work well in the context of whole-object validation and multi-property rules was my original motivation for requesting an alternative event mechanism. |
OK, but that's not what the whole-object validation refers to. Remember that we sometimes define a custom rule on a property, for example
Now if the user changes firstName from 'Fred' to 'Bob', this rule fails, but we don't know until we call We did discuss this in great detail when we were designing the solution, and the ideal would be to track dependencies between rules, but that is extremely expensive. An alternative is to allow the computedFrom style approach so the user can specify dependent properties, but this is also risky and requires dependency trees be tracked, making things potentially messy with predicates etc. The use case you just added "one is assured to always be starting from a valid object" is probably not a typical one, as most people are simply asking for a valid email and a few required fields, but unless they are editing an existing (previously validated) record, the rules for Can you explain what you mean by
Can you give an example of exactly what you would like to see happen? What events would you expect to see, and where? The current ValidationRender provides all failed rules, even if there's no element associated, so is that not what you wanted? |
yes, that is # 2 of the use case. I think this use case is common when editing simple entities. I'm not suggesting any changes based on that use case. I was simply answering your question to identify a use case where calling Regarding your other questions, can you please consult these: here and here. Thanks |
Yeah, I looked at those earlier. I did mention adding a metadata property in here #328, but the tag property essentially already does that. Your second post #279 (comment) seems to say the same thing, and again, I think |
@jsobell I think you had written a blog post I think on how to make an "event renderer". Does that cover the need or is there more we need to do here? |
People are constantly asking: "I have a Save button that I want disabled until all fields have passed validation. Is there a way to check that the entire form is valid"? followed by: "How can I know when the validation state may have changed so that I can refresh the button enabled state accordingly?". Clearly the answer to the first question can be something like "instantiate a The answer to the second question appears to have several potential answers, but the first choice appears to be: "check out Ideally, I think, there would be a cleaner way of doing this. But at minimum it would be nice to document this answer clearly in the Aurelia docs, with some example code. This is a very common use case. |
I think it is important to think in terms of Aurelia binding. I do not
disagree with you on those questions. Ideally your form should have a
state that is triggered as invalid when a validation within it is invalid.
I can not remember if we simply did this by having the renderer (which is a
poor name, it really is a handler or a processor) emit an event that is
caught by the form and toggles the state. If that state is two way bound
the parent panel reflects the form validity. Ie. When you include a form
into the panel you pass the form state variable in as two way bound. The
form recognizes invalid changes via an even and triggers the value. This
means you need a custom form element of course that wraps an HTML <form>.
It could certainly be easier that is for sure.
Jason
…On Jan 25, 2017 1:33 PM, "Doug Kent" ***@***.***> wrote:
People are constantly asking: "I have a Save button that I want disabled
until all fields have passed validation. Is there a way to check that the
entire form is valid"? followed by: "How can I know when the validation
state may have changed so that I can refresh the button enabled state
accordingly?".
Clearly the answer to the first question can be something like
"instantiate a Validator (via DI) and use it to validate the entire form
without affecting the GUI."
The answer to the second question appears to have several potential
answers, but the first choice appears to be: "check out
ValidationRenderer.render because it is called whenever the validation
state changes". This is a somewhat unintuitive answer to what seems like a
simple question.
Ideally, I think, there would be a cleaner way of doing this. But at
minimum it would be nice to document this answer clearly in the Aurelia
docs, with some example code. This is a very common use case.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#318 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADEpfp9Qw2ZNYwZwBGhLqA0to2xlFIV3ks5rV5WLgaJpZM4J10oN>
.
|
The complexity is that because the objects are validated, not the form, we don't know at the form or validator level if a property has been validated or not. Essentially, enabling and disabling 'Save' buttons on a form is a risky business. If there is a server-side check to make before save, you can't make that check unless it's enabled. So if you leave it disabled until the rest of the fields are valid, the user will get an error when they click the enabled 'save', and will think "WTF? If it's invalid, why did it enable the button?" Sure, there will be times when all you care about is whether all mandatory fields on the form are filled, but we don't provide a feature to enable it in those cases, as it will be misleading and inappropriate in other cases. |
Closing- not a lot of requests for events at this time. We definitely can revisit in the future, should the need arise. |
I would just like to refer to my last comment above, where I noted that this use case is very commonly asked-about, and concluded "Ideally, I think, there would be a cleaner way of doing this. But at minimum it would be nice to document this answer clearly in the Aurelia docs, with some example code. This is a very common use case." |
ok- re-opening 👍 |
Whoa! That was some fast coding! |
There are times when it's helpful to know when the validation collection has been changed, for instance enabling/disabling buttons, and having custom error displays for specific rules/properties.
Is it worth adding an event to the controller class so VMs can register to it?
One use-case I was experimenting with was how to display a specific property's error code in the UI. Obviously the errors collection can contain the error, but I don't see an obvious way to validation message for property "A", as without a Signal the message display fields won't refresh.
I suppose we could tell people to add another validation renderer to generate a signal, but it's all a bit inelegant.
The text was updated successfully, but these errors were encountered: