feat(keyEvents): support for <div (keyup.enter)="callback()"> #1136

divdavem · 2015-03-27T17:00:18Z

This pull request is a first implementation of #523. It adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key.

Here are some examples:
(keydown.shift.enter)
(keyup.space)
(keydown.control.shift.a)
(keyup.f1)

Key names mostly follow the DOM Level 3 event key values:
http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables

There are some limitations to be worked on (see later), but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout):

alt, control, shift, meta (those keys can be combined with other keys)
tab, enter, backspace, pause, scrolllock, capslock, numlock
insert, delete, home, end, pageup, pagedown
arrowup, arrowdown, arrowleft, arrowright
latin letters (a-z), function keys (f1-f12)
numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver)

There is a sample to play with in examples/src/key_events/.

Here is a list of limitations/items to work on:

Chrome and Firefox have different behavior, because they implement different versions of the DOM level 3 events specification (Chrome implements an old draft version with the "keyIdentifier" property, Firefox implements the latest draft version with the "key" property).
Especially, using the shift modifier changes the value of the "key" property, but not the value of the "keyIdentifier" property, which means "shift.1" on Chrome is reported as "shift:!" on Firefox (for the US qwerty keyboard).
As a consequence, on Firefox, keys can quickly be linked with a keyboard layout, for example: "shift.:" only works on layouts for which it is needed to press shift to get the colon (which is the case on US qwerty keyboards bot not on French azerty keyboards).
Chrome reports strange values for some keys (most keys not reported in my previous list, such as: ":", ";", "?", "<", ">" ...).
Chromedriver does not correctly simulate keys from the numeric keypad
It would probably be a good idea to add support for a "command" or "accel" modifier which would be "meta" on Mac and "control" on other systems (cf here)

vicb · 2015-03-27T17:11:45Z

modules/angular2/src/core/events/key_events.js

+
+  static parseEventName(eventName: string) {
+    eventName = eventName.toLowerCase();
+    var parts = StringWrapper.split(eventName, splitRegExp);


eventName.split('.') & drop the regexp

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

vicb · 2015-03-27T17:16:47Z

modules/angular2/src/core/events/key_events.js

+    var fullKey = "";
+    ListWrapper.forEach(modifierKeys, (modifierName) => {
+      if (ListWrapper.contains(parts, modifierName)) {
+        ListWrapper.remove(parts, modifierName);


Not sure if Dart allow removing in a forEach.
Anyway a counter might perform better.

edit: there would be no pb with dart you ieterate & remove on different lists

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

vicb · 2015-03-27T17:28:18Z

modules/angular2/src/core/events/key_events.js

+
+    var outsideHandler = shouldSupportBubble ?
+      KeyEventsPlugin.bubbleCallback(element, parsedEvent.fullKey, handler, this.manager.getZone()) :
+      KeyEventsPlugin.sameElementCallback(element, parsedEvent.fullKey, handler, this.manager.getZone());


factorize the 2 methods by passing shouldSupportBubble ?

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

vicb · 2015-03-27T17:34:07Z

modules/angular2/src/core/events/key_events.js

+    }
+    if (key.length == 0) {
+      throw new BaseException('A key must be specified.');
+    }


not sure if we should throw here or only return null

Someone might with a plugin that would support keydown.fancymodifier chained with this one

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

divdavem · 2015-03-27T19:50:59Z

@vicb Thank you very much for reviewing my pull request so quickly, and thank you for your comments. I have done all the changes you suggested.
The travis-ci failure for the js part does not seem to be linked with my changes. However, I can see here that the e2e test I added is still failing for dart:

http://localhost:8002/examples/src/key_events/index.dart.js 1791:3 Uncaught Uncaught Error: NoSuchMethodError: undefined is not a function

Unfortunately, dart does not work well on my Windows machine currently, so I am unable to check what's going wrong right now, but I will try to fix it.
For info, I will probably not have time to look at it before Monday.
Have a good week-end!

vicb · 2015-03-30T09:34:56Z

modules/angular2/src/core/events/key_events.js

+  }
+
+  static getEventKey(event): string {
+    var key = event.key;


key does not seem to be a valid Dart property, see https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart:html.KeyboardEvent

@vicb Thank you for you comment.

As far as I can see, there is no equivalent for the standard key property. The keyCode property could be used, but it is deprecated:

This shouldn't be used by new web applications. IE and Firefox already (partially) support KeyboardEvent.key. Also, Google Chrome and Safari support KeyboardEvent.keyIdentifier which is defined in the old draft of DOM Level 3 Events. If you can use them or one of them, you should use them/it as far as possible.

Is there any hope that Dart can be updated soon to follow web standards regarding key events?
Or should we include in the Dart version of angular2 a map of keyCode values to their corresponding key values? I feel that it does not make much sense, because the Dart code is ultimately transpiled to JS (as Dart will not be supported natively in Chrome), and the key or keyIdentifier property would be available if the check done by Dart was not so strict... Is there any way to bypass the strict check from Dart?

What do you suggest?

Also, for info, I have tried today to make the dart build work on my Windows machine, but it is still failing, both on my branch and on master. Here is the error I am getting when running gulp build.dart:

dist\dart\angular2:[hint] Unused import (D:\git_clones\angular\dist\dart\angular2\test\core\compiler\view_container_spec.dart, line 8, col 8)
dist\dart\angular2:[hint] Unused import (D:\git_clones\angular\dist\dart\angular2\test\core\compiler\view_spec.dart, line 20, col 8)
dist\dart\angular2:[hint] Unused import (D:\git_clones\angular\dist\dart\angular2\test\test_lib\test_lib_spec.dart, line 16, col 8)
[16:37:52] 'build/analyze.dart' errored after 8.06 s
[16:37:52] Error: Dartanalyzer showed hints
at ChildProcess. (D:\git_clones\angular\tools\build\dartanalyzer.js:84:19)
at ChildProcess.emit (events.js:98:17)
at maybeClose (child_process.js:756:16)
at Process.ChildProcess._handle.onexit (child_process.js:823:5)
[16:37:52] 'build.dart' errored after 53 s
[16:37:52] Error: [object Object]
at formatError (D:\git_clones\nodist\bin\node_modules\gulp\bin\gulp.js:169:10)
at Gulp. (D:\git_clones\nodist\bin\node_modules\gulp\bin\gulp.js:195:15)
at Gulp.emit (events.js:117:20)
at Gulp.Orchestrator._emitTaskDone (D:\git_clones\angular\node_modules\gulp\node_modules\orchestrator\index.js:264:8)
at D:\git_clones\angular\node_modules\gulp\node_modules\orchestrator\index.js:275:23
at finish (D:\git_clones\angular\node_modules\gulp\node_modules\orchestrator\lib\runTask.js:21:8)
at cb (D:\git_clones\angular\node_modules\gulp\node_modules\orchestrator\lib\runTask.js:29:3)
at finish (D:\git_clones\angular\node_modules\run-sequence\index.js:38:5)
at Gulp.onError (D:\git_clones\angular\node_modules\run-sequence\index.js:45:4)
at Gulp.emit (events.js:117:20)

vicb · 2015-03-31T09:23:57Z

@divdavem you should probably check dartbug.com or create a new issue there to get some information/plan about implementation plans.
In the meantime, Dart should also be supported, should you create a separate .dart file with a map.

Your build error seems to be caused by unused imports

dist\dart\angular2:[hint] Unused import (D:\git_clones\angular\dist\dart\angular2\test\core\compiler\view_container_spec.dart, line 8, col 8)
dist\dart\angular2:[hint] Unused import (D:\git_clones\angular\dist\dart\angular2\test\core\compiler\view_spec.dart, line 20, col 8)
dist\dart\angular2:[hint] Unused import (D:\git_clones\angular\dist\dart\angular2\test\test_lib\test_lib_spec.dart, line 16, col 8)

The strange thing is that a first look your PR does not seem to affect those files, can you build the master branch ?

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

vicb · 2015-04-06T07:09:24Z

@divdavem what is the status on this PR ?

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

divdavem · 2015-04-08T16:26:18Z

@vicb Thank you for your comments.
I have updated the PR to make it compatible with dart (I used a virtual machine with Linux, as I gave up trying to make dart work on my Windows machine) and I added unit tests for the parseEventName function (to follow the suggestion from @pkozlowski-opensource ).
Note that the map to convert keyCode values to key values for Dart does not include all possible keys (only those I reported as reliable at the beginning of this PR). The missing keys will be reported as 'Unidentified'.
Could you have a look at my PR again? It could be merged if it is ok.

vicb · 2015-04-10T08:07:25Z

modules/angular2/src/dom/browser_adapter.dart

@@ -14,6 +14,87 @@ class _IdentitySanitizer implements NodeTreeSanitizer {

 final _identitySanitizer = new _IdentitySanitizer();

+final _keyCodeToKeyMap = const {
+  8: 'Backspace',


I'm wondering if we should add constants for "special" keys, ie
var KEY_BACKSPACE = 'Backspace';

that's something we can do later, should not block the PR

vicb · 2015-04-10T08:14:36Z

lgtm with a few minor comments

This commit adds a plugin for the event manager, to allow a key name to be appended to the event name (for keyup and keydown events), so that the callback is only called for that key. Here are some examples: (keydown.shift.enter) (keyup.space) (keydown.control.shift.a) (keyup.f1) Key names mostly follow the DOM Level 3 event key values: http://www.w3.org/TR/DOM-Level-3-Events-key/#key-value-tables There are some limitations to be worked on (cf details in angular#1136) but for now, this implementation is reliable for the following keys (by "reliable" I mean compatible with Chrome and Firefox and not depending on the keyboard layout): - alt, control, shift, meta (those keys can be combined with other keys) - tab, enter, backspace, pause, scrolllock, capslock, numlock - insert, delete, home, end, pageup, pagedown - arrowup, arrowdown, arrowleft, arrowright - latin letters (a-z), function keys (f1-f12) - numbers on the numeric keypad (but those keys are not correctly simulated by Chromedriver) There is a sample to play with in examples/src/key_events/. close angular#523 close angular#1136

divdavem · 2015-04-10T11:58:15Z

@vicb Thank you for your review. I have updated the code.

angular-automatic-lock-bot · 2019-09-06T04:22:18Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Mar 27, 2015

vicb reviewed Mar 27, 2015
View reviewed changes

divdavem force-pushed the keyEvents branch from 7b59c44 to 1fad12d Compare March 27, 2015 17:16

vicb reviewed Mar 27, 2015
View reviewed changes

divdavem force-pushed the keyEvents branch from 1fad12d to 628864d Compare March 27, 2015 17:25

vicb reviewed Mar 27, 2015
View reviewed changes

divdavem force-pushed the keyEvents branch from 628864d to b85a8c7 Compare March 27, 2015 17:33

vicb reviewed Mar 27, 2015
View reviewed changes

divdavem force-pushed the keyEvents branch from b85a8c7 to 9c191df Compare March 27, 2015 18:00

divdavem force-pushed the keyEvents branch from 9c191df to e618e20 Compare March 27, 2015 18:18

vicb reviewed Mar 30, 2015
View reviewed changes

divdavem force-pushed the keyEvents branch from e618e20 to 0041b58 Compare March 31, 2015 17:41

rkirov assigned vicb Apr 2, 2015

rkirov added the ♥ ongoing label Apr 2, 2015

mhevery modified the milestones: M7: GT Customer (blocking), M8: GT Customer (non-blocking) Apr 6, 2015

divdavem force-pushed the keyEvents branch from 0041b58 to 051b2a7 Compare April 8, 2015 08:46

divdavem force-pushed the keyEvents branch from 051b2a7 to 2ee257c Compare April 8, 2015 16:08

divdavem force-pushed the keyEvents branch from 2ee257c to a698b93 Compare April 8, 2015 16:08

vicb reviewed Apr 10, 2015
View reviewed changes

divdavem force-pushed the keyEvents branch from a698b93 to 8fa1539 Compare April 10, 2015 11:30

vicb merged commit 8fa1539 into angular:master Apr 10, 2015

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(keyEvents): support for <div (keyup.enter)="callback()"> #1136

feat(keyEvents): support for <div (keyup.enter)="callback()"> #1136

divdavem commented Mar 27, 2015

vicb Mar 27, 2015

vicb Mar 27, 2015

vicb Mar 27, 2015

vicb Mar 27, 2015

divdavem commented Mar 27, 2015

vicb Mar 30, 2015

divdavem Mar 30, 2015

vicb commented Mar 31, 2015

vicb commented Apr 6, 2015

divdavem commented Apr 8, 2015

vicb Apr 10, 2015

vicb commented Apr 10, 2015

divdavem commented Apr 10, 2015

angular-automatic-lock-bot bot commented Sep 6, 2019

feat(keyEvents): support for <div (keyup.enter)="callback()"> #1136

feat(keyEvents): support for <div (keyup.enter)="callback()"> #1136

Conversation

divdavem commented Mar 27, 2015

vicb Mar 27, 2015

Choose a reason for hiding this comment

vicb Mar 27, 2015

Choose a reason for hiding this comment

vicb Mar 27, 2015

Choose a reason for hiding this comment

vicb Mar 27, 2015

Choose a reason for hiding this comment

divdavem commented Mar 27, 2015

vicb Mar 30, 2015

Choose a reason for hiding this comment

divdavem Mar 30, 2015

Choose a reason for hiding this comment

vicb commented Mar 31, 2015

vicb commented Apr 6, 2015

divdavem commented Apr 8, 2015

vicb Apr 10, 2015

Choose a reason for hiding this comment

vicb commented Apr 10, 2015

divdavem commented Apr 10, 2015

angular-automatic-lock-bot bot commented Sep 6, 2019