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

Closes angular#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

gkalpak commented Jan 15, 2016

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

We need to properly handle added/removed controls.

@Narretz Narretz modified the milestones: 1.6.x, Backlog Jan 15, 2016
@Narretz Narretz force-pushed the master branch 2 times, most recently from 70049ae to b77e14b Compare January 16, 2016 21:59
@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

gkalpak commented Jan 18, 2016

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-js linoleum-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?

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ngForm stays $dirty after control.$setPristine()
6 participants