Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbstractControl.markAs* asymmetry is confusing, limiting #11774

Closed
xealot opened this issue Sep 20, 2016 · 19 comments
Closed

AbstractControl.markAs* asymmetry is confusing, limiting #11774

xealot opened this issue Sep 20, 2016 · 19 comments

Comments

@xealot
Copy link

xealot commented Sep 20, 2016

I'm submitting a ... (check one with "x")

[x] bug report
[x] feature request

Current behavior
There are markAs methods that can manipulate a form's internal validation state. These are highly useful. However, they propagate to children only sometimes.

The following methods also mark their children:
markAsUnTouched
markAsPristine

These methods do not mark their children:
markAsTouched
markAsDirty

I'm not even sure what this method is for...
markAsPending

Expected behavior
All methods to mark validation should interact with their groups either always or optionally.

Alternatively, some basic iteration machinery should be introduced so that we may instrument our own state changes easily.

What is the motivation / use case for changing the behavior?
For example; I want to show validation when loading a record from the DB but I would prefer not to show validation when creating a new record. It would be convenient to to mark everything as dirty or at least touched (as my validation would key off of those states) at the form level.

Conceptually, considering the semantic meaning of FormGroup, it seems if a parent control is marked as dirty that would indicate the children are dirty as well. The same reasoning which was originally used to implement reset() and the other markAs functions that handle their children.

  • Angular version: 2.0.0
  • Browser: all
  • Language: all
@xealot
Copy link
Author

xealot commented Sep 21, 2016

Alternatively to this, having a way to manually kick off validation would solve my particular use case.

@pkozlowski-opensource
Copy link
Member

Conceptually, considering the semantic meaning of FormGroup, it seems if a parent control is marked as dirty that would indicate the children are dirty as well.

I'm not sure this parallel holds... dirty means that a user interacted with a given control. So if we take a parent<->child relationship into consideration - if a form is dirty it means that a user interacted with a form. But it doesn't meant that she interacted with each and every control of a form.

The same reasoning which was originally used to implement reset() and the other markAs functions that handle their children.

This is different. With reset you are saying - get me the form to the initial state. This implies resetting children as well.

I don't think we can consider dirty and reset as a similar use-case and I don't think that those methods should behave the same. What would be interesting is to better understand your use-case as I'm not quite clear about:

For example; I want to show validation when loading a record from the DB but I would prefer not to show validation when creating a new record.

Remember that what gets validated is user input so I'm not sure I understand why entry for a new record should be validated differently from an updated one. I mean - if a user enters invalid info into a field it is still invalid - new or update. In any case you could always have a newRecord / update flag in your component and base validation errors display based on this.

All in all I don't think that the mentioned methods should be symetric but it is hard to me to propose an alternative since I don't grasp your use-case. Care to elaborate?

@xealot
Copy link
Author

xealot commented Sep 21, 2016

Sure thing, let's ignore my particular use case for one moment and simply discuss the concept of marking children as dirty.

FormGroup and FormArray are not controls that the user interacts with directly, they are collections of controls that the user interacts with. The statuses they surface are a reduction of their children, as the documentation says. With this in mind, if FormGroup provides a markAsDirty() function, does it make sense that the FormGroup can be dirty and yet all of its children can be clean?

I assert, that while this seems a minor distinction, does not make as much sense as the opposite.

Coming back around to my use case. I have a component that displays validation errors and typically it keys off of whether or not some interaction has happened on the screen. In some cases we want to show validation errors in the form from the start (load from db, collaboration) or trigger them all to show during some page event (submit without changes) simply to inform the user about what's wrong.

It's confusing to have a record loaded with invalid controls and require the user to touch each one to see the errors. There are a few ways to solve this:

  • Adding a flag for error display component that optionally shows/hide errors regardless of control state
  • Using CSS adjacency selectors and flags to hide/show validation (.ng-invalid ~ .errors) and relying on a parent selector to make that behavior optional
  • Iterating through controls and marking as dirty to force validation to show, even though the controls have no been touched

As an example from a separate domain, on the server-side, most HTML form libraries will not show validation errors unless a validate() method has been called. This indicates that you want to see what issues are present.

In summary, I suppose my real complaint is that showing or hiding validation is conflated somewhat with state of the control and it's not always sensible or easy to mutate that state.

