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

Commit

Permalink
fix(ngAria): do not scroll when pressing spacebar on custom buttons
Browse files Browse the repository at this point in the history
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

Closes #16604
  • Loading branch information
gkalpak committed Jun 18, 2018
1 parent af1e6a3 commit 6c224a2
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
5 changes: 4 additions & 1 deletion src/ngAria/aria.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
9 changes: 5 additions & 4 deletions test/ngAria/ariaSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.isDefaultPrevented());
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(
'<section>' +
'<div ng-click="onClick($event)"></div>' +
Expand All @@ -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`',
Expand All @@ -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)']);
}
);

Expand Down

4 comments on commit 6c224a2

@Sergi0Limit
Copy link

@Sergi0Limit Sergi0Limit commented on 6c224a2 Aug 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try use ngclick with input wrapped in the div!!!
<div ng-click="asd()"><input type="text" ng-model="m.text" placeholder="try wrote spacebar there"/></div>
when input/textarea in the wrapped tag with ng-click i can't wrote spacebars in the text

https://codepen.io/sergi0limit/pen/mjYLGE

@levicot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem that @Sergi0Limit, please rollback this change

@Sergi0Limit
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@levicot
See #16664 for details

@levicot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sergi0Limit thanks

Please sign in to comment.