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

feat(ngAria[ngClick]): anchor should treat keypress as click #13996

Closed
wants to merge 8 commits into from
21 changes: 21 additions & 0 deletions src/ngAria/aria.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,27 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
});
}
}

if ($aria.config('bindKeypress') && isNodeOneOf(elem, ['A', 'BUTTON'])) {
Copy link
Member

Choose a reason for hiding this comment

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

Actually, we don't want to do anything with buttons, just as. The browser handles pressing both ENTER and SPACE on buttons.

Copy link
Member

Choose a reason for hiding this comment

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

You should also check that attr.ngKeypress is falsy.

elem.on('keypress', function(event) {
var keyCode = event.which || event.keyCode;

if (elem[0].nodeName === 'A') {
var hasHref = elem.attr('href') != null && elem.attr('href') !== '';
Copy link
Member

Choose a reason for hiding this comment

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

You just have to check if elem.attr('href') is falsy.

BTW, if we need to also support SVG, then we need to check xlink:href (instead of href).
(You can take a look at the htmlAnchorDirective in the source code for more details.)

if ((keyCode === 13 && !hasHref) || keyCode === 32) {
Copy link
Member

Choose a reason for hiding this comment

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

The browser handles ENTER for anchor. We just need to handle SPACE (and I would still check that it has no href).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ENTER doesn't seem to be handled for anchors when href is falsy. I think we need to trigger the click when ENTER and no href. The SPACE is not handled regardless of the href.

scope.$apply(callback);
}
} else if (elem[0].nodeName === 'BUTTON') {
if (keyCode === 32) {
scope.$apply(callback);
}
}

function callback() {
fn(scope, { $event: event });
}
});
}
};
}
};
Expand Down
29 changes: 29 additions & 0 deletions test/ngAria/ariaSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,35 @@ describe('$aria', function() {
element.triggerHandler({ type: 'keypress', keyCode: 13 });
expect(element.text()).toBe('');
});

it('should bind keypress to anchor elements without href defined', function() {
compileElement('<a ng-click="event = $event">{{event.type}}{{event.keyCode}}</a>');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why wouldn't you use a button element for this? I don't understand the use case.

expect(element.text()).toBe('');
element.triggerHandler({ type: 'keypress', keyCode: 13 });
expect(element.text()).toBe('keypress13');
});

it('should bind keypress to anchor elements with empty href defined', function() {
compileElement('<a href="" ng-click="event = $event">{{event.type}}{{event.keyCode}}</a>');
expect(element.text()).toBe('');
element.triggerHandler({ type: 'keypress', keyCode: 13 });
expect(element.text()).toBe('keypress13');
});

it('should not bind keypress to anchor elements with href defined', function() {
compileElement('<a href="http://somwehere" ng-click="event = $event">{{event.type}}{{event.keyCode}}</a>');
expect(element.text()).toBe('');
element.triggerHandler({ type: 'keypress', keyCode: 13 });
expect(element.text()).toBe('');
});

it('should bind space keypress to button elements', function() {
compileElement('<button ng-click="event = $event">{{event.type}}{{event.keyCode}}</button>');
expect(element.text()).toBe('');
element.triggerHandler({ type: 'keypress', keyCode: 32 });
expect(element.text()).toBe('keypress32');
});

});

describe('actions when bindRoleForClick is set to false', function() {
Expand Down