Extend FormController's API to access list of form fields

[slavafomin]

Hello!

Thank you for your hard work!

I want to propose a feature to list fields of the form.

Right now we can access individual form fields using form[fieldName] expression. However, form controller has a lot of properties and methods, so it's not possible to iterate the list of form fields directly.

I have to use this code to iterate all fields of the form:

var fields = ['firstName', 'lastName', 'email'];
angular.forEach(fields, function (fieldName) {
  var field = form[fieldName];
  // ...
});

However, it's a bad practice to hardcode array of field names this way.

The proposed API will allow us to do something like this:

angular.forEach(form.$fields, function (field) {
  // ...
});

The other workaround is to iterate all properties of the form controller and detect ngModel objects, however it is also somewhat ugly.

I hope it makes sense. Thank you!

[Narretz]

What's the use case for which you need a list of all form controls?

[slavafomin]

I'm checking underlying form fields for their validation states in order to determine entire form's validation state. Otherwise, I will have to use a very long and ugly expression in HTML.

[wesleycho]

This sounds like a nice-to-have feature - if one has a dynamic form, then one has to somehow create a tracking mechanism to keep track of all of the specific form components. Something exposed like formController.$controls could provide a convenience at minimal cost to the internals I think.

[slavafomin]

Also, the Form Controller's interface would be more organized and professional this way (if you care about good architecture).

[petebacondarwin]

@slavafomin I guess that you are aware that the form has a validation state of its own, which is already a composition of the validation states off the inputs in the form?

Are you trying to do something more fine grained, such as know the validation of only a subset of the inputs in a form?

If so, and if the inputs that you care about are sequential, then you could group them under ngForm directives and then look at the validation of the ngForms.

Or perhaps you want to do something even more clever...

Can you provide more detail of your use case?

[slavafomin]

@petebacondarwin, sure. I'm looking for fields which are both $touched and $invalid and then I'm displaying a notification to the user that he should fix the errors.

[slavafomin]

Also, from the extensibility perspective, some developer could decide to add additional properties or logic to the Model Controller, like I did in Input Modified module. And it's a good thing to have ability to iterate all fields of the form to aggregate some data from them, like to check if they are all modified or not.

[petebacondarwin]

I don't really want to keep a totally new collection ($fields) in sync with the collection of controls on a form.
I feel OK about exposing the list of controls on the FormController, except that this might allow them to be modified, which would be a pain.
If we could do it in a read-only manner then that would be ideal. Any ideas?

[wesleycho]

Maybe just an array of the control names would work, that way it is up to the user to use that to iterate over the form controls to access them.

[petebacondarwin]

OK, I guess we could do that - just have to keep it in synch via the addControl, removeControl methods.

[gkalpak]

IIRC, we had discussed that it makes sense to expose the list of controls in another issue (but I can't find it). I don't see how exposing just the names makes it safer.

We should sure document that you should not add/remove elements to/from the list, but the same is true whether we expose the controls themselves or just their names.

If we are worried that modifying the list would mess with our internal implementation and want to avoid that, we can expose a "copy" of the list. We will have to keep that in sync with our list used internally, but this is no different if we return just the names.

[slavafomin]

I think a warning considering read-only state of the list is enough in the documentation. However, returning only names is a poor «interface» decision in my opinion. The end developer will have to do an extra work to fetch the actual list of models.

[petebacondarwin]

The thing about having a collection called $controls is that is kind of looks like it is the sort of thing you could just add to or take away from ... compared to having these properties directly on the controller.

Having just the names would not encourage people to think of it as a mechanism to add or remove controls.

Returning a copy of the controls controls would not work because these controls are mutable objects that have validation values that you would want to check... A shallow copy would be fine - since we would only want to prevent adding and removing items from the original collection.

Iterating over the names to get the controls would not be very arduous...

formController.$controlNames.forEach(function(name) {
  var control = formController[name];
  ...
});

But I guess if we document that $controls is not to be modified, then it is the simplest solution

[slavafomin]

To make it more obvious (and safe), we can expose it via getter-function, e.g.

NgModelController[] FormController.$getControls().

[petebacondarwin]

So if $getControls() returned a shallow copy of the internal controls collection then that would be safe.
What do others think?

[gkalpak]

I'm fine with $getControls(), but I would rather return a synchronized copy of the internal list of controls.

Update: Would be happy either way actually. It is probably easier/safer to create a new copy each time, but more time-consuming on forms with lots of inputs and/or nested forms.

[Narretz]

What's a synchronized copy?

[gkalpak]

A synchronized copy is a copy that is kept in sync with the original. I.e. whenever we add/remove a control to/from the original (internal) array, we should make the equivalent modification to the copy as well.