Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keyDown, 'keyPress and friends do not support altKey or ctrlKey` values on the events #5

Open
jvanoostveen opened this issue Jun 17, 2016 · 3 comments

Comments

@jvanoostveen
Copy link
Member

The control or alt key are not taken into account when handling a keyDown or keyPress.

Example test scenario / API:

it('will remember if alt or ctrl key is pressed when simulating another keyPress', () => {
  let handleKeyDown = sinon.stub();
  let vnode = h('div', { tabindex: '-1', onkeydown: handleKeyDown });

  var nodeQuery = createQuery(vnode);
  nodeQuery.simulate.keyDown(17); // control
  nodeQuery.simulate.keyDown(18); // alt
  nodeQuery.simulate.keyPress(97, '', ''); // a

  expect(handleKeyDown).to.have.been.calledThrice;

  let event: KeyboardEvent = handleKeyDown.lastCall.args[0];
  expect(event.ctrlKey, 'ctrlKey should be set').to.be.true;
  expect(event.altKey, 'altKey should be set').to.be.true;

  nodeQuery.simulate.keyUp(17); // control
  nodeQuery.simulate.keyUp(18); // alt
  nodeQuery.simulate.keyPress(97, '', ''); // a

  event = handleKeyDown.lastCall.args[0];
  expect(event.ctrlKey, 'ctrlKey should not be set').to.be.false;
  expect(event.altKey, 'altKey should not be set').to.be.false;
});

This would require the simulator to remember the state.

Issue in current implementation:

  • each time nodeQuery.simulate is done, a new simulator is created.

Issues when one simulator per query is created:

  • it might leak state if not reset properly.
  • if the query is not stored, it would still create a new simulator for each query.

Other possible solution would be to allow some event data in the keyDown/keyPress calls:

nodeQuery.simulate.keyDown(97, undefned, { ctrlKey: true, altKey: true });
nodeQuery.simulate.keyPress(97, '', '', undefned, { ctrlKey: true, altKey: true });

but this does not make a really nice API or corresponds on how the keys are handles by the browser (it would not help you making sure you can handle a keyDown event for only the control key).

@jvanoostveen
Copy link
Member Author

A possible patch for the simulator.
It would still require a solution/fix for the NodeQuery.simulate returning a new Simulator on each access.

Index: src/simulator.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/simulator.ts    (revision c73295345ac4f90ba6f295ee3b9c6a9a493bd2ff)
+++ src/simulator.ts    (revision )
@@ -67,13 +67,6 @@
   return <any>result;
 };

-let createKeyEvent = (which: number, target: any): KeyboardEvent => {
-  let event = <any>createEvent(target);
-  event.which = which;
-  event.keyCode = which;
-  return event;
-};
-
 let createMouseEvent = (target: any, parameters?: MouseEventParameters): MouseEvent => {
   let event = <any>createEvent(target);
   if (parameters) {
@@ -92,16 +85,34 @@
 };

 export let createSimulator = (vnode: VNode, defaultFakeDomNode?: Object): Simulator => {
+  let ctrlKeyPressed = false;
+  let altKeyPressed = false;
+
+  let createKeyEvent = (which: number, target: any): KeyboardEvent => {
+    let event = <any>createEvent(target);
+    event.which = which;
+    event.keyCode = which;
+    event.ctrlKey = ctrlKeyPressed;
+    event.altKey = altKeyPressed;
+    return event;
+  };
+
   let properties = vnode.properties;
   return {

     keyDown: (keyCode: number | string, fakeDomNode?: Object) => {
+      ctrlKeyPressed = keyCode === 17 ? true : ctrlKeyPressed;
+      altKeyPressed = keyCode === 18 ? true : altKeyPressed;
+
       let event = createKeyEvent(getKeyCode(keyCode), fakeDomNode || defaultFakeDomNode);
       properties.onkeydown(event);
       return event;
     },

     keyUp: (keyCode: number | string, fakeDomNode?: Object) => {
+      ctrlKeyPressed = keyCode === 17 ? false : ctrlKeyPressed;
+      altKeyPressed = keyCode === 18 ? false : altKeyPressed;
+
       let event = createKeyEvent(getKeyCode(keyCode), fakeDomNode || defaultFakeDomNode);
       properties.onkeyup(event);
       return event;

@johan-gorter
Copy link
Contributor

It is technically possible for the user to have the ctrl down, before an element or the page gains the focus. This means the element would not receive the keydown for the ctrl key, but still, the event.ctrlKey would be set.

A better approach would be to either invoke VNode.properties.onkeydown() manually or have the second argument to simulate.keydown be an Object that would be merged with the Event to dispatch.

@jvanoostveen
Copy link
Member Author

For now, I've fallen back to using the VNode.properties.onkeydown(), but I think the extra object (replacing and including the fakeDomNode) would be a nice solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants