feat: improve control directive to not set value if not necessary + some first tests

Author: MrWolfZ

src/control/directive.spec.ts

@@ -0,0 +1,91 @@
+import { ElementRef } from '@angular/core';
+import { ControlValueAccessor } from '@angular/forms';
+import { Action, ActionsSubject } from '@ngrx/store';
+import { Observable } from 'rxjs/Observable';
+import { BehaviorSubject } from 'rxjs/BehaviorSubject';
+import 'rxjs/add/operator/first';
+import 'rxjs/add/operator/count';
+
+import { SetValueAction } from '../actions';
+import { createFormControlState } from '../state';
+import { NgrxFormControlDirective } from './directive';
+
+describe(NgrxFormControlDirective.name, () => {
+  let directive: NgrxFormControlDirective<string>;
+  let elementRef: ElementRef;
+  let document: Document;
+  let actionsSubject: ActionsSubject;
+  let actions$: Observable<Action>;
+  let valueAccessor: ControlValueAccessor;
+  let onChange: (value: string) => void;
+  let onTouched: () => void;
+  const FORM_CONTROL_ID = 'test ID';
+  const INITIAL_FORM_CONTROL_VALUE = 'value';
+  const INITIAL_STATE = createFormControlState<string>(FORM_CONTROL_ID, INITIAL_FORM_CONTROL_VALUE);
+
+  beforeEach(() => {
+    elementRef = { nativeElement: { focus: () => void 0, blur: () => void 0 } } as any as ElementRef;
+    document = {} as any as Document;
+    actionsSubject = new BehaviorSubject<Action>({ type: '' }) as ActionsSubject;
+    actions$ = actionsSubject as any; // required due to mismatch of lift() function signature
+    valueAccessor = {
+      writeValue: () => void 0,
+      registerOnChange: fn => onChange = fn,
+      registerOnTouched: fn => onTouched = fn,
+      setDisabledState: () => void 0,
+    };
+    directive = new NgrxFormControlDirective<string>(elementRef, document, actionsSubject, [valueAccessor]);
+    directive.ngrxFormControlState = INITIAL_STATE;
+    directive.ngOnInit();
+  });
+
+  afterEach(() => {
+    directive.ngOnDestroy();
+  });
+
+  it('should write the value when the state changes', () => {
+    const newValue = 'new value';
+    const spy = spyOn(valueAccessor, 'writeValue');
+    directive.ngrxFormControlState = { ...INITIAL_STATE, value: newValue };
+    expect(spy).toHaveBeenCalledWith(newValue);
+  });
+
+  it('should not write the value when the state value does not change', () => {
+    const spy = spyOn(valueAccessor, 'writeValue');
+    directive.ngrxFormControlState = INITIAL_STATE;
+    expect(spy).not.toHaveBeenCalled();
+  });
+
+  it('should not write the value when the state value is the same as the view value', () => {
+    const newValue = 'new value';
+    onChange(newValue);
+    const spy = spyOn(valueAccessor, 'writeValue');
+    directive.ngrxFormControlState = { ...INITIAL_STATE, value: newValue };
+    expect(spy).not.toHaveBeenCalled();
+  });
+
+  it('should dispatch an action if the view value changes', done => {
+    const newValue = 'new value';
+    onChange(newValue);
+    actions$.first().subscribe(a => {
+      expect(a).toEqual(new SetValueAction(INITIAL_STATE.id, newValue));
+      done();
+    });
+  });
+
+  it('should not dispatch an action if the view value is the same as the state', done => {
+    onChange(INITIAL_STATE.value);
+    actionsSubject.complete();
+    actions$.count().subscribe(c => {
+      expect(c).toEqual(0);
+      done();
+    });
+  });
+
+  // TODO: throwing error on undefined state
+  // TODO: value conversion
+  // TODO: mark as touched
+  // TODO: disabling and enabling
+  // TODO: focus tracking
+  // TODO: last keydown code tracking
+});

