From 84f8de3605d7d2c6c997522f661a626d7aa98107 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Sat, 16 Jun 2018 12:54:09 +0300 Subject: [PATCH 1/3] refactor(ngAria): clean up `accessible actions` tests --- test/ngAria/ariaSpec.js | 144 ++++++++++++++++++---------------------- 1 file changed, 64 insertions(+), 80 deletions(-) diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 1970b01438b0..651be9999c51 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -922,116 +922,100 @@ describe('$aria', function() { }); describe('accessible actions', function() { - beforeEach(injectScopeAndCompiler); + var clickEvents; - var clickFn; + beforeEach(injectScopeAndCompiler); + beforeEach(function() { + clickEvents = []; + scope.onClick = jasmine.createSpy('onClick').and.callFake(function(evt) { + var nodeName = evt ? evt.target.nodeName.toLowerCase() : ''; + clickEvents.push(nodeName); + }); + }); it('should trigger a click from the keyboard', function() { - scope.someAction = function() {}; - - var elements = $compile('
' + - '
' + - '' + - '
')(scope); - - scope.$digest(); - - clickFn = spyOn(scope, 'someAction'); + compileElement( + '
' + + '
' + + '' + + '
'); - var divElement = elements.find('div'); - var liElement = elements.find('li'); + var divElement = element.find('div'); + var liElement = element.find('li'); + divElement.triggerHandler({type: 'keydown', keyCode: 13}); + liElement.triggerHandler({type: 'keydown', keyCode: 13}); divElement.triggerHandler({type: 'keydown', keyCode: 32}); liElement.triggerHandler({type: 'keydown', keyCode: 32}); - expect(clickFn).toHaveBeenCalledWith('div'); - expect(clickFn).toHaveBeenCalledWith('li'); + expect(clickEvents).toEqual(['div', 'li', 'div', 'li']); }); - it('should trigger a click in browsers that provide event.which instead of event.keyCode', function() { - scope.someAction = function() {}; + it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`', + function() { + compileElement( + '
' + + '
' + + '' + + '
'); + + var divElement = element.find('div'); + var liElement = element.find('li'); - var elements = $compile('
' + - '
' + - '' + - '
')(scope); + divElement.triggerHandler({type: 'keydown', which: 13}); + liElement.triggerHandler({type: 'keydown', which: 13}); + divElement.triggerHandler({type: 'keydown', which: 32}); + liElement.triggerHandler({type: 'keydown', which: 32}); - scope.$digest(); + expect(clickEvents).toEqual(['div', 'li', 'div', 'li']); + } + ); - clickFn = spyOn(scope, 'someAction'); + they('should not bind to key events if there is existing `ng-$prop`', + ['keydown', 'keypress', 'keyup'], function(eventName) { + scope.onKeyEvent = jasmine.createSpy('onKeyEvent'); + compileElement('
'); - var divElement = elements.find('div'); - var liElement = elements.find('li'); + element.triggerHandler({type: eventName, keyCode: 13}); + element.triggerHandler({type: eventName, keyCode: 32}); - divElement.triggerHandler({type: 'keydown', which: 32}); - liElement.triggerHandler({type: 'keydown', which: 32}); + expect(scope.onClick).not.toHaveBeenCalled(); + expect(scope.onKeyEvent).toHaveBeenCalledTimes(2); + } + ); - expect(clickFn).toHaveBeenCalledWith('div'); - expect(clickFn).toHaveBeenCalledWith('li'); - }); + it('should update bindings when keydown is handled', function() { + scope.count = 0; + compileElement('
Count: {{ count }}
'); - it('should not bind to key events if there is existing ng-keydown', function() { - scope.onClick = jasmine.createSpy('onClick'); - scope.onKeydown = jasmine.createSpy('onKeydown'); + expect(element.text()).toBe('Count: 0'); - var tmpl = '
'; - compileElement(tmpl); + element.triggerHandler({type: 'keydown', keyCode: 13}); + expect(element.text()).toBe('Count: 1'); element.triggerHandler({type: 'keydown', keyCode: 32}); - - expect(scope.onKeydown).toHaveBeenCalled(); - expect(scope.onClick).not.toHaveBeenCalled(); + expect(element.text()).toBe('Count: 2'); }); - it('should not bind to key events if there is existing ng-keypress', function() { - scope.onClick = jasmine.createSpy('onClick'); - scope.onKeypress = jasmine.createSpy('onKeypress'); - - var tmpl = '
'; - compileElement(tmpl); + it('should pass `$event` to `ng-click` handler as local', function() { + compileElement('
{{ event.type }}{{ event.keyCode }}
'); + expect(element.text()).toBe(''); - element.triggerHandler({type: 'keypress', keyCode: 32}); + element.triggerHandler({type: 'keydown', keyCode: 13}); + expect(element.text()).toBe('keydown13'); - expect(scope.onKeypress).toHaveBeenCalled(); - expect(scope.onClick).not.toHaveBeenCalled(); + element.triggerHandler({type: 'keydown', keyCode: 32}); + expect(element.text()).toBe('keydown32'); }); - it('should not bind to key events if there is existing ng-keyup', function() { - scope.onClick = jasmine.createSpy('onClick'); - scope.onKeyup = jasmine.createSpy('onKeyup'); - - var tmpl = '
'; - compileElement(tmpl); + it('should not bind keydown to natively interactive elements', function() { + compileElement(''); - element.triggerHandler({type: 'keyup', keyCode: 32}); + element.triggerHandler({type: 'keydown', keyCode: 13}); + element.triggerHandler({type: 'keydown', keyCode: 32}); - expect(scope.onKeyup).toHaveBeenCalled(); expect(scope.onClick).not.toHaveBeenCalled(); }); - - it('should update bindings when keydown is handled', function() { - compileElement('
{{text}}
'); - expect(element.text()).toBe(''); - spyOn(scope.$root, '$digest').and.callThrough(); - element.triggerHandler({ type: 'keydown', keyCode: 13 }); - expect(element.text()).toBe('clicked!'); - expect(scope.$root.$digest).toHaveBeenCalledOnce(); - }); - - it('should pass $event to ng-click handler as local', function() { - compileElement('
{{event.type}}' + - '{{event.keyCode}}
'); - expect(element.text()).toBe(''); - element.triggerHandler({ type: 'keydown', keyCode: 13 }); - expect(element.text()).toBe('keydown13'); - }); - - it('should not bind keydown to natively interactive elements', function() { - compileElement(''); - expect(element.text()).toBe(''); - element.triggerHandler({ type: 'keydown', keyCode: 13 }); - expect(element.text()).toBe(''); - }); }); describe('actions when bindRoleForClick is set to false', function() { From 898470823fba55da498eda1f7a72f21ab7004af1 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Sat, 16 Jun 2018 12:56:41 +0300 Subject: [PATCH 2/3] fix(ngAria): do not scroll when pressing spacebar on custom buttons By default, pressing spacebar causes the browser to scroll down. However, when a native button is focused, the button is clicked instead. `ngAria`'s `ngClick` directive, sets elements up to behave like buttons. For example, it adds `role="button"` and forwards `ENTER` and `SPACEBAR` keypresses to the `click` handler (to emulate the behavior of native buttons). Yet, pressing spacebar on such an element, still invokes the default browser behavior of scrolling down. This commit fixes this, by calling `preventDefault()` on the keyboard event, thus preventing the default scrolling behavior and making custom buttons behave closer to native ones. Closes #14665 --- src/ngAria/aria.js | 5 ++++- test/ngAria/ariaSpec.js | 9 +++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index 904d9a0e5959..73e9e0733f9f 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -387,7 +387,10 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { if ($aria.config('bindKeydown') && !attr.ngKeydown && !attr.ngKeypress && !attr.ngKeyup) { elem.on('keydown', function(event) { var keyCode = event.which || event.keyCode; - if (keyCode === 32 || keyCode === 13) { + + if (keyCode === 13 || keyCode === 32) { + // Prevent the default browser behavior (e.g. scrolling when pressing spacebar). + event.preventDefault(); scope.$apply(callback); } diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 651be9999c51..bc4704b40420 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -929,11 +929,12 @@ describe('$aria', function() { clickEvents = []; scope.onClick = jasmine.createSpy('onClick').and.callFake(function(evt) { var nodeName = evt ? evt.target.nodeName.toLowerCase() : ''; - clickEvents.push(nodeName); + var prevented = !!(evt && evt.defaultPrevented); + clickEvents.push(nodeName + '(' + prevented + ')'); }); }); - it('should trigger a click from the keyboard', function() { + it('should trigger a click from the keyboard (and prevent default action)', function() { compileElement( '
' + '
' + @@ -948,7 +949,7 @@ describe('$aria', function() { divElement.triggerHandler({type: 'keydown', keyCode: 32}); liElement.triggerHandler({type: 'keydown', keyCode: 32}); - expect(clickEvents).toEqual(['div', 'li', 'div', 'li']); + expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']); }); it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`', @@ -967,7 +968,7 @@ describe('$aria', function() { divElement.triggerHandler({type: 'keydown', which: 32}); liElement.triggerHandler({type: 'keydown', which: 32}); - expect(clickEvents).toEqual(['div', 'li', 'div', 'li']); + expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']); } ); From a0df567ee2bf5ef4cdf1c104ebfadeff1069c20d Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 18 Jun 2018 16:41:44 +0300 Subject: [PATCH 3/3] fixup! fix(ngAria): do not scroll when pressing spacebar on custom buttons --- test/ngAria/ariaSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index bc4704b40420..11130b17430e 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -929,7 +929,7 @@ describe('$aria', function() { clickEvents = []; scope.onClick = jasmine.createSpy('onClick').and.callFake(function(evt) { var nodeName = evt ? evt.target.nodeName.toLowerCase() : ''; - var prevented = !!(evt && evt.defaultPrevented); + var prevented = !!(evt && evt.isDefaultPrevented()); clickEvents.push(nodeName + '(' + prevented + ')'); }); });