Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit ad41baa

Browse files
LeeAdcockgkalpak
authored andcommitted
fix(ngAria): bind to keydown instead of keypress in ngClick
Previously, `ngAria` would provide keyboard support for non-native buttons (via `ngClick`), by binding the `ngClick` handler to the `keypress` event. In an attempt to better emulate the behavior of native buttons, `ngAria` will now bind to the `keydown` event (instead of `keypress`). The configuration flag for this feature has been renamed from `bindKeypress` to `bindKeydown`, to closer describe the underlying behavior. Fixes #14063 Closes #14065 BREAKING CHANGE: If you were explicitly setting the value of the `bindKeypress` flag, you need to change your code to use `bindKeydown` instead. Before: `$ariaProvider.config({bindKeypress: xyz})` After: `$ariaProvider.config({bindKeydown: xyz})` **Note:** If the element already has any of the `ngKeydown`/`ngKeyup`/`ngKeypress` directives, `ngAria` will _not_ bind to the `keydown` event, since it assumes that the developer has already taken care of keyboard interaction for that element. Although it is not expected to affect many applications, it might be desirable to keep the previous behavior of binding to the `keypress` event instead of the `keydown`. In that case, you need to manually use the `ngKeypress` directive (in addition to `ngClick`). Before: ```html <div ng-click="onClick()"> I respond to `click` and `keypress` (not `keydown`) </div> ``` After: ```html <div ng-click="onClick()" ng-keypress="onClick()"> I respond to `click` and `keypress` (not `keydown`) </div> <!-- OR --> <div ng-click="onClick()"> I respond to `click` and `keydown` (not `keypress`) </div> ``` Finally, it is possible that this change affects your unit or end-to-end tests. If you are currently expecting your custom buttons to automatically respond to the `keypress` event (due to `ngAria`), you need to change the tests to trigger `keydown` events instead.
1 parent 3043695 commit ad41baa

File tree

2 files changed

+80
-44
lines changed

2 files changed

+80
-44
lines changed

src/ngAria/aria.js

+23-20
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,19 @@
2121
*
2222
* Below is a more detailed breakdown of the attributes handled by ngAria:
2323
*
24-
* | Directive | Supported Attributes |
25-
* |---------------------------------------------|----------------------------------------------------------------------------------------|
24+
* | Directive | Supported Attributes |
25+
* |---------------------------------------------|-----------------------------------------------------------------------------------------------------|
2626
* | {@link ng.directive:ngModel ngModel} | aria-checked, aria-valuemin, aria-valuemax, aria-valuenow, aria-invalid, aria-required, input roles |
27-
* | {@link ng.directive:ngDisabled ngDisabled} | aria-disabled |
28-
* | {@link ng.directive:ngRequired ngRequired} | aria-required
29-
* | {@link ng.directive:ngChecked ngChecked} | aria-checked
30-
* | {@link ng.directive:ngReadonly ngReadonly} | aria-readonly |
31-
* | {@link ng.directive:ngValue ngValue} | aria-checked |
32-
* | {@link ng.directive:ngShow ngShow} | aria-hidden |
33-
* | {@link ng.directive:ngHide ngHide} | aria-hidden |
34-
* | {@link ng.directive:ngDblclick ngDblclick} | tabindex |
35-
* | {@link module:ngMessages ngMessages} | aria-live |
36-
* | {@link ng.directive:ngClick ngClick} | tabindex, keypress event, button role |
27+
* | {@link ng.directive:ngDisabled ngDisabled} | aria-disabled |
28+
* | {@link ng.directive:ngRequired ngRequired} | aria-required |
29+
* | {@link ng.directive:ngChecked ngChecked} | aria-checked |
30+
* | {@link ng.directive:ngReadonly ngReadonly} | aria-readonly |
31+
* | {@link ng.directive:ngValue ngValue} | aria-checked |
32+
* | {@link ng.directive:ngShow ngShow} | aria-hidden |
33+
* | {@link ng.directive:ngHide ngHide} | aria-hidden |
34+
* | {@link ng.directive:ngDblclick ngDblclick} | tabindex |
35+
* | {@link module:ngMessages ngMessages} | aria-live |
36+
* | {@link ng.directive:ngClick ngClick} | tabindex, keydown event, button role |
3737
*
3838
* Find out more information about each directive by reading the
3939
* {@link guide/accessibility ngAria Developer Guide}.
@@ -98,7 +98,7 @@ function $AriaProvider() {
9898
ariaInvalid: true,
9999
ariaValue: true,
100100
tabindex: true,
101-
bindKeypress: true,
101+
bindKeydown: true,
102102
bindRoleForClick: true
103103
};
104104

