These are chat archives for angular/angular.js/ng-model-options

5th
Apr 2014
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:01
Actually this is better, it is under the angular name
So we were discussing #7011
Luis Ramón López
@lrlopez
Apr 05 2014 21:03
Wait, I'll amend the commit so we can discuss actual code
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:05
We need a programmatic way to change the value of an input box in the case where the trigger that will cause an update has not yet been triggered (like a blur).
Hold on! I just had a thought... What if we had a directive that attached a custom DOM event, we could add that to the updateOn list and it would solve our problem, no?
In our particular use case we would have a directive that listened for a keydown event and raised its own "clear" event if the key pressed was the escape key.
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:12

Something like:

mod.directive('clearInput', function() {
  return {
    link: function(scope, element, attr) {
      element.on(attr.clearInput, function(ev) {
        if (e.keyCode == 27) {
          element.val('');
          element.triggerHandler('clear', ev);
        }
      });
    }
  };
});

then on our input we could have:

<input ng-model="x" ng-model-options="{updateOn: 'blur clear'}">
Luis Ramón López
@lrlopez
Apr 05 2014 21:15
@shahata has commented on #7011
Hmmm... This should work, but it may be too complicated
Igor Minar
@IgorMinar
Apr 05 2014 21:18
did you see the message from shahata
$render() should be all you need. no new api needed
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:19
Yes I was trying to add him to this but he is not yet on Gitter. We started the original discussion on his issue but he was not responding. I have posted a comment
Igor Minar
@IgorMinar
Apr 05 2014 21:19
ok
Luis Ramón López
@lrlopez
Apr 05 2014 21:21
welcome!
Shahar Talmi
@shahata
Apr 05 2014 21:23
hi all
Igor Minar
@IgorMinar
Apr 05 2014 21:23
hi there!
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:23
Sorry about the mix up
Shahar Talmi
@shahata
Apr 05 2014 21:24
no problem
Luis Ramón López
@lrlopez
Apr 05 2014 21:24
Yes, sorry about that
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:24
@shahata - I really like your ability to find these tricky cases
So @lrlopez and I were chatting on G+ but we found it hard to discuss code. That was why we made the PR.
Now back to the problem. I think you are right we don't need to run the formatters for the first case.
In the second case, I guess we can just force a $render...
Shahar Talmi
@shahata
Apr 05 2014 21:27
we could, the problem is that it seems a bit shady to me. sadly I can't think of a beter option
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:28
It is actually quite clean and doesn't rely on knowing anything about the particular input directive
  $scope.cancel = function (e) {
    if (e.keyCode == 27) {
      $scope.form2.search2.$render();
    }
  };
Luis Ramón López
@lrlopez
Apr 05 2014 21:31
I agree
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:32
So the first one, just resets the input box to what the model says it should be.
  $scope.cancel = function (e) {
    if (e.keyCode == 27) {
      $scope.search2 = '';
      $scope.form2.search2.$render();
    }
  };
Also resets the model to empty string
Both situations seem to work well
It is just a shame that $render seems a bit cryptic and would almost always need a comment to explain what is happening
Luis Ramón López
@lrlopez
Apr 05 2014 21:35
We may add some notes about that in the ngModelOptions docs
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:35
Yes, all these cases should be well documented with examples.
Luis Ramón López
@lrlopez
Apr 05 2014 21:35
Something along the lines of: If you need to programmatically update the model, you should use $render()to force the update
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:36
... if the input box is waiting for an event to trigger an update.
Luis Ramón López
@lrlopez
Apr 05 2014 21:37
...or use $cancelDebounce() if your are used delayed updates
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:38
It seems a shame that these two methods are not somehow combined. What if I am not sure if I am updating before an event happens or while a debounce is pending? I would have to write:
$scope.form2.search2.$cancelDebounce();
$scope.form2.search2.$render();
Igor Minar
@IgorMinar
Apr 05 2014 21:39
why do you need to call $render explicitly? $cancelDebounce should call it for you
Luis Ramón López
@lrlopez
Apr 05 2014 21:39
that is
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:39
What if $cancelDebounce() was renamed $cancelUpdate() and it always called $render()
@IgorMinar - $cancelDebounce() only calls render if there is a debounce pending
Shahar Talmi
@shahata
Apr 05 2014 21:40
maybe we should rename $cancelDebounce and tell ppl call it in any case of nf-model-options
yeah, what pete said
Luis Ramón López
@lrlopez
Apr 05 2014 21:41
Then, we should add a parameter to $cancelUpdate() so we could skip $render
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:41
Actually the "current" $cancelDebounce doesn't call $render() at all but it should in order to fix the first issue that @shahata raised.
Igor Minar
@IgorMinar
Apr 05 2014 21:41
right
and why can't we just keep it called $cancelDebounce, but have it call $render?
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:42
I think if you are going to call $cancelUpdate() then you probably want $render() to occur every time.
Luis Ramón López
@lrlopez
Apr 05 2014 21:42
not always... think about $setViewValue
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:43
@IgorMinar - because the second issue that @shahata raised is not caused by debounce
Igor Minar
@IgorMinar
Apr 05 2014 21:43
what's the scenario when you want to call $render when no debounce is pending?
Shahar Talmi
@shahata
Apr 05 2014 21:43
the example I gave is an excel like cell
where you enter a value and pressing escape cancels it
and the value is updated only on blur
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:44
Igor Minar
@IgorMinar
Apr 05 2014 21:44
on blur? the issue says on focus
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:44
@shahata - finessed the use case to one less esoteric
Shahar Talmi
@shahata
Apr 05 2014 21:44
yeah, pete said it was unrealistic example, so the pnblur is the realiastic example that followed
Igor Minar
@IgorMinar
Apr 05 2014 21:45
ok
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:45
@lrlopez - what do you mean about $setViewValue()?
Oh, right, we would want to cancel debounces in $setViewValue
Shahar Talmi
@shahata
Apr 05 2014 21:47
so it seems strange to tell ppl call $render is some case and call $cancelDebounce in another. so maybe $cancelUpdate can be used for both cases
Luis Ramón López
@lrlopez
Apr 05 2014 21:47
@petebacondarwin Everytime an event is triggered it will call $setViewValue(). There we debounce the call if there is a pending one, so we call $cancelDebounce(), but I think we should skip the call to $render() in this case
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:47
but not re-render
Igor Minar
@IgorMinar
Apr 05 2014 21:47
sorry for stupid questions, but what's wrong with the second (on blur) example?
what's the expected vs actual behavior
?
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:47
The second one is fixed by calling $render
If you remove that line from the script.js it is a mess
Shahar Talmi
@shahata
Apr 05 2014 21:49
expected - input value gets reset when u press escape. actual - it remains the same since the ngModelWatch ignores the update
@lrlopez of course, this is what @petebacondarwin originally suggested
Luis Ramón López
@lrlopez
Apr 05 2014 21:54
yes, @shahata. I think the only change we should do is what he suggested in the first place. But I agree that we could rename $cancelDebounce() to $cancelUpdate() so it works in every case. If someone needs to rerender the view but not cancel a debounce, he should call $render() directly
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:54
Here is a clearer example: http://plnkr.co/edit/izzrbB?p=preview
Igor Minar
@IgorMinar
Apr 05 2014 21:56
yeah, that's better
but did you notice that if you repeat the instructions twice for the broken input it will work?
Luis Ramón López
@lrlopez
Apr 05 2014 21:56
We could use that as an example and as a Protractor test
Igor Minar
@IgorMinar
Apr 05 2014 21:56
it doesn't work only before the first update
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:56
So we are saying that we should have :
this.$cancelUpdate = function(skipRender) {
    if ( pendingDebounce ) {
      $timeout.cancel(pendingDebounce);
      pendingDebounce = null;
    }
    if ( !skipRender ) {
      this.$render();
    }
  };
When called from $setViewValue() it passes true
@IgorMinar - this is because the model has changed?
Igor Minar
@IgorMinar
Apr 05 2014 21:58
i don't know.. I haven't looked at the code much. only played with the examples you guys posted
Luis Ramón López
@lrlopez
Apr 05 2014 21:58
Also, here are the updated tests:
    it('should allow canceling pending updates', inject(function($timeout) {
      compileInput(
        '<input type="text" ng-model="name" name="alias" '+
          'ng-model-options="{ debounce: 10000 }" />');

      changeInputValueTo('a');
      expect(scope.name).toEqual(undefined);
      $timeout.flush(2000);
      scope.form.alias.$cancelUpdate();
      expect(scope.name).toEqual(undefined);
      $timeout.flush(10000);
      expect(scope.name).toEqual(undefined);
    }));


    it("should reset the input value when cancelUpdate is called", inject(function($timeout) {
      compileInput(
        '<input type="text" ng-model="name" name="alias" '+
          'ng-model-options="{ debounce: 2000 }" />');

      changeInputValueTo('a');
      scope.form.alias.$cancelUpdate();
      expect(inputElm.val()).toBe('');
      $timeout.flush(3000);
      expect(inputElm.val()).toBe('');
    }));
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 21:58
It only fails if the original model is the same as what you want to reset the view to
Since the dirty checking is unable to realise that we want to make a change to the view, since the model hasn't changed (yet)
Shahar Talmi
@shahata
Apr 05 2014 21:59
BTW, I wanted to ask about that - Is it really necessary to test pendingDebounce and set it to null? or can we simply call $timeout.cancel() without all the rest?
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:00
Good question @shahata
$timeout.cancel(promise) does fail quietly if promise is not a valid promise
Luis Ramón López
@lrlopez
Apr 05 2014 22:01
Yes, I've just checked it
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:01
So we could just have:
this.$cancelUpdate = function(skipRender) {
    $timeout.cancel(pendingDebounce);
    if ( !skipRender ) {
      this.$render();
    }
  };
Infact:
this.$cancelUpdate = function() {
   $timeout.cancel(pendingDebounce);
   this.$render();
};
and in $setViewValue() just $timeout.cancel(pendingDebounce)
Luis Ramón López
@lrlopez
Apr 05 2014 22:03
He he, nice one.
Shahar Talmi
@shahata
Apr 05 2014 22:03
:)
Luis Ramón López
@lrlopez
Apr 05 2014 22:04
@shahata could you create a PR with all these changes? You deserve the credit
Shahar Talmi
@shahata
Apr 05 2014 22:04
np
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:05
I've got to head to bed. Glad you are on board @shahata . Thanks @lrlopez for the initial discussions earlier and thanks @IgorMinar for keeping an eye on us !
Shahar Talmi
@shahata
Apr 05 2014 22:05
BTW, I was thinking a bit about this feature. Maybe it would be possible to easily add {updateOn: 'submit'} to a form. The current implementation is not very compatible with it, since in this case we would want to run $parsers immediately when an input is changed, so it would need some tweaking. What do you think about it?
Luis Ramón López
@lrlopez
Apr 05 2014 22:06
Thanks to you too Pete for your support!
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:06
What about {updateOn: 'submit default'}?
Luis Ramón López
@lrlopez
Apr 05 2014 22:06
Yeah, updateOn: 'submit' will be mandatory when using delayed updates in you're processing forms in the client side. Otherwise it wouldn't post the actual input value
Shahar Talmi
@shahata
Apr 05 2014 22:07
I meant that the model of all inputs in the for will not get update until the form is submitted
in the form
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:08
Right, so what you are saying is that on form submit we need to force all the promises to resolve?
Luis Ramón López
@lrlopez
Apr 05 2014 22:10
Yep. This is a very special case, but it should be optional. We could just add a new option to ngModelOptions, i.e. forceUpdateOnSubmit or something better
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:10
We could modify NgModelController to listen for a "submit" event on the FormController?
Luis Ramón López
@lrlopez
Apr 05 2014 22:11
yeah: the flag could add a submit event handler
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:11
if the input had ng-model-options="{ updateOn: 'submit', debounce: { submit: 0} }" then this would act as the flag no?
Shahar Talmi
@shahata
Apr 05 2014 22:12
the issue u are discussing is also important, but I meant something else entirely :). currently when ppl let the user edit something in the form they always create a copy of their model and associate the form to it, then wait for the submit and angular.copy the model of the form back to the original model. I'm saying that I would like to let ppl configure the form so that they don't need to copy the model - it will simply not get updated until the submit similarily to how it currently doesn't get updated until the blur.
Luis Ramón López
@lrlopez
Apr 05 2014 22:14
@petebacondarwin That was my initial idea, but I don't know if setting up a different behavior for a particular event is nice. So, what do you think guys about it? Resolve everything by default when submit is in the event list or just use a different flag?
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:14
So this would be achieved with what I was suggesting, no? By applying updateOn: 'submit' to all the inputs, they would not update until the submit was triggered on the form. We just need to update the form to pass this event down the DOM to all the inputs...
Luis Ramón López
@lrlopez
Apr 05 2014 22:14
You're right Pete, as always
Pete Bacon Darwin
@petebacondarwin
Apr 05 2014 22:15
which could be done outside of angular core with a simple directive
if you don't like hijacking the builtin 'submit' event, it could be a custom one, say 'formSubmit'
Go to go. I look forward to seeing the PR @shahata
Luis Ramón López
@lrlopez
Apr 05 2014 22:16
Above there are some corrected tests for inputSpec.js
I'm also going to bed. It's already Sunday here :)
Shahar Talmi
@shahata
Apr 05 2014 22:18
here too :) good night guys
Luis Ramón López
@lrlopez
Apr 05 2014 22:19
bye!