feat: support showing errors when a control is marked as touched #114
Conversation
|
|
|
@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 |
| @@ -43,7 +45,7 @@ const errorTailorClass = 'error-tailor-has-error'; | |||
| '[formControlName]:not([controlErrorsIgnore]), [formControl]:not([controlErrorsIgnore]), [formGroup]:not([controlErrorsIgnore]), [formGroupName]:not([controlErrorsIgnore]), [formArrayName]:not([controlErrorsIgnore]), [ngModel]:not([controlErrorsIgnore])', | |||
| exportAs: 'errorTailor', | |||
| }) | |||
| export class ControlErrorsDirective implements OnInit, OnDestroy { | |||
| export class ControlErrorsDirective implements OnInit, OnDestroy, DoCheck { | |||
| @Input('controlErrors') customErrors: ErrorsMap = {}; | |||
There was a problem hiding this comment.
What do u think about converting the inputs to signal inputs?
There was a problem hiding this comment.
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.
| }); | ||
| } | ||
|
|
||
| ngDoCheck(): void { |
There was a problem hiding this comment.
Let's use afterRender hook and only invoke it when this.mergedConfig.controlErrorsOn.touched
There was a problem hiding this comment.
What about monkey patch ngControl methods instead?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sure, I can look into the afterRender solution. Do you think it would be more performant?
There was a problem hiding this comment.
Yes, because it'll only do it if this.mergedConfig.controlErrorsOn.touched is true
There was a problem hiding this comment.
Sure, going to call detectChanges in the changesOnTouched$ which has distinctUntilChanged, so will only run when it actually changes.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I'm very busy at work currently. Try to comment some code and see where is the issue (You can maybe begin with setError)
There was a problem hiding this comment.
@NetanelBasal Does it even make sense to use afterRender? Something that will need for sure a render to be shown, probably doesn't make sense to be in afterRender.
Furthermore, the documentation says that
The execution of render callbacks are not tied to any specific component instance, but instead an application-wide hook.
so the callback would be run after every render application-wide.
There was a problem hiding this comment.
so the callback would be run after every render application-wide.
Correct. That's one of the main issues with this approach. I think we can go with MutationObserver or monkey patching
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #64
Currently errors are not shown if a control is marked as
touchedprogrammatically viacontrol.markAsTouched()orformGroup.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 beingtouched.Does this PR introduce a breaking change?