@@ -114,12 +114,15 @@ function $AriaProvider() {
114114
* - **ariaDisabled** – `{boolean}` – Enables/disables aria-disabled tags
115115
* - **ariaRequired** – `{boolean}` – Enables/disables aria-required tags
116116
* - **ariaInvalid** – `{boolean}` – Enables/disables aria-invalid tags
117-
* - **ariaValue** – `{boolean}` – Enables/disables aria-valuemin, aria-valuemax and aria-valuenow tags
117+
* - **ariaValue** – `{boolean}` – Enables/disables aria-valuemin, aria-valuemax and
118+
* aria-valuenow tags
118119
* - **tabindex** – `{boolean}` – Enables/disables tabindex tags
119-
* - **bindKeypress** – `{boolean}` – Enables/disables keypress event binding on `div` and
120-
* `li` elements with ng-click
121-
* - **bindRoleForClick** – `{boolean}` – Adds role=button to non-interactive elements like `div`
122-
* using ng-click, making them more accessible to users of assistive technologies
120+
* - **bindKeydown** – `{boolean}` – Enables/disables keyboard event binding on non-interactive
121+
* elements (such as `div` or `li`) using ng-click, making them more accessible to users of
122+
* assistive technologies
123+
* - **bindRoleForClick** – `{boolean}` – Adds role=button to non-interactive elements (such as
124+
* `div` or `li`) using ng-click, making them more accessible to users of assistive
125+
* technologies
123126
*
124127
* @description
125128
* Enables/disables various ARIA attributes
@@ -373,8 +376,8 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
373376
elem.attr('tabindex', 0);
374377
}
375378

376-
if ($aria.config('bindKeypress') && !attr.ngKeypress) {
377-
elem.on('keypress', function(event) {
379+
if ($aria.config('bindKeydown') && !attr.ngKeydown && !attr.ngKeypress && !attr.ngKeyup) {
380+
elem.on('keydown', function(event) {
378381
var keyCode = event.which || event.keyCode;
379382
if (keyCode === 32 || keyCode === 13) {
380383
scope.$apply(callback);

test/ngAria/ariaSpec.js

+57-24
Original file line numberDiff line numberDiff line change
@@ -681,8 +681,8 @@ describe('$aria', function() {
681681
var divElement = elements.find('div');
682682
var liElement = elements.find('li');
683683

684-
divElement.triggerHandler({type: 'keypress', keyCode: 32});
685-
liElement.triggerHandler({type: 'keypress', keyCode: 32});
684+
divElement.triggerHandler({type: 'keydown', keyCode: 32});
685+
liElement.triggerHandler({type: 'keydown', keyCode: 32});
686686

687687
expect(clickFn).toHaveBeenCalledWith('div');
688688
expect(clickFn).toHaveBeenCalledWith('li');
@@ -703,32 +703,57 @@ describe('$aria', function() {
703703
var divElement = elements.find('div');
704704
var liElement = elements.find('li');
705705

706-
divElement.triggerHandler({type: 'keypress', which: 32});
707-
liElement.triggerHandler({type: 'keypress', which: 32});
706+
divElement.triggerHandler({type: 'keydown', which: 32});
707+
liElement.triggerHandler({type: 'keydown', which: 32});
708708

709709
expect(clickFn).toHaveBeenCalledWith('div');
710710
expect(clickFn).toHaveBeenCalledWith('li');
711711
});
712712

713-
it('should not override existing ng-keypress', function() {
714-
scope.someOtherAction = function() {};
715-
var keypressFn = spyOn(scope, 'someOtherAction');
713+
it('should not bind to key events if there is existing ng-keydown', function() {
714+
scope.onClick = jasmine.createSpy('onClick');
715+
scope.onKeydown = jasmine.createSpy('onKeydown');
716716

717-
scope.someAction = function() {};
718-
clickFn = spyOn(scope, 'someAction');
719-
compileElement('<div ng-click="someAction()" ng-keypress="someOtherAction()" tabindex="0"></div>');
717+
var tmpl = '<div ng-click="onClick()" ng-keydown="onKeydown()" tabindex="0"></div>';
718+
compileElement(tmpl);
719+
720+
element.triggerHandler({type: 'keydown', keyCode: 32});
721+
722+
expect(scope.onKeydown).toHaveBeenCalled();
723+
expect(scope.onClick).not.toHaveBeenCalled();
724+
});
725+
726+
it('should not bind to key events if there is existing ng-keypress', function() {
727+
scope.onClick = jasmine.createSpy('onClick');
728+
scope.onKeypress = jasmine.createSpy('onKeypress');
729+
730+
var tmpl = '<div ng-click="onClick()" ng-keypress="onKeypress()" tabindex="0"></div>';
731+
compileElement(tmpl);
720732

721733
element.triggerHandler({type: 'keypress', keyCode: 32});
722734

723-
expect(clickFn).not.toHaveBeenCalled();
724-
expect(keypressFn).toHaveBeenCalled();
735+
expect(scope.onKeypress).toHaveBeenCalled();
736+
expect(scope.onClick).not.toHaveBeenCalled();
725737
});
726738

727-
it('should update bindings when keypress handled', function() {
739+
it('should not bind to key events if there is existing ng-keyup', function() {
740+
scope.onClick = jasmine.createSpy('onClick');
741+
scope.onKeyup = jasmine.createSpy('onKeyup');
742+
743+
var tmpl = '<div ng-click="onClick()" ng-keyup="onKeyup()" tabindex="0"></div>';
744+
compileElement(tmpl);
745+
746+
element.triggerHandler({type: 'keyup', keyCode: 32});
747+
748+
expect(scope.onKeyup).toHaveBeenCalled();
749+
expect(scope.onClick).not.toHaveBeenCalled();
750+
});
751+
752+
it('should update bindings when keydown is handled', function() {
728753
compileElement('<div ng-click="text = \'clicked!\'">{{text}}</div>');
729754
expect(element.text()).toBe('');
730755
spyOn(scope.$root, '$digest').and.callThrough();
731-
element.triggerHandler({ type: 'keypress', keyCode: 13 });
756+
element.triggerHandler({ type: 'keydown', keyCode: 13 });
732757
expect(element.text()).toBe('clicked!');
733758
expect(scope.$root.$digest).toHaveBeenCalledOnce();
734759
});
@@ -737,14 +762,14 @@ describe('$aria', function() {
737762
compileElement('<div ng-click="event = $event">{{event.type}}' +
738763
'{{event.keyCode}}</div>');
739764
expect(element.text()).toBe('');
740-
element.triggerHandler({ type: 'keypress', keyCode: 13 });
741-
expect(element.text()).toBe('keypress13');
765+
element.triggerHandler({ type: 'keydown', keyCode: 13 });
766+
expect(element.text()).toBe('keydown13');
742767
});
743768

744-
it('should not bind keypress to elements not in the default config', function() {
769+
it('should not bind keydown to natively interactive elements', function() {
745770
compileElement('<button ng-click="event = $event">{{event.type}}{{event.keyCode}}</button>');
746771
expect(element.text()).toBe('');
747-
element.triggerHandler({ type: 'keypress', keyCode: 13 });
772+
element.triggerHandler({ type: 'keydown', keyCode: 13 });
748773
expect(element.text()).toBe('');
749774
});
750775
});
@@ -761,21 +786,29 @@ describe('$aria', function() {
761786
});
762787
});
763788

764-
describe('actions when bindKeypress is set to false', function() {
789+
describe('actions when bindKeydown is set to false', function() {
765790
beforeEach(configAriaProvider({
766-
bindKeypress: false
791+
bindKeydown: false
767792
}));
768793
beforeEach(injectScopeAndCompiler);
769794

770-
it('should not a trigger click', function() {
771-
scope.someAction = function() {};
772-
var clickFn = spyOn(scope, 'someAction');
795+
it('should not trigger click', function() {
796+
scope.someAction = jasmine.createSpy('someAction');
773797

774798
element = $compile('<div ng-click="someAction()" tabindex="0"></div>')(scope);
775799

800+
element.triggerHandler({type: 'keydown', keyCode: 13});
801+
element.triggerHandler({type: 'keydown', keyCode: 32});
802+
element.triggerHandler({type: 'keypress', keyCode: 13});
776803
element.triggerHandler({type: 'keypress', keyCode: 32});
804+
element.triggerHandler({type: 'keyup', keyCode: 13});
805+
element.triggerHandler({type: 'keyup', keyCode: 32});
806+
807+
expect(scope.someAction).not.toHaveBeenCalled();
808+
809+
element.triggerHandler({type: 'click', keyCode: 32});
777810

778-
expect(clickFn).not.toHaveBeenCalled();
811+
expect(scope.someAction).toHaveBeenCalledOnce();
779812
});
780813
});
781814

0 commit comments

Comments
 (0)