markAllAsTouched() not propegated to subforms

[ntziolis]

Repro:

• One root form A with sub form B
• Submit button on root form can always be clicked
  • Submit calls markAllAsTouched()

Expected:

• all fields on the entire form tree incl sub-forms should be touched

Actual:

• only controls directly on the root form are being marked as touched

Note:

• I assume it does not work as the sub-form is only connected to the root form via value accessor
• and looking at the value accessor interface, touched is not an event that gets propagated from parent to child (only the other way around registerOnTouched)

The intent of this issue is to try and come up with a best practice to handle this and if possible to extend the library to assist in these cases.

[ntziolis]

Was thinking about this issue, but also more generally that there are various others arising from not being able to have access to the parent formGroup of the sub-form.

They all stem from the limited interface of the ControlValueAccessor, which wasn't intended to handle sub forms. Hence I was thinking something along the lines of:

• Create an interface SubFormValueAccessor that extends ControlValueAccessor
  • this interface would at the very least make the parent ´formGroup` available to the sub-form
• Create a directive subFormName that works similar to formControlName / formArrayName but that requires SubFormValueAccessor to be implemented instead of ControlValueAccessor on the component

Goal:

• not to break existing usage of ngx-sub-form via [formControlName]
  • Since SubFormValueAccessor would be a superset of ControlValueAccessor using sub-forms with [formControlName] would still work, while not providing all fucntionality ngx-sub-form has to offer (markAllAsTouched etc)
• bring the behaviour of sub-forms more in line with how they would behave when their FromGroups were all defined on the root form (standard reactive forms)

Note:
Im still looking into if there is a way to hook into things markAllAsTouched etc. on the now available parent formGroup. Will report back here when I had time to look at this.

[ntziolis]

Results of further investigation for how to hook into calls on the parent, sample markAllAsTouched():

• The method loops to the child controls
• This leaves 2 approaches
  1. wrap and reassign methods like markAllAsTouched()
     • since we have access to the parent inside the directive that's easy todo while always trying to avoid this way of hooks
  2. or actually create the controls on the passed in parent form (based on ControlInterface)
     • however this means that the there might be 2 formGroups on a sub-form (when ControlInterface != FormInterface a second form group (ControlValueAccessor)
     • in addition if there are 2 these two would need to by "synced" as well so
       • wrapping + reassigning the various methods again,
       • for all relevant methods that could be called that could be called on a formGroup
     • the main appeal would be that this would actually also allow full access to the entire sub form from the root form hence 100% the same experience as when working with normal form groups on a root form

I'm aware that especially the second approach would require significant changes in how the lib works today. Right now this is meant merely to spark a discussion of what you guys think of this.

[ntziolis]

So I took a swing at trying to see how exzessive it would be todo this:

• first I tried to override the formControlName, but replacing an existing directive does not seem to be possible with angular
  • being able todo this would be my preferred approach as it would mean one can keep using formControlName / formControl instead of custom directive names
• so I went down the route of implementing the custom subFormName directive which is derived from formControlName

I was slightly worried that the injection of the derived directive would get messed up, but no need it worked like a charm!

• replaced all occurrences of formControlNamewith subFormName and boom,
  • all touched / untouched events were propagated successfully.
  • This is easily expanded to other methods on the control where propagation is wanted
• Note:
  emitInitialValueOnInit = false on the sub-forms is required as otherwise the sub forms will be touched by default

Below is the directive, there is an equivalent subFormdirective which takes a control instead of a name:

const controlNameBinding: any = {
  provide: NgControl,
  useExisting: forwardRef(() => SubFormNameDirective)
};

@Directive({ selector: '[subFormName]', providers: [controlNameBinding] })
export class SubFormNameDirective extends FormControlName implements OnInit {
  @Input('subFormName') name!: string;

  // this is copied from angular FormControlName directive
  // https://github.com/angular/angular/blob/master/packages/forms/src/directives/reactive_directives/form_control_name.ts
  constructor(
    @Optional() @Host() @SkipSelf() parent: ControlContainer,
    @Optional() @Self() @Inject(NG_VALIDATORS) validators: Array<Validator | ValidatorFn>,
    @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators: Array<AsyncValidator | AsyncValidatorFn>,
    @Optional() @Self() @Inject(NG_VALUE_ACCESSOR) valueAccessors: ControlValueAccessor[]
  ) {
    super(parent, validators, asyncValidators, valueAccessors, null);
  }

  ngOnInit(): void {
    const self = (this as unknown) as FormControlName;

    if (self.valueAccessor instanceof NgxSubFormComponent) {
      const subForm = self.valueAccessor;

      const control = self.control;

      const markAsTouched = control.markAsTouched.bind(
        control
      ) as AbstractControl['markAsTouched'];
      control.markAsTouched = (
        opts: Parameters<AbstractControl['markAsTouched']>[0]
      ) => {
        markAsTouched(opts);
        subForm.formGroup.markAllAsTouched();
      };

      const markAsUntouched = control.markAsUntouched.bind(
        control
      ) as AbstractControl['markAsUntouched'];
      control.markAsUntouched = (
        opts: Parameters<AbstractControl['markAsUntouched']>[0]
      ) => {
        markAsUntouched(opts);
        subForm.formGroup.markAsUntouched();
      };
    }
  }
}

What do you guys think?

[RicardoArdito]

Hi @ntziolis ,

First of all congrats for the great job you guys are doing with ngx-sub-form. I started using it a few days ago, with some very complex forms where we were struggling to handle validation. Using ngx-sub-form was pretty straight forward and did solved the enormous boilerplate we had due to our deep component tree on those forms.

But we soon noticed this problem with markAllAsTouched(): We use Angular Material, and although all fields were being correctly validated, it was not being reflected in the interface (since they were not being marked as touched on form submit). That was when we found this thread.

We implemented your SubFormNameDirective, and noticed a few points that I would like to share with you:

• We use ChangeDetectionStrategy.OnPush in all our components, and it seems SubFormNameDirective is not working well with that. We were able to workaround this adding a ChangeDetectorRef.markForCheck() right after subForm.formGroup.markAllAsTouched(). It worked, but we are not sure this would be the best solution;

• A second issue we have found is that when any field is changed, all other fields are marked as touched, and then get immediately marked as invalid. It seems this is due to the onTouched() call at line 385 of NgxSubFormComponent.

Please let me know if there is any thing we can do to help with those issues, or if we are missing something.

Thanks, guys!

[ntziolis]

@RicardoArdito First off, me and my team really are just heavy users of ngx-sub-form as well, credits for the lib should go to the creators ;)

In regards to your comments:

• mark for check is just fine and exactly meant for cases like these so I see nothing wrong with it
• OnTouched call is indeed the culprit here and I see no way of fixing this without making key changes to the underlying lib

In general the markAllAsTouched is only the most prominent of various problems that arise with using FormControl's to hide complex forms underneath it as it severs the connection between main form and controls on the sub form.

Initially we thought we can get around this by using custom directives but that is not the case as we quickly ran into issues as you encountered as well. We need both a better way to intercept as well as propagate events / calls to the form controls on the sub form.

Here are some requirements we have for a solutions:

• all FormGroup events / calls should be properly propagated to the form controls on the sub forms
• ability to enable / disable all controls from outside the sub-form
• we also wanne make sure to keep the separation between form / control type

With that in mind we are thinking this could be achieved by:

• creating a SubFormGroup class derived from FormGroup
  • accessing controls. would simply map to the controls of the sub form
  • while valuechanges and others would output the control value
• making key changes to the base ngx-sub-form component to allow for things like resetting the form (which should clear mark as touched) as well as enable another currently broken use case when resetting forms

Right now I believe such solution would not require custom directives, but merely the use of SubFormGroup instead for FormControl inside the getControls call.

What do you think?

[maxime1992]

Hey guys, small update. We've had to focus on our app in the last few weeks but we will try to get back on ngx-sub-form a bit next week.

Thanks for keeping up great discussions in the meantime, we'll have a look asap :) !

PS: Thanks for the kind words Ricardo, feels free to share more about your experience here: #112 !

[MonsieurMan]

We have the same use case as @RicardoArdito.

I prefer my submit buttons to never be disabled and show errors only when the user tries to submit the form instead. But I can't as there is no way to access child controls from root form for now.

@ntziolis, I follow you on the requirements. About the solution I don't know much about the code base but it seems right at a glance.

[MiroslavKral]

Hey, we have the same problem. Our solution is simple marker directive for sub forms components.

@Directive({
  selector: '[rbSubForm]',
})
export class RbSubFormDirective implements OnInit, OnDestroy {
  private destroy = new Subject<void>();

  constructor(private ngControl: NgControl, private rbForm: RbFormDirective) {}

  ngOnInit(): void {
    this.rbForm
      .onSubmit()
      .pipe(takeUntil(this.destroy))
      .subscribe(() => {
        const valueAccessor = this.ngControl
          .valueAccessor as NgxSubFormComponent<unknown>;
        valueAccessor.formGroup.markAllAsTouched();
      });
  }

  ngOnDestroy(): void {
    this.destroy.next();
    this.destroy.complete();
  }
}

Then we use it as follows:

<rb-address-form
  rbSubForm
  id="permanent-address"
  [formControlName]="formControlNames.permanentAddress"
></rb-address-form>

Yes it is manual solution, but it serves us well.

[ntziolis]

@MiroslavKral This (additional directive) was our initial approach as well and then we simply moved the logic into the subFormGroup to avoid having 2 directives. This approach works great for the markAllAsTouched use case.

However it does not work when wanting todo other things like reset the form (both data as well as touched state). While directive approach does make sure all formControls on all subForms are initially marked untouched, this gets reverted by the way the OnTouched is being called in the root NgxSubFromComponent. So one ends up with a partial reset form (data is reset, formControls are marked as touched).

[ntziolis]

The more we we make use of the sub-form library it becomes clear that we need access to the actual form controls on a sub form to enable full control from the outside if needed (while we actively trying to avoid it where possible)

I’m currently playing with creating a class derived from form group based on the requirements outlined above. Since this approach would allow to use formGroup of the sub form directly we wouldn’t need the sub-form-component class to handle things like on touched anymore. Trying to figure out if a change in the lib is required to make this work or if it’s possible to simply ignore what the lib does in regards to touched.

Will report back here soon.

[maxime1992]

Can we all please give a 👍 to the following issue: angular/angular#27315 ?

If that was solved upstream we could implement this feature really easily ❤️

[elvispdosreis]

please share RbFormDirective

Hey, we have the same problem. Our solution is simple marker directive for sub forms components.

@Directive({
  selector: '[rbSubForm]',
})
export class RbSubFormDirective implements OnInit, OnDestroy {
  private destroy = new Subject<void>();

  constructor(private ngControl: NgControl, private rbForm: RbFormDirective) {}

  ngOnInit(): void {
    this.rbForm
      .onSubmit()
      .pipe(takeUntil(this.destroy))
      .subscribe(() => {
        const valueAccessor = this.ngControl
          .valueAccessor as NgxSubFormComponent<unknown>;
        valueAccessor.formGroup.markAllAsTouched();
      });
  }

  ngOnDestroy(): void {
    this.destroy.next();
    this.destroy.complete();
  }
}

Then we use it as follows:

<rb-address-form
  rbSubForm
  id="permanent-address"
  [formControlName]="formControlNames.permanentAddress"
></rb-address-form>

Yes it is manual solution, but it serves us well.

[agallardol]

Hi everyone, today I was dealing with this problem and trying to use your ideas as inspiration I created the following gist that let me workaround the markAsTouched/markAllAsTouched problem.

NgxSubForm markAllAsTouched fix

All of your feedback is very preciated and I hope this could help someone of you.

[elvispdosreis]

could you please provide a usage example

<form [formGroup]="form" (ngSubmit)="onSubmit()">

because it already has a directive with the same name [formGroup]

[agallardol]

Hi @elvispdosreis I hope you will be fine. I created this example to demonstrate how works the proposed workaround. I think It could be easily extended and generalized.

NgxSubForm markAllAsTouched fix example

Tell me if I can help with another example or case 🤟

[agallardol]

Hi, I added minor changes to the example to add ngSubmit usage.

[elvispdosreis]

Thanks

is that correct?
line 47

if (isSubForm) {
    subForm = value;
    return;
}

[agallardol]

Hi, It's ok, but I was reading again my code and I applied small refactorizations to improve the code readability. Basically I'm looking for a NgxSubForm instance inside any ControlValueAccessor (asuming this is in the root) of the FormGroup/FormControl.

[maxime1992]

Hello 👋

I'm glad we've got some work around explained above above but as explained here, this would really need to be fixed upstream so I've marked this issue as needs fix upstream and won't solve here I'm afraid.

I'll close this issue as there's nothing we can do more than offer work arounds, which has been done :)

Thanks everyone for the brainstorming around the work arounds!

[elvispdosreis]

I need help, I'm using your fix but it only marks the first form

https://stackblitz.com/edit/ngx-sub-form-basics-patchvalue?file=src%2Fapp%2Fngx-sub-form-mark-all-as-touched-fix.ts