Conversation dump

So I talked with Luke last night for a while and I think we have some general ideas for how we can split the two repositories up and get some tasks in place. The big blocker that I can foresee is that thus far I've only been helping with some maintenance and easy tasks so I need to dive deeper in to the 'flow' of validation to make sure we maintain everything properly. I spent a few hours last night combing through the code to get a better technical understanding. I'm proposing that for the beta releases we make sure the validation library is rock-solid and that aurelia-templating-validation is priority #2. Removing the DOM manipulation that currently takes place in Validation seems like it is going to greatly simplify things and enable implementing better hooks for templating but it seems that the DOM-related stuff is all Bootstrap and I'm not certain if it adds as much value up front as having a stabilized and rock-solid API for validation. Most of the DOM manipulation should be able to be accomplished in a way that's not as dependent on DOM structure anyway so it's likely to have more changes than validation anyway. Thoughts / feedback?

jdanyow Oct 27 11:33
Agree- couple thoughts:
Does the validation repo need to be considered beta or can it remain alpha while this stuff is sorted out?
And, it would be great if Aurelia-breeze could reuse whatever error renderers that come out of the refactor

PWKad Oct 27 11:36
I think it's fine to leave in Alpha - it is a pretty heavily used plugin though so if we did would probably want to get it up to beta soon

jdanyow Oct 27 11:36
Here's what I mean about the error renderers

PWKad Oct 27 11:36
So aurelia-breeze would have an optional dependency on the templating piece?

jdanyow Oct 27 11:36
https://github.com/jdanyow/aurelia-breeze/blob/master/src/error-renderer.js
This is in the middle of a refactor but it's the general idea
If Aurelia validation offered bootstrap form error render and a bootstrap grid error renderer and others for foundation,etc
Aurelia-breeze has a dep on the ErrorRenderer interface and can use whatever has been configured in the container

jdanyow Oct 27 11:45
actually- what do you think about this-

jdanyow Oct 27 11:51
I take out all the validation code in aurelia-breeze.
aurelia-validation is split up into a few main interfaces: ErrorRenderer, EntityErrorObserver, PropertyErrorObserver. These have getObserver(entity) and getObserver(entity, propertyName) respectively. A lot like how the binding engine observes property changes, the validation engine observes validation state changes
aurelia-breeze registers implementations of EntityErrorObserver, PropertyErrorObserver. aurelia-validation also has it's own standard set that works off from the fluent API and property observation system.
aurelia validation uses the view to find all the two-way bindings on the form/grid/whatever (a better implementation of what this code is doing).
At that point the validation engine knows what all the inputs, all the entities and properties are. It can then use the observers to watch the validation state, and the registered renderers to farm out the display of the errors

PWKad Oct 27 11:52
Does the ErrorRenderer belong in the aurelia-templating-validation lib in that scenario?

jdanyow Oct 27 11:53
i would say yeah
validation would export these interfaces and potentially some implementations for bootstrap unless we feel that the implementation should be externalized

PWKad Oct 27 11:54
So EntityErrorObserver would be for observing the breeze entities and PropertyErrorObserver would be for observing POJO's
Ah I see what you mean - I think the concept in Validation is currently a ValidationGroup which would be an Entity in the example above

jdanyow Oct 27 11:55
EntityErrorObserver would be for observing errors that occur that are not specific to a particular property. PropertyErrorObserver would be for property specific errors
there would be implementations of both for POJOs and for Breeze entities
ah yes
maybe EntityErrorObserver is a bad name

PWKad Oct 27 11:56
Got it - I believe the way the lib currently works is to combine the PropertyValidations in to a group of validations - the group of validations make up an Entity so do you think it would be ok to use the same nomenclature?

jdanyow Oct 27 11:58
i think so- i'm probably thinking in too much of a breeze-specific manner

PWKad Oct 27 11:58
No I agree I always do the same since I use it so much

jdanyow Oct 27 11:59
so the way the current aurelia-breeze validation works is they put an attribute on a container element (a form or div or something) and it goes and finds all the two-way bindings to breeze entities within. The attribute is either bound to a single breeze entity or a breeze entity manager.
I think in the new way there could still be an attribute
but we wouldn't bind it to a particular entity or entity manger or object
because we have the observers to find out if errors are observable for a particular object or property
and instead the attribute could be bound to nothing or to the name of a particular error renderer
so they can choose whether they need the form renderer or the table renderer or some custom renderer
not sure if that makes sense or not, just an idea

PWKad Oct 27 12:12
Eating lunch will respond in a few

PWKad Oct 27 12:33
So do you think it would make sense that the ValidationGroup would have a optional ErrorRenderer? I need to look in to the grouping and how it works but I think you can have a group of groups if you want as well to cover larger forms or related entities.

jdanyow Oct 27 12:40
not sure- i'm not familiar with what aurelia-validation supports. I've been looking at it with a narrow view, probably too narrow. In terms of aurelia-breeze this has been:
there's an area on the screen (form/div/table) containing two-way bindings.
At the top of this "validation area", a validation summary needs to appear that shows all errors (property specific and non-specific), as they occur.
When a property specific error occurs some sort of indication should be shown near the input.
multiple instances of the these "validation areas" should be supported

PWKad Oct 27 12:47
Yeah so I think currently the validation library does a similar function where it crawls the DOM downward looking for properties that need to validate
It has a custom attribute which seems to be added to enable this
I'm not a big fan of that method just because it takes control away a bit and makes things seem to magical for me but I could go either way - I was thinking if the error renderer was on a group of entities / objects / properties then you could control it a bit more without having to traverse the DOM
I think this is what does the crawling at the moment - https://github.com/aurelia/validation/blob/master/src/validate-custom-attribute.js