@pkozlowski-opensource
Copy link
Member

In summary, I suppose my real complaint is that showing or hiding validation is conflated somewhat with state of the control and it's not always sensible or easy to mutate that state.

I would like to understand this better: how / when you show error messages us totally up to you, right? I mean, you can choose whatever combination of flags / control state to achieve desired result. You can totally show errors only on submit.

I'm starting to get the impression that we start to mix control state + errors attached to it with when those errors are displayed. Maybe a minimal plunk with your distilled use-case would help make this discussion more concrete?

@xealot
Copy link
Author

xealot commented Sep 21, 2016

I would like to understand this better: how / when you show error messages us totally up to you, right?

Anything can be implemented with enough effort. :)

I mean, you can choose whatever combination of flags / control state to achieve desired result. You can totally show errors only on submit.

If I wanted to show all of the form errors when a user clicked a "Show Errors" button, can I do that without implementing a mechanism totally outside of form control state to achieve that?

I think the decision to show validation is always derived from the state of the control. Whether it's valid certainly, and sometimes whether it's dirty/touched.

Vastly simple example:
http://plnkr.co/edit/po7NGV

@kara
Copy link
Contributor

kara commented Oct 13, 2016

It seems like there are two concerns at play here: how we are calculating dirtiness in markAsDirty (and etc) and controlling when/how validation runs.

Regarding the first, agree with @pkozlowski-opensource. It's worth noting dirty and pristine state don't have the same contract with their children. If a group is dirty, it means at least one of its children is dirty. In contrast, a group is pristine only when all its children are pristine. So when you reset a group, all children must be marked pristine to maintain the model. The same is true with "untouched". It's less clear with dirtiness because only one needs to be marked dirty to satisfy the model. Marking just one would be arbitrary and marking the group dirty doesn't necessarily mean that you want every single child dirty. Because of the ambiguity, this method is conservative and does not mark the children at all. Inclined to leave it as is because I don't think changing this behavior would have sufficient benefits to justify breaking people. That said, we might consider adding additional markAllDirtyand markAllTouched methods if this proves to be a pain point.

The second concern about manual validation is a valid one. I think we can do better. But it appears to be a duplicate of #6170, so let's continue the discussion there.

@MarcinMilewski
Copy link

Procedure to mark all form control tree dirty:

static markAllDirty(control: AbstractControl) {
      if(control.hasOwnProperty('controls')) {
          control.markAsDirty(true) // mark group
          let ctrl = <any>control;
          for (let inner in ctrl.controls) {
              this.markAllDirty(ctrl.controls[inner] as AbstractControl);
          }
      }
      else {
          (<FormControl>(control)).markAsDirty(true);
      }
  }

@dominique-mueller
Copy link

While @MarcinMilewski's solution works perfectly, a markAllAsDirty method in Angular Forms itself would be much nicer indeed.

@touqeershafi
Copy link

Yeah that will be good if it can be available for next release ?

@dan-lennox
Copy link

dan-lennox commented Jul 4, 2017

Recursive version for when there are a few levels of nested controls.

public static markAsDirtyDeep(control: AbstractControl): void {

  // Mark this control as dirty.
  control.markAsDirty();

  // Recursively mark any children as dirty.
  if (control.hasOwnProperty('controls')) {
    _.each((<any>control).controls, (childControl) => {
      this.markAsDirtyDeep(childControl);
    });
  }
}

@murilobd
Copy link

murilobd commented Oct 2, 2017

In case anyone reading this, that's my solution:

export function markFormGroupTouched(formGroup: FormGroup) {

	if (formGroup.controls) {
		const keys = Object.keys(formGroup.controls);
		for (let i = 0; i < keys.length; i++) {
			const control = formGroup.controls[keys[i]];

			if (control instanceof FormControl) {
				control.markAsTouched();
			} else if (control instanceof FormGroup) {
				this.markFormGroupTouched(control);
			}
		}
	}
}

@gbochmann
Copy link

Leaving this here for people who have similar issues AbstractControl.parent.touched also might help with use cases where it's unhandy to mark fields as dirty that were actually never touched.

@earbullet
Copy link

My component had an ngIf that prevented the ng-pristine class from being removed / applied when I called markAsDirty(). I ended up changing the ngIf to a class and then the css classes were properly applied to my input textbox.

This is probably just a lifecycle issue for me as the ngIf condition was probably not true when I called markAsDirty, but the css approach works in my situation.

Control Template where ng-pristine is not removed

<div *ngIf="!isEditMode">
{{value}}
</div>
<div *ngIf="isEditMode">
<input class="form-control" type="text" [(ngModel)]="value" [maxlength]="maxLength" [placeholder]="placeHolder" (blur)="onBlur()">
</div>

CSS approach so markAsDirty removes ng-pristine from my input element.

<div *ngIf="!isEditMode">
{{value}}
</div>
<div [class.hide]="!isEditMode">
<input class="form-control" type="text" [(ngModel)]="value" [maxlength]="maxLength" [placeholder]="placeHolder" (blur)="onBlur()">
</div>

@Ismaestro
Copy link

In case anyone reading this, that's my solution:

export function markFormGroupTouched(formGroup: FormGroup) {

	if (formGroup.controls) {
		const keys = Object.keys(formGroup.controls);
		for (let i = 0; i < keys.length; i++) {
			const control = formGroup.controls[keys[i]];

			if (control instanceof FormControl) {
				control.markAsTouched();
			} else if (control instanceof FormGroup) {
				this.markFormGroupTouched(control);
			}
		}
	}
}

Thanks much appreciate!

@ivancas84
Copy link

Recursive version for when there are a few levels of nested controls.

public static markAsDirtyDeep(control: AbstractControl): void {

  // Mark this control as dirty.
  control.markAsDirty();

  // Recursively mark any children as dirty.
  if (control.hasOwnProperty('controls')) {
    _.each((<any>control).controls, (childControl) => {
      this.markAsDirtyDeep(childControl);
    });
  }
}

In case anyone reading this, that's my solution:

export function markFormGroupTouched(formGroup: FormGroup) {

	if (formGroup.controls) {
		const keys = Object.keys(formGroup.controls);
		for (let i = 0; i < keys.length; i++) {
			const control = formGroup.controls[keys[i]];

			if (control instanceof FormControl) {
				control.markAsTouched();
			} else if (control instanceof FormGroup) {
				this.markFormGroupTouched(control);
			}
		}
	}
}

Thanks much appreciate!

Doesn't work with FormArray

@arthurspa
Copy link

arthurspa commented Jan 24, 2019

For those worried about performance, this solution doesn't use recursion, although it still iterates over all controls in all levels.

@ivancas84 This works also witth FormArray. (See it working here)

/**
  * Iterates over a FormGroup or FormArray and mark all controls as
  * touched, including its children.
  *
  * @param {(FormGroup | FormArray)} rootControl - Root form
  * group or form array
  * @param {boolean} [visitChildren=true] - Specify whether it should
  * iterate over nested controls
  */
  public markControlsAsTouched(rootControl: FormGroup | FormArray,
    visitChildren: boolean = true) {

    let stack: (FormGroup | FormArray)[] = [];

    // Stack the root FormGroup or FormArray
    if (rootControl &&
      (rootControl instanceof FormGroup || rootControl instanceof FormArray)) {
      stack.push(rootControl);
    }

    while (stack.length > 0) {
      let currentControl = stack.pop();
      (<any>Object).values(currentControl.controls).forEach((control) => {
        // If there are nested forms or formArrays, stack them to visit later
        if (visitChildren &&
            (control instanceof FormGroup || control instanceof FormArray)
           ) {
           stack.push(control);
        } else {
           control.markAsTouched();
        }
      });
    }
  }

Original Stack Overflow thread.

@Ismaestro
Copy link

Ismaestro commented Jan 24, 2019

@seba161189
Copy link

seba161189 commented Feb 18, 2019

static markAllDirty(control: AbstractControl) {
if(control.hasOwnProperty('controls')) {
control.markAsDirty(true) // mark group
let ctrl = control;
for (let inner in ctrl.controls) {
this.markAllDirty(ctrl.controls[inner] as AbstractControl);
}
}
else {
((control)).markAsDirty(true);
}
}

For me works changing markAsTouched() instead of markAsDirty

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests