Make Angular combobox, date-select, etc. implement ControlValueAccessor

[Jane Sandberg]

Here's a discussion item/request.

I propose that we change some of our Angular components so that they can be easily used with Angular's Reactive forms. I had the combobox, date-select, and org-select in mind. My understanding is that this would require us to implement ControlValueAccessor within the individual components, and add NG_VALUE_ACCESSOR to our providers.

A plug for reactive forms: I've been experimenting with them lately. They are pretty handy to work with: I particularly appreciated their validation capabilities and how they create handy Observables for each field. Plus, Reactive Forms is a pretty popular way to create forms in Angular, so it's easy to find code examples.

My understanding is that implementing ControlValueAccessor would also allow us to use ngModel and ngModelChange on form fields, which would provide a smooth transition for folks used to the AngularJS client.

Thoughts?

[Jane Sandberg (sandbergja)]

Here's a proof of concept branch: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1831390_combobox_date_select_ControlValueAccessor

user/sandbergja/lp1831390_combobox_date_select_ControlValueAccessor

This branch ControlValueAccessor-izes combobox and date-select. It also includes a few sandbox examples, featuring date-select and combobox used with [(ngModel)], and combobox used with a reactive form and Angular validation.

I found this tutorial about ControlValueAccessor very helpful: https://blog.thoughtram.io/angular/2016/07/27/custom-form-controls-in-angular-2.html

Let me know what you think!

[Jane Sandberg (sandbergja)]

P.S. if we think this is a good direction to move in, we'll also need to decide on the best format for writeValue and propagateChange to accept for date-select -- this branch has YMD, but javascript Date or ISO date format might be preferable.

[Jane Sandberg (sandbergja)]

One issue with this branch is that registerOnTouched should be called on a blur event, according to the documentation: https://angular.io/api/forms/ControlValueAccessor#registerOnTouched

[Bill Erickson (berick)]

Jane, this is awesome. Thanks for diving into this. +1 all around from me.

I have pushed a continuation branch, partly for my own experimentation, but also to apply some changes and a signoff.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1831390-ang-form-control-value-accessor

From the commit:

    Make eg-date-select traffic in Date objects instead of YMD strings.
    Added simple combobox [(ngModel)] example.
    Added combobox freetext testing
    Avoid forcing startIdFiresOnChange for combobox.
    Avoid redundant FormsModule import.
    Minor lint repairs.

I went ahead with the date select Date change since that seems to the typical way date widgets pass data.

[Jane Sandberg (sandbergja)]

Thanks for working on this, Bill. Here's a branch that signs off on your changes, and also fixes the registerOnTouched issue I mentioned in comment #3:

user/sandbergja/lp1831390-ang-form-control-value-accessor-continued

[Jane Sandberg (sandbergja)]

I just force-pushed a quick change to my branch that:

1) fixes a copy/pasto
2) improves how the datepicker displays default values set with reactive forms or ngModel

[Jane Sandberg (sandbergja)]

Here's a rebased branch: user/sandbergja/lp1831390-ang-form-control-value-accessor-continued-and-rebased

[Bill Erickson (berick)]

Sign-off to final commit:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1831390-ang-form-control-value-accessor-2

I did a bit more testing and all looks good to me. Existing date selectors, for example, continue to behave as expected.

[Jane Sandberg (sandbergja)]

Just to continue the branch fun, I've pushed another branch: user/sandbergja/lp1831390-ang-form-control-value-accessor-3

Here's a link: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1831390-ang-form-control-value-accessor-3

This adds a commit that fixes some undesirable behavior related setting default values for Comboboxes. Previously, the startId was not working when a combobox was used with ngModel or reactive forms. This is because the writeValue method was rudely clobbering the startId value with null values. This commit stops the clobbering. It also makes the behavior of ngModel more consistent by always using ComboboxEntry for its value.

[Galen Charlton (gmc)]

Rebasing was easy and the branch works, so I've merged this to master. Thanks, Bill and Jane!