Skip to content

ng-model-options debounce prevents default value from being displayed #127

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

Closed
wants to merge 12 commits into from

Conversation

jamesbrobb
Copy link
Contributor

In the ngModel directive restoreInputFieldmethod you need to call modelCtrl.$commitViewValue()after modelCtrl.$setViewValue()to ensure that the default value is displayed

@jrief
Copy link
Owner

jrief commented Nov 12, 2014

The Angular docs say:

..
$commitViewValue();
Commit a pending update to the $modelValue.
Updates may be pending by a debounced event or because the input is waiting for a some future event defined in ng-model-options. this method is rarely needed as NgModelController usually handles calling this in response to input events.
..

why do I then need this. Can you point me on a behavior where not calling this function causes some unwanted effect?

@jamesbrobb
Copy link
Contributor Author

This is the quoted functionality of your ngModel directive:

This directive overrides some of the internal behavior on forms if used together with AngularJS.
Otherwise, the content of bound forms is not displayed, because AngularJS does not know about
the concept of bound forms and thus hides values preset by Django while rendering HTML.

This functionality no longer works if you have ng-model-options={ debounce:500 } set on an input. Calling $setViewValue does not display the default value in the field, it's just blank. If you call $commitViewValue after, it forces the input view to update as the value is set on the model.

Although is there a particular reason why you're not using modelCtrl.$modelValue to set the default value?

@jamesbrobb
Copy link
Contributor Author

I've tested this against 0.7.10 and it works for angular 1.3+ when using the new ng-model-options debounce on an input. If you test the second commit (which uses 1.2.3), then the name 'james' should appear as the defaultValue in the 'first name' input, but it doesn't.

If you check your console you'll see that 'james' is output and if you inspect the ngModel instance, although the $viewValue is initially set to 'james', it then appears to be set back to undefined.

Calling $commitViewValue fixes this issue as this method was introduced on ngModelController in 1.3 to do exactly that. Force the view value to the model when debounce is being used.

Using $modelValue to set the default instead (as i'd suggested earlier) doesn't work in versions before 1.3.

@jamesbrobb
Copy link
Contributor Author

@jrief You can ignore everything here except for the final commit.

This is my preferred fix for this issue, as it simply sets the default value directly onto the scope model, which ngModel then uses to populate the fields. Going forward, if any new features are added to the angular 1.x branch that could affect the way in which values are applied to ngModel (as happened in 1.3), no modification will be necessary for it to continue working. This fix is version agnostic.

For an alternate fix that's closer to your original, but which tests for 1.3 features and uses the ngModelController to set the default value, please see issue #152.

@adrienbrunet
Copy link
Collaborator

Hi @jamesbrobb I'm trying to go through old issues and PR to clean this repo a bit. Do you know if this PR is still needed? Can you point me a failing behavior right that this PR fixes? You said there is an alternative solution for that problem? I'm eager to merge #152 as well 'cause I'm facing this issue right now...

@jamesbrobb
Copy link
Contributor Author

Hi @adrienbrunet , i'm afraid i'm not as it's been a while since i submitted this PR (and the rest) and i've not used django-angular since, so i have no idea what's been changed. It's worth looking at #160, as i'd found quite a few issues with the way the JS has been written and give a fairly extensive explanation of the changes i'd made.

@adrienbrunet
Copy link
Collaborator

@jrief I'm closing this one. If we need to merge something, it should be from PR #152
@jamesbrobb contributions are really good, the problem is explained and the solution is quite clear as well. Code is well structured. It kust takes A LOT of time to read it through, understand the problem, the solution and ensure the way it's done is one of the best we can come up with. Sorry for the delay..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants