Ninja-Squad / ngx-valdemort

Simpler, cleaner Angular validation error messages
https://ngx-valdemort.ninja-squad.com
MIT License
199 stars 6 forks source link

Validation errors do not appear on submit with nested form controls (and ChangeDetection.OnPush) #317

Open david-bulte opened 2 years ago

david-bulte commented 2 years ago

When some form controls are on a nested component, validation erros do not always appear on submit.

Check out this stackblitz: https://stackblitz.com/edit/angular-ivy-kprprr

Case 1) No nested component When clicking submit, error messages appear underneath the controls.

Case 2) With nested component When clicking submit, error messages do not appear underneath the controls

Case 3) Nested component and explicitedly call form.markAllAsTouched() When clicking submit, and calling form.markAllAsTouched in the submit method, error messages do appear.

Case 4) Nested component that has ChangeDetection.OnPush However, even when calling form.markAllAsTouched(), error messages do not appear when clicking submit.

Is this a bug? Or is there a workaround to make this work?

cexbrayat commented 2 years ago

Hi @david-bulte

Thank you for opening an issue and providing a repro.

Valdemort shows an error if the field is touched or the surrounding form group directive has been submitted (by default).

2) I think this is expected as Valdemort checks if the surrounding FormGroupDirective has been submitted. When you nest [formGroup] and submit the top one, the nested one is not submitted, and the errors do not show.

3) That's why your workaround does the job: when you mark the field as touched then the error shows up

4) If you're using OnPush, then your component is checked by Angular only if the input changes. In your case you only have the parent input, and it does not change, so the component is not updated. A potential workaround would be to add another input to let the nested component know that the form was submitted. Something like:

<form [formGroup]="form" #formOnPush="ngForm">
  <app-nested-onpush [parent]="form" [submitted]="formOnPush.submitted"></app-nested-onpush>

This additional input will trigger the change detection of the nested component, and the errors will show. This is not super pretty but should do the job. Let me know if that helps.

To sum up, I don't think there is an issue here, as this looks like standard Angular behavior. But I'll keep this issue open and I'll discuss with the rest of the team to see if we can improve something in the library for these cases, or if we have a better workaround idea.

david-bulte commented 2 years ago

Thanks for your workaround - it helps.

I have another idea, at least one that works for us.

Our forms are decorated with a special directive "form-helper" (that takes care of all kinds of form-related stuff, e.g. dirty checking etc...). The directive exposes a submitted$ observable.

Now for the fix, I have written a directive for each val-errors component, that subscribes to that submitted$ observable. When the form submits, the directive marks the val-errors component as dirty.

Not sure whether it's the way to go though, as we only rarely use ChangeDetection.OnPush, so maybe it's a bit overkill?

Here's the code from that directive:

@Directive({
  selector: 'val-errors',
})
export class MarkForCheckOnSubmitDirective {
  constructor(private cdr: ChangeDetectorRef, @Optional() formHelperDirective?: FormHelperDirective) {
    formHelperDirective?.submitted$.subscribe(() => {
      cdr.markForCheck();
    });
  }
}

You could probably do something similar with a 'form' directive (selector: 'form') that exposes the ngSubmit, and inject that directive in ValidationErrorsComponent.