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

Programmatic updates of the model in ngModelOptions #6994

Closed
shahata opened this issue Apr 4, 2014 · 8 comments
Closed

Programmatic updates of the model in ngModelOptions #6994

shahata opened this issue Apr 4, 2014 · 8 comments
Assignees

Comments

@shahata
Copy link
Contributor

shahata commented Apr 4, 2014

Sadly there are still issues with programmatic updates of the model in the new ngModelOptions:
http://plnkr.co/edit/C8IzuF?p=preview

  • In the first input, we have a debounced update. Try to start typing in the input and then click the clear button. The debounced is canceled alright, but the value in the input is not reset. :( This obviously happens because the model update did not reach the view since the model was not actually changed. Maybe this can be fixed by forcing a view update when debounce is canceled?
  • In the second input, we have an {updateOn: 'focus'}, so if we enter a value and then click the clear, the value also is not reset. This is essentially the same issue as the previous one, but here I don't have a good suggestion because we don't have anything like the $cancelDebounce() to indicate us to force the view change.

What do you think?

I'll be glad to take this PR, but first I think we should discuss a strategy of how to solve it.

@petebacondarwin petebacondarwin self-assigned this Apr 5, 2014
@petebacondarwin
Copy link
Member

The first one is solved by fixing up what $cancelDebounce() does.
See http://plnkr.co/edit/ReONqq?p=preview

We simply call $render() inside $cancelDebounce(). But then, because we use $cancelDebounce() from $setViewValue(), we have to tweak it slightly....

  this.$cancelDebounce = function(noRender) {
    if ( pendingDebounce ) {
      $timeout.cancel(pendingDebounce);
      pendingDebounce = null;
      if ( !noRender ) {
        this.$render();
      }
    }
  };
  this.$setViewValue = function(value, trigger) {
    var that = this;
    var debounceDelay = this.$options && (isObject(this.$options.debounce)
        ? (this.$options.debounce[trigger] || this.$options.debounce['default'] || 0)
        : this.$options.debounce) || 0;

    that.$cancelDebounce(true);
    if ( debounceDelay ) {
      pendingDebounce = $timeout(function() {
        pendingDebounce = null;
        that.$$realSetViewValue(value);
      }, debounceDelay);
    } else {
      that.$$realSetViewValue(value);
    }
  };

@shahata - if you agree, do you want to knock up a PR with this fix. It would need to have a unit test that fails without the fix. Something like:

    it("should reset the input box when cancelDebounce is called", function() {
      compileInput(
          '<form name="test">'+
            '<input type="text" ng-model="name" name="alias" '+
              'ng-model-options="{ debounce: 10000 }" />'+
            '</form>');
      changeInputValueTo('a');
      scope.test.alias.$cancelDebounce();
      expect(inputElm.val()).toBe('');
    });

@petebacondarwin
Copy link
Member

It may be that we should refactor the $cancelDebounce function but the sentiment is still the same.

@petebacondarwin
Copy link
Member

Regarding the second problem, using updateOn: 'focus' seems like such a weird scenario that I can't think of any real use case for it. @shahata - do you have one? If not, then I think we should just document that certain weird choices of triggers may cause the application to behave weirdly.

@shahata
Copy link
Contributor Author

shahata commented Apr 5, 2014

Yes, I agree those issues can be resolved by forcing an update of the view, that's exactly what I was going for. As for the second issue, here's a more realistic example: http://plnkr.co/edit/kjMG4K?p=preview - the idea here is that the second input uses updateOn: 'blur', but there is also a ngKeydown waiting to reset the input if escape is pressed. So, try to type something, then press escape, but the input does not get cleared.

@petebacondarwin
Copy link
Member

What if we had a method called $setModelValue() on NgModelController? This could force the $formatters to run and force an update to the view?

@petebacondarwin
Copy link
Member

@petebacondarwin
Copy link
Member

Actually a more robust option (since this $setModelValue() doesn't actually modify the "real" model) would be to have a $resetModelValue(). See http://plnkr.co/edit/3w70Bd?p=preview

@lrlopez
Copy link
Contributor

lrlopez commented Apr 5, 2014

I agree

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants