Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(form): make ngForm $pristine after nested control.$setPristine() (counter version)#13773

Open
wants to merge 2 commits into
base:master
Choose a base branch
from

Conversation

linoleum-js
Copy link

When calling $setPristine on the nested form or control,
form becomes $pristine of all the nested controls are pristine

Closes#13715

When calling $setPristine on the nested form or control, form becomes $pristine of all the nested controls are pristine Closesangular#13715
@@ -714,7 +714,8 @@ describe('form', function() {
expect(form.$error.maxlength[0].$name).toBe('childform');

inputController.$setPristine();
expect(form.$dirty).toBe(true);
// this assertion prevents to propagate prestine to the parent form
// expect(form.$dirty).toBe(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should change this to expect(form.$dirty).toBe(false); and remove the subsequent call to form.$setPristine().

@gkalpak
Copy link
Member

It generally LGTM (with a couple of nitpicks), BUT:

We need to properly handle added/removed controls.

@linoleum-js
Copy link
Author

Now it uses internal counter. It's a bit complicated, because we have to divide $setPristine propagtion into capturing and bubbling.
Also I'm not 100% sure that idempotence test cases are sufficient (I think this is crucial).

@gkalpak
Copy link
Member

I think it's best to have the two alternative approaches as two independent PRs (so we can review/update/decide upon separately).
Could you split revert the second commit and put it into a separate PR ?

Thx @linoleum-js for working on this, btw 👍

@linoleum-jslinoleum-js changed the title fix(form): make ngForm $pristine after nested control.$setPristine()fix(form): make ngForm $pristine after nested control.$setPristine() (counter version)Jan 19, 2016
@petebacondarwin
Copy link
Contributor

Has this been split?

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.
6 participants
@linoleum-js@gkalpak@petebacondarwin@lucasmaj@Narretz@googlebot
close