Skip to content
This repository has been archived by the owner on May 5, 2018. It is now read-only.

fix(keypress): allow events to ommit key modifiers #486

Merged
merged 3 commits into from
Mar 15, 2013

Conversation

caiotoon
Copy link
Contributor

Programmatically triggered events that do not include key modifiers (altKey, shiftKey, ctrlKey) properties would fail even when combination didn't require these keys.

Programmatically triggered events that do not include key modifiers (`altKey`, `shiftKey`, `ctrlKey`) properties would fail even when combination didn't require these keys.
@ProLoser
Copy link
Member

Strange, no idea why we're even using this || false pattern when we should probably just be using !!

@ProLoser
Copy link
Member

@caiotoon
Copy link
Contributor Author

You're right regarding || false pattern. !! is better. Updated, also changed for other parts of code previously committed this way.

Regarding the test case, the only difference is that event.altKey, event.ctrlKey and event.shiftKey are not defined, leading the handlers to never run.

As you can see, in my test cases I don't use the createEvent utility. Do you believe we could remove these test cases due to its simplicity?

@caiotoon
Copy link
Contributor Author

This is the test that would fail:

it('should support modifiers to be ommited from event', function () {
  var keyEvent = jQuery.Event('keydown');
  keyEvent.keyCode = 13;
  createElement({'13': 'event=true'}).trigger(keyEvent);

  expect($scope.event).toBe(true);
});

Notice that keyEvent is not created by utility and the modifiers key are not set.

@ProLoser
Copy link
Member

If you would like to refactor the tests in this PR, go ahead. It may be more relevant to remove your test too then since that would mean the very first test and yours would then be identical.

@caiotoon
Copy link
Contributor Author

Just did it. Test utility do not send key modifiers unless explicitly defined. This way will cover all cases, and make my tests unnecessary indeed.

ProLoser added a commit that referenced this pull request Mar 15, 2013
fix(keypress): allow events to ommit key modifiers
@ProLoser ProLoser merged commit 497cccd into angular-ui:master Mar 15, 2013
@caiotoon caiotoon deleted the fix-keypress-custom-event branch March 15, 2013 19:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants