Issue #12859 Keyboard modifiers support for simulateKeyEvent#12863
Issue #12859 Keyboard modifiers support for simulateKeyEvent#12863petetnt merged 11 commits intoadobe:masterfrom
Conversation
|
Sorry about being so slow with this, been swamped with work as of late. I'll review this on Sunday / Monday. Again, apologies for taking so long! |
|
No worries! Thanks for putting aside time to review this.
… On 25 Nov 2016, at 08:47, Pete Nykänen ***@***.***> wrote:
Sorry about being so slow with this, been swamped with work as of late. I'll review this on Sunday / Monday.
Again, apologies for taking so long!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
petetnt
left a comment
petetnt
left a comment
There was a problem hiding this comment.
Some review comments, mostly looks good to me! Needs an unit test though 👍
| function simulateKeyEvent(key, event, element, options) { | ||
| var doc = element.ownerDocument; | ||
|
|
||
| if(typeof options === 'undefined') { |
There was a problem hiding this comment.
We can use Object.assign to make this a bit easier:
var defaultOptions = {
options.view = doc.defaultView;
options.bubbles = true;
options.cancelable = true;
options.keyIdentifier = key;
};
var newOptions = Object.assign({}, options, defaultOptions});
var oEvent = new KeyboardEvent(event, newOptions);| console.log("SpecRunnerUtils.simulateKeyEvent() - unsupported keyevent: " + event); | ||
| return; | ||
| } | ||
| } |
|
Made the requested changes, and provided a unit test. Had to pop it in a new file, |
| cancelable: true, | ||
| keyIdentifier: key | ||
| }; | ||
| var newOptions = Object.assign({}, options, defaultOptions); |
There was a problem hiding this comment.
It was brought up in another PR that using Object.assign actually brokes the Linux version due to using an age old CEF which doesn't support it 😢
If you could change this to the previous format then this one is ready to be merged
There was a problem hiding this comment.
No problem at all, reverted to the old version. 👍
|
I'll try and test this out for the final time today and then merge it in, LGTM so far 🥇 |
|
Great, thanks!
… On 22 Dec 2016, at 08:40, Pete Nykänen ***@***.***> wrote:
I'll try and test this out for the final time today and then merge it in, LGTM so far 🥇
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
| @@ -995,15 +995,28 @@ define(function (require, exports, module) { | |||
| * Simulate key event. Found this code here: | |||
There was a problem hiding this comment.
We can remove these comments now as the implementation is different
|
Okay, one more nitpick and then it's ready to go 👍 Great work @haslam22! |
|
Woops, forgot to update that part. Should be all good now. |
|
Thanks for this great contribution @haslam22! |
2 participants
Added support for sending additional arguments to simulateKeyEvent, e.g. modifiers such as
ctrlKey: trueor any other value supported byKeyboardEventInit.