src/control/directive.ts

@@ -15,6 +15,7 @@ import { ActionsSubject } from '@ngrx/store';
 import { Observable } from 'rxjs/Observable';
 import { BehaviorSubject } from 'rxjs/BehaviorSubject';
 import { Subscription } from 'rxjs/Subscription';
+import 'rxjs/add/operator/filter';
 import 'rxjs/add/operator/map';
 import 'rxjs/add/operator/distinctUntilChanged';
 
@@ -44,6 +45,7 @@ export class NgrxFormControlDirective<TValue extends FormControlValueTypes> impl
   @Input() ngrxEnableFocusTracking = false;
   @Input() ngrxEnableLastKeydownCodeTracking = false;
 
+  // TODO: move this into a separate directive
   // automatically apply the attribute that's used by the CDK to set initial focus
   @HostBinding('attr.cdk-focus-region-start') get focusRegionStartAttr() {
     return this.state && this.state.isFocused ? '' : null;
@@ -58,6 +60,15 @@ export class NgrxFormControlDirective<TValue extends FormControlValueTypes> impl
   // the <any> cast is required due to a mismatch in the typing of lift() between Observable and BehaviorSubject
   private get state$(): Observable<FormControlState<TValue>> { return this.stateSubject$ as any; }
 
+  // we have to store the last reported value since when the action to update the state
+  // is dispatched a new state will be received inside the directive, which in turn would
+  // trigger a view update; however, most input elements move the cursor to the end of the
+  // input when a new value is written programmatically which means whenever the user
+  // types something the cursor is forced to the end of the input; to prevent this
+  // behavior we compare the last reported value with the value to be set and filter out
+  // those values that are equal to the last reported value
+  private lastReportedViewValue: TValue;
+
   constructor(
     private el: ElementRef,
     @Inject(DOCUMENT) private dom: Document,
@@ -71,9 +82,14 @@ export class NgrxFormControlDirective<TValue extends FormControlValueTypes> impl
   @Input() convertModelValue: (value: TValue) => any = value => value;
 
   ngOnInit() {
-    this.valueAccessor.registerOnChange((newValue: any) => {
+    if (!this.state) {
+      throw new Error('The form state must not be undefined!');
+    }
+
+    this.valueAccessor.registerOnChange((newValue: TValue) => {
       newValue = this.convertViewValue(newValue);
       if (newValue !== this.state.value) {
+        this.lastReportedViewValue = newValue;
         this.actionsSubject.next(new SetValueAction(this.state.id, newValue));
       }
     });
@@ -86,23 +102,28 @@ export class NgrxFormControlDirective<TValue extends FormControlValueTypes> impl
 
     this.subscriptions.push(
       this.state$
+        .map(s => ({ id: s.id, value: this.convertModelValue(s.value) }))
+        .distinctUntilChanged((l, r) => l.id === r.id && l.value === r.value)
         .map(s => s.value)
-        .map(this.convertModelValue)
+        .filter(v => v !== this.lastReportedViewValue)
         .subscribe(value => this.valueAccessor.writeValue(value))
     );
 
     if (this.valueAccessor.setDisabledState) {
       this.subscriptions.push(
         this.state$
+          .map(s => ({ id: s.id, isDisabled: s.isDisabled }))
+          .distinctUntilChanged((l, r) => l.id === r.id && l.isDisabled === r.isDisabled)
           .map(s => s.isDisabled)
           .subscribe(isDisabled => this.valueAccessor.setDisabledState!(isDisabled))
       );
     }
 
     this.subscriptions.push(
       this.state$
+        .map(s => ({ id: s.id, isFocused: s.isFocused }))
+        .distinctUntilChanged((l, r) => l.id === r.id && l.isFocused === r.isFocused)
         .map(s => s.isFocused)
-        .distinctUntilChanged()
         .subscribe(isFocused => {
           if (isFocused) {
             this.el.nativeElement.focus();