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

Bug #1831390 reported by Jane Sandberg on 2019-06-02
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Undecided
Unassigned

Bug Description

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 (sandbej) wrote :

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 (sandbej) wrote :

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 (sandbej) wrote :

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) on 2019-06-28
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
status: New → Confirmed
Bill Erickson (berick) wrote :

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.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Jane Sandberg (sandbej) wrote :

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

tags: added: pullrequest
removed: needsdiscussion
Jane Sandberg (sandbej) wrote :

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 (sandbej) wrote :

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

Bill Erickson (berick) wrote :

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.

Changed in evergreen:
milestone: none → 3.4-beta1
tags: added: signedoff
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers