ng-pattern-restrict icon indicating copy to clipboard operation
ng-pattern-restrict copied to clipboard

Consider triggering change event after reverting to previous value

Open rickarubio opened this issue 9 years ago • 9 comments

Hi @AlphaGit, I'm currently using ng-pattern-restrict with a slight modification:

Original Version:

function revertToPreviousValue() {
  iElement.val(oldValue);

   if (!angular.isUndefined(caretPosition)) {
     setCaretPosition(caretPosition);
    }
}

Normally, whenever a user changes the input value, a change event is automatically fired. Many other 3rd party directives such as angular-floating-labels depend on an input or change event to be triggered to take some action.

val() will not trigger any events to be fired, so when the value is reverted, any other directives which may also be applied to an input will be unaware of the changes made by val().

Is this something that was omitted by design?

If not, would you like me to submit a pull request to add this in?

Modified Version:

function revertToPreviousValue() {
  iElement.val(oldValue);
  iElement.trigger("change");

   if (!angular.isUndefined(caretPosition)) {
     setCaretPosition(caretPosition);
    }
}

rickarubio avatar Jan 26 '17 23:01 rickarubio

Hi @rickarubio! Thanks a lot for the feedback.

Your appreciation is correct. I did have some corrections in place to deal with this behavior related to ngModel, but it feels more appropriate to trigger the low-level change event and have that affect both the low-level hanging directives and the high-level binding. These last changes I am talking about are in #43.

Yes, if you could create a PR for that, it'd be great. If you find yourself without much time available, I can help you out, since you already cleared out the path and the change is pretty straightforward.

I may ask you as well to contribute to the e2e tests. Again, if you don't have time for it, I can set it up myself. Pretty much we want to add a test that will hook up to the change event and display/do something that protractor can verify. Doing that in the revert situation (rejected input) should not trigger the right event, and with this new change, it should.

Thanks a lot!

AlphaGit avatar Jan 27 '17 01:01 AlphaGit

Thanks @AlphaGit I will submit a PR this weekend with the e2e tests, would love to help out where I can!

rickarubio avatar Jan 27 '17 19:01 rickarubio

I've followed the readme to get the protractor specs running but I keep getting:

Running "connect:server" (connect) task
Started connect web server on http://localhost:9001

Running "protractor_webdriver:webDriverStart" (protractor_webdriver) task
Starting Selenium server
Started Selenium server: http://127.0.0.1:4444

Running "protractor:test" (protractor) task
Using the selenium server at http://localhost:4444/wd/hub
[launcher] Running 1 instances of WebDriver
Session created: count=1, browserName=chrome
Exception thrown: Going to shut down the Selenium server
Shutting down Selenium server: http://127.0.0.1:4444
Shut down Selenium server: http://127.0.0.1:4444 (OKOK)
Fatal error: 18:18:39.871 WARN - Exception thrown
npm ERR! Test failed.  See above for more details.

Which prevents me from being able to run any of the specs.

Using latest versions of firefox/chrome and did webdriver-manager update.

Attempted to run specs using npm test, then manually running webdriver-manager start & protractor protractor-conf.js

At this point, I will defer the changes to you, although I am curious what may be missing from the readme instructions to run the specs locally.

rickarubio avatar Jan 30 '17 02:01 rickarubio

Hi! That's really strange -- did you get any particular error from Selenium?

I remember having a similar issue when refactoring stuff around and the browser wouldn't load the page at all. Also, when trying to use the IE webdriver, which does not work for me.

I'll look into this later on and get back to you. Let me know if you get any more detail on the error -- I'd love to improve the documentation for any other possible step I may have missed.

AlphaGit avatar Jan 30 '17 12:01 AlphaGit

For reference (if you don't mind helping me, @rickarubio), here's the list of my installed npm packages:

My packages:

$ npm list
[email protected] D:\GitHub\ng-pattern-restrict
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\bufferutil\package.json
+-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| |   `-- [email protected]
| +-- [email protected]
| `-- [email protected]
+-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| `-- [email protected]
+-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | | `-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | | `-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | +-- [email protected]
| | | | +-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   | `-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| `-- [email protected]
|   `-- [email protected]
+-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   | +-- [email protected]
| | |   | +-- [email protected]
| | |   | +-- [email protected]
| | |   | +-- [email protected]
| | |   | `-- [email protected]
| | |   `-- [email protected]
| | `-- [email protected]
| |   +-- [email protected]
| |   `-- [email protected]
| |     +-- [email protected]
| |     | +-- [email protected]
| |     | `-- [email protected]
| |     +-- [email protected]
| |     +-- [email protected]
| |     +-- [email protected]
| |     | +-- [email protected]
| |     | +-- [email protected]
| |     | | `-- [email protected]
| |     | +-- [email protected]
| |     | `-- [email protected]
| |     |   +-- [email protected]
| |     |   | `-- [email protected]
| |     |   `-- [email protected]
| |     |     +-- [email protected]
| |     |     `-- [email protected]
| |     +-- [email protected]
| |     +-- [email protected]
| |     | +-- [email protected]
| |     | | +-- [email protected]
| |     | | `-- [email protected]
| |     | |   `-- [email protected]
| |     | `-- [email protected]
| |     |   +-- [email protected]
| |     |   | +-- [email protected]
| |     |   | +-- [email protected]
| |     |   | | `-- [email protected]
| |     |   | +-- [email protected]
| |     |   | +-- [email protected]
| |     |   | | `-- [email protected]
| |     |   | `-- [email protected]
| |     |   |   `-- [email protected]
| |     |   `-- [email protected]
| |     |     +-- [email protected]
| |     |     +-- [email protected]
| |     |     `-- [email protected]
| |     |       `-- [email protected]
| |     +-- [email protected]
| |     | +-- [email protected]
| |     | | `-- [email protected]
| |     | |   `-- [email protected]
| |     | |     `-- [email protected]
| |     | `-- [email protected]
| |     `-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| |   +-- [email protected]
| |   +-- [email protected]
| |   +-- [email protected]
| |   `-- [email protected]
| `-- [email protected]
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\grunt-karma\package.json
+-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| `-- [email protected]
|   +-- [email protected]
|   | +-- [email protected]
|   | +-- [email protected]
|   | `-- [email protected]
|   `-- [email protected]
+-- [email protected]
| `-- [email protected]
|   `-- [email protected]
+-- [email protected] extraneous
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma\package.json
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma-chrome-launcher\package.json
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma-firefox-launcher\package.json
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma-ie-launcher\package.json
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma-junit-reporter\package.json
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma-ng-scenario\package.json
+--  extraneous error: ENOENT: no such file or directory, open 'D:\GitHub\ng-pattern-restrict\node_modules\karma-phantomjs-launcher\package.json
`-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  |   +-- [email protected]
  |   `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | `-- [email protected]
  | |   `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   | `-- [email protected]
  | |   +-- [email protected]
  | |   | `-- [email protected]
  | |   |   +-- [email protected]
  | |   |   `-- [email protected]
  | |   +-- [email protected]
  | |   `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | +-- UNMET OPTIONAL DEPENDENCY [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | `-- UNMET OPTIONAL DEPENDENCY [email protected]
  | `-- [email protected]
  |   +-- [email protected]
  |   `-- [email protected]
  `-- [email protected]
    `-- [email protected]
      `-- [email protected]

My globally installed packages:

$ npm list -g
C:\Program Files\nodejs
+-- [email protected]
+-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| |   `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   `-- [email protected]
| | |     +-- [email protected]
| | |     | +-- [email protected]
| | |     | +-- [email protected]
| | |     | | `-- [email protected]
| | |     | `-- [email protected]
| | |     |   `-- [email protected]
| | |     `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   | `-- [email protected]
| | |   |   `-- [email protected]
| | |   |     `-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | |     `-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   | `-- [email protected]
| | |   +-- [email protected]
| | |   +-- [email protected]
| | |   | `-- [email protected]
| | |   |   +-- [email protected]
| | |   |   `-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | | `-- [email protected]
| | | | |   +-- [email protected]
| | | | |   +-- [email protected]
| | | | |   | `-- [email protected]
| | | | |   +-- [email protected]
| | | | |   `-- [email protected]
| | | | +-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | | `-- [email protected]
| | | | `-- [email protected]
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | | `-- [email protected]
| | | | `-- [email protected]
| | | `-- [email protected]
| | |   +-- [email protected]
| | |   `-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| +-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | `-- [email protected]
| +-- [email protected]
| | +-- [email protected]
| | `-- [email protected]
| `-- [email protected]
|   +-- [email protected]
|   +-- [email protected]
|   | `-- [email protected]
|   +-- [email protected]
|   +-- [email protected]
|   | `-- [email protected]
|   |   `-- [email protected]
|   +-- [email protected]
|   | +-- [email protected]
|   | `-- [email protected]
|   |   `-- [email protected]
|   +-- [email protected]
|   `-- [email protected]
`-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  |     `-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  |     `-- [email protected]
  |       +-- [email protected]
  |       `-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | |   +-- [email protected]
  | |   `-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | | `-- [email protected]
  | | |   +-- [email protected]
  | | |   `-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | |   +-- [email protected]
  | |   `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | | `-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | | +-- [email protected]
  | | | | | `-- [email protected]
  | | | | `-- [email protected]
  | | | |   `-- [email protected]
  | | | `-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  |     +-- [email protected]
  |     | `-- [email protected]
  |     `-- [email protected]
  |       +-- [email protected]
  |       `-- [email protected]
  |         `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | `-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | | `-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | | +-- [email protected]
  | | | | | `-- [email protected]
  | | | | `-- [email protected]
  | | | |   `-- [email protected]
  | | | `-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | | `-- [email protected]
  | | | `-- [email protected]
  | | |   `-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | | `-- [email protected]
  | | |   +-- [email protected]
  | | |   `-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | `-- [email protected]
  | | +-- [email protected]
  | | | `-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | | `-- [email protected]
  | | | +-- [email protected]
  | | | `-- [email protected]
  | | `-- [email protected]
  | |   `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | +-- [email protected]
  | | | +-- [email protected]
  | | | +-- [email protected]
  | | | `-- [email protected]
  | | `-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   +-- [email protected]
  | |   `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  |   +-- [email protected]
  |   `-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  | `-- [email protected]
  +-- [email protected]
  `-- [email protected]

My npm version:

$ npm -v
3.10.8

My node version:

$ node -v
v7.0.0

Sorry for the extraneous packages -- I was working in another branch for Angular2 compatibility and checking out branches back and forth.

Also, I'm using a Windows 10 x64 platform.

I'll get started on the changes now.

AlphaGit avatar Jan 30 '17 23:01 AlphaGit

Hey, after setting up my tests, I couldn't reproduce the scenario at all. I detected that in fact multiple change events where being triggered by the current implementation.

In this scenario, I'm testing with the following approach:

<input type="text" id="textbox" ng-pattern-restrict="{{pattern}}" pattern="{{htmlPattern}}" ng-model="someValue" ng-change="lastChangeDetected = someValue" />
<p id="lastChangeDetected">{{lastChangeDetected}}</p>

testingchange

The browsers are:

  • Chrome 55.0.2883.87 m
  • Firefox 40.0.3
  • IE 11.576.14393.0
  • Edge 38.14393.0.0

In each of the browsers, I'm trying to erase as back as possible until the expression (empty) will be rejected. At that point, revertValue kicks in and replaces it with the last valid value. You can see that in all cases, the model is synchronized with the actual view value of the input.

I also tested a more vanilla approach, just in case Angular was playing on me here:

<input type="text" id="textbox" ng-pattern-restrict="{{pattern}}" pattern="{{htmlPattern}}" />

<p id="lastChangeDetected"></p>

<script type="text/javascript">
  var lastChangeDetected = document.getElementById('lastChangeDetected');
  var text = document.getElementById('textbox');
  text.addEventListener('change', function() {
    console.debug('event firing');
    lastChangeDetected.innerText = text.value;
  });
</script>

In this case, the behavior stays, except that the change event is only fired when I leave the input, which, I believe, is common browser behavior for inputs.

That being the case, I could not reproduce the situation that you see as problematic. Would you mind to help me understand a case in which this is not behaving properly?

AlphaGit avatar Jan 31 '17 00:01 AlphaGit

Hey @AlphaGit ok, kind of long post here, apologies in advance:

TLDR; scroll to the bottom

OS: OSX 10.11.6 (El Capitan) Node: v7.0.0 NPM: 3.10.8

After wrangling with the getting the tests to run for awhile, I've finally got them running.

npm test always fails for me so instead I:

  1. edited Gruntfile.js to add keepalive: true to the connect task
  2. bower install, otherwise angularjs will be missing when the tests are run
  3. npm install -g webdriver-manager
  4. webdriver-manager update
  5. webdriver-manager start
  6. grunt connect
  7. grunt protractor test

All specs pass (I've branched off master) except for this one which consistently fails:

Running "protractor:test" (protractor) task
Using the selenium server at http://localhost:4444/wd/hub
[launcher] Running 1 instances of WebDriver
Focus
  should not focus the elements it uses - fail


  1) Focus should not focus the elements it uses
   Message:
     UnknownCommandError: POST /session/dcfbfff4-6642-244f-8879-14aa13f733bb/keys did not match a known command
Build info: version: '3.0.1', revision: '1969d75', time: '2016-10-18 09:48:19 -0700'
System info: host: 'rfsosx', ip: '192.168.1.103', os.name: 'Mac OS X', os.arch: 'x86_64', os.version: '10.11.6', java.version: '1.8.0_45'
Driver info: org.openqa.selenium.firefox.FirefoxDriver
Capabilities [{rotatable=false, raisesAccessibilityExceptions=false, count=1, firefoxOptions={args=[], profile=UEsDBBQACAgIACeNQUoAAAAAAAAAAAAAAAAHAAAAdXNlci5
qc51WTW/bMAy971cMOW3AKqTretlOXdcBA4Z1aFDsKMgSbauRJU0fcfPvR/mjSRNHbndKbJMS+fj4yOjBUeugfLconGnxiXhWQvdf6oo0TLXMAQHNCgVi8eFtyZSH91/exJ2nYAFtrHEhudTAVKj7Z4JGG8ln/
DWE1rg1qUOwxNbS19uz9Nky788U6CrU6Pjx8vK52xiwAybwR0AAHkB8l86HK4yFK0C34OJhuKbBvB4pr51pgHrupA3URU2DbJLLxXL6osAKTxAOfauvlfEwnc1oLUyrlWEC79KsSsDWpv1Tg14hWgmpaXeLQdn
g02W0MYKpGexhE4xRnoBzxnGjvVH7cB+n72WljUbUGmgKcKvu0edz8eC9RKtgkAsOfETcSgyUcsd8nfdVUq+JsaApPAZwmqlUzFczqExlvYt6+rIWCuHkBp8Z54DljBoz90gHysEFP4nEU6Wkt4ptQdycL1e/D
DInlfbTtDG+Erf6j9RYX3++JBIvMvd3P9FjwQoTw+dCMb1eHHOuT4gypeiDRzBSnLKH/ji2RysRbrQlbS0DKOkDHvA3SneKCYkGaxnI0E0j6zC5xIUsAIphYbAB8lTzibfRkhq7xuLZtAXFUwdFl0q6OEg5VVt
3rCHRYoGBaIS23N6jyat1JNrUSjfZ8IBHJ8MWmaIA/xEfnOSBGicrqak1SvJtnqoaWmw7MuSTqeazQPuTSXq5ikUju1b53b286sj4Mt2cPCab8VN3DoVJRUHLE+q160NMs+34e9yIpizRDs6YtZ4g+0xLiy0VU
LKowrScjLBzbwf+TEd7zIcs20Y6I/dRqYLbkl4ZO/uPc7bZo0dEbu5/XpELwnZFOk7vgW1YPyGyonpYirFH8jw6jtvBQzfEspf0NE3H5/PcQ5zFYL71SrDiSIIkOufLUxran0o1C/hIYQM6zAyIvdF4ixSbjad
Qhq87UXtRpTwwx+uBN3NjtyE9u4mIjc1WDVn+Ii4+I4xBhJwU04p6JEKH6nMxvfr0an02th/SKMQZyAfbE9Iy7Djzy83e5PTdXN9NTpSg2Gl1FrlErekg9nrH9KOu24F+6Ot+TTm5nQ0zi3ZqJRBGXdGk/zMkq
QHHJa+Br7/1gvW1//L6cUaYEDJFzGYmweTS/L/LsjJVlV4OW1xW+/d6qcv4GrUR27WQKrFPy6oO6sS023Mdb/wfaaNjI5AG98KOiIu0Szo4wGi3yqTVE4mNC+TMLrPre1X6aPsVYLdl/ANQSwcIy87bUaYDAAC
2DAAAUEsBAhQAFAAICAgAJ41BSsvO21GmAwAAtgwAAAcAAAAAAAAAAAAAAAAAAAAAAHVzZXIuanNQSwUGAAAAAAEAAQA1AAAA2wMAAAAA}, appBuildId=20170125094131, version=51.0.1, platfor
m=MAC, proxy={}, command_id=1, specificationLevel=0, acceptSslCerts=false, processId=80576, browserVersion=51.0.1, platformVersion=15.6.0, XULappId={ec8030f7-
c20a-464f-9b0e-13a3a9e97384}, browserName=firefox, takesScreenshot=true, takesElementScreenshot=true, platformName=darwin}]
Session ID: dcfbfff4-6642-244f-8879-14aa13f733bb
   Stacktrace:
     UnknownCommandError: POST /session/dcfbfff4-6642-244f-8879-14aa13f733bb/keys did not match a known command
Build info: version: '3.0.1', revision: '1969d75', time: '2016-10-18 09:48:19 -0700'
System info: host: 'rfsosx', ip: '192.168.1.103', os.name: 'Mac OS X', os.arch: 'x86_64', os.version: '10.11.6', java.version: '1.8.0_45'
Driver info: org.openqa.selenium.firefox.FirefoxDriver

However, if I switch to using chrome instead of firefox, all the test pass, but I had to modify the pageObjects pages by adding a class to identify the rootElement for protractor, following this fix: http://stackoverflow.com/questions/20059943/running-into-error-while-waiting-for-protractor-to-sync-with-the-page-with-basic

Using:

Firefox v51.0.1 64bit Chrome: Version 55.0.2883.95 (64-bit)

I also had a friend try clone down the repo and run specs (also OSX user), ran into the same issues as me trying to initially get specs running.

What do you think about using something along the lines of npm-shrinkwrap to lock down the dependencies to specific versions? It may help reduce inconsistencies between different installs of the ng-pattern-restrict repo.

Back to the original issue:

I've modified your original plunker to try show what this issue is about:

https://plnkr.co/edit/ngCI7ZJVsQpup1IN9aeH?p=preview

Try the following:

  1. type in "hello" -- notice that demo has a red background color applied
  2. backspace so that "hello" is completely erased -- "" -- notice that demo background color goes away
  3. type in "hello7" -- notice that demo is still red, but because of the revert to previous value function, user only sees "hello"
  4. now erase everything, demo is back to normal
  5. type in the number 3

notice that after step 5, demo is now red, but the input itself is blank to the user.

What just happened is that when you typed in 5, an input event was triggered, the value of the input at this point is 3.

Since 3 does not validate against the input ng-pattern-restrict regex, revertToPreviousValue kicks in and using the val() function, reverts to the previous value.

The problem here is that val() does not trigger any events, input or change.

Other directives, or functionality that depends on being able to take some action based on the input value is oblivious to the fact the input value was just changed programmatically.

That is why I added the trigger('change') line to the revertToPrevious function.

JQuery docs also hint at this:

Setting values using this method (or using the native value property) does not cause the dispatch of the change event. For this reason, the relevant event handlers will not be executed. If you want to execute them, you should call .trigger( "change" ) after setting the value.

So, when val() is used to change the input value back to the previous value, no input or change event is fired. Any other functionality that depends on knowing the fact that the input value has changed (to apply some styling perhaps for example) does not know about the change.

Hope that clears it up, let me know if this makes sense.

If you'd like the changes, seems like I have the tests working now on my system, I could add a test in, just let me know.

rickarubio avatar Feb 02 '17 06:02 rickarubio

@rickarubio I am absolutely impressed with your investigation and your explanation -- amazing work! Thank you very much for putting the time into this.

Yes, I believe freezing the dependencies is a great idea. Feel free to make the modifications and send the tests, and even to adjust the scripts so that everyone can run the tests. If you don't do that I'll completely understand, you've helped a lot already so I won't hold it against you. :P

Again, thank you very much, and yes, the changes are very welcome!

AlphaGit avatar Feb 04 '17 21:02 AlphaGit

@AlphaGit Thanks! Awesome, I'll have some time this weekend to submit PRs for the changes (adding change event firing, locking down dependencies, spec running tweaks).

rickarubio avatar Feb 07 '17 16:02 rickarubio