feat: support showing errors when a control is marked as touched

[frabonomi]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #64

Currently errors are not shown if a control is marked as touched programmatically via control.markAsTouched() or formGroup.markAllAsTouched().

What is the new behavior?

When setting controlErrorsOn: { touched: true } in the config or when setting the [controlErrorsOnTouched]="true" input on a form control, errors will be show when the control is marked as touched (control.touched === true) and will be hidden if the control becomes pristine again after being touched.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[frabonomi]

@NetanelBasal Looking forward to know your thoughts.

Took it a step further and since configs to change the default behavior are a few now, I thought it would be good to explain a bit better how it works, so I added a Customize the behavior section in the README.md, let me know what you think.

[NetanelBasal]

What do u think about converting the inputs to signal inputs?

[frabonomi]

Would be nice indeed! How about doing it in a separate PR, though? I believe we would also need to update the Angular version. input seems to be exported from v17.2.0 on, unless I'm missing something.

[NetanelBasal]

Sure

[NetanelBasal]

Let's use afterRender hook and only invoke it when this.mergedConfig.controlErrorsOn.touched

[NetanelBasal]

What about monkey patch ngControl methods instead?

[frabonomi]

I considered monkey patching since it would be the most performant solution (called the least amount of times), but I think it's not the safest solution.

First of all we'd need to monkey patch all methods that might change the touched property, so control.markAsTouched(), control.markAsUntouched(), formGroup.markAllAsTouched(). We also could miss methods introduced in new versions of Angular that change that property, for example formGroup.markAllAsTouched() was introduced in v8.

[frabonomi]

Sure, I can look into the afterRender solution. Do you think it would be more performant?

[NetanelBasal]

Yes, because it'll only do it if this.mergedConfig.controlErrorsOn.touched is true

[frabonomi]

Back with some results. I compared these three implementations:

• ngDoCheck (as in the current PR):

  ngDoCheck(): void {
    if (this.mergedConfig.controlErrorsOn.touched) {
      this.touchedChanges$.next(this.control.touched);
    }
  }

• afterRender:

  if (this.mergedConfig.controlErrorsOn.touched) {
    afterRender(
      () => {
        this.touchedChanges$.next(this.control.touched);
      },
      { injector: this.injector },
    );
  }

  and then call this.changeDetectorRef.detectChanges()
• MutationObserver:

  if (this.mergedConfig.controlErrorsOn.touched) {
    const observer = new MutationObserver((mutations: MutationRecord[]) => {
      mutations.forEach((mutation) => {
        const isTouched = (mutation.target as HTMLElement).classList.contains('ng-touched');
        this.touchedChanges$.next(isTouched);
      });
    });
    observer.observe(this.host, {
      attributeFilter: ['class'],
    });
  }

For each implementation I placed a console.count('rerender') in ngDoCheck() to count how many rerenders were performed (only for the name form control). I tested by loading the page, then after one second I called markAsTouched() and then after another second I called markAsUntouched(). If I didn't do any mistake, these are the results:

• ngDoCheck: 5 rerenders

  GIF

  

• afterRender: 7 rerenders

  GIF

  

• MutationObserver: 8 rerenders

  GIF

  

Honestly I don't understand why the MutationObserver implementation would cause 3 more rerenders. If you want I can push the code to a separate branch on my fork if you want to take a look, let me know.

If these results are correct then maybe it's better to go with the ngDoCheck implementation? As I see it right now the comparison is between:

• ngDoCheck solution: accessing the this.mergedConfig.controlErrorsOn.touched on each change detection (doesn't do anything else if false)
• other solutions: causing more rerenders, which will definitely run more code since the whole Angular's change detection logic will be set into motion.

What do you think? Had some fun digging into this matter 😄 but if you feel strongly that we should go for the other solutions I can implement the one you prefer.

[NetanelBasal]

Let's go with afterRender but only call detect changes if the prev !== current

[frabonomi]

Sure, going to call detectChanges in the changesOnTouched$ which has distinctUntilChanged, so will only run when it actually changes.

[frabonomi]

Mmm, I'm a bit puzzled. I'm pretty sure I successfully tested the afterRender solution earlier, but now when trying to implement it again I get this error if I call change detection:

Error: NG0102: A new render operation began before the previous operation ended. Did you trigger change detection from afterRender or afterNextRender?

which didn't show up before… Seems a timing issue, because if I call detectChanges() in a setTimeout with just 10ms of delay the error doesn't throw and it works fine.

I created a separate branch with the change, care to have a look?

[NetanelBasal]

I'm very busy at work currently. Try to comment some code and see where is the issue (You can maybe begin with setError)