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

fix(ngAria): check if element is contenteditable before blocking spac… #16762

Closed
wants to merge 3 commits into from
Closed

fix(ngAria): check if element is contenteditable before blocking spac… #16762

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f23fd9
fix(aria/ngClick): check if element is `contenteditable` before block…
Eblax Nov 16, 2018
628bd59
fixup! fix(aria/ngClick): check if element is `contenteditable` befor…
gkalpak Nov 26, 2018
2ed73c8
fixup! fix(aria/ngClick): check if element is `contenteditable` befor…
gkalpak Nov 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 53 additions & 20 deletions test/ngAria/ariaSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,59 @@ describe('$aria', function() {
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`',
function() {
compileElement(
'<section>' +
'<div ng-click="onClick($event)"></div>' +
'<ul><li ng-click="onClick($event)"></li></ul>' +
'</section>');

var divElement = element.find('div');
var liElement = element.find('li');

divElement.triggerHandler({type: 'keydown', which: 13});
liElement.triggerHandler({type: 'keydown', which: 13});
divElement.triggerHandler({type: 'keydown', which: 32});
liElement.triggerHandler({type: 'keydown', which: 32});

expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']);
}
);

it('should not prevent default keyboard action if the target element has editable content',
function() {
compileElement(
'<section>' +
'<div ng-click="onClick($event)" ng-attr-contenteditable="{{ edit }}"></div>' +
'<ul ng-click="onClick($event)"><li ng-attr-contenteditable="{{ edit }}"></li></ul>' +
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not also check static contenteditable attributes, as well as dynamically setting the attribute via ng-attr...? Or is this already covered elsewhere?

Copy link
Member

Choose a reason for hiding this comment

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

This is just an easy way to add/remove the attribute. Since we are not really checking the attribute itself (but rather checking the isContentEditable property which is automatically updated by the browser based on the attribute), I felt this is fine.

'</section>');

var divElement = element.find('div');
var ulElement = element.find('ul');
var liElement = element.find('li');

// Using `browserTrigger()`, because it supports event bubbling.

// Element are not editable yet.
browserTrigger(divElement, 'keydown', {bubbles: true, cancelable: true, keyCode: 13});
browserTrigger(ulElement, 'keydown', {bubbles: true, cancelable: true, keyCode: 32});
browserTrigger(liElement, 'keydown', {bubbles: true, cancelable: true, keyCode: 13});

expect(clickEvents).toEqual(['div(true)', 'ul(true)', 'li(true)']);

clickEvents = [];
scope.$apply('edit = true');

// Element are now editable.
browserTrigger(divElement, 'keydown', {bubbles: true, cancelable: true, keyCode: 32});
browserTrigger(ulElement, 'keydown', {bubbles: true, cancelable: true, keyCode: 13});
browserTrigger(liElement, 'keydown', {bubbles: true, cancelable: true, keyCode: 32});

expect(clickEvents).toEqual(['div(false)', 'ul(true)', 'li(false)']);
}
);

they('should not prevent default keyboard action if an interactive $type element' +
'is nested inside ng-click', nodeBlackList, function(elementType) {
function createHTML(type) {
Expand Down Expand Up @@ -981,26 +1034,6 @@ describe('$aria', function() {
}
);

it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`',
function() {
compileElement(
'<section>' +
'<div ng-click="onClick($event)"></div>' +
'<ul><li ng-click="onClick($event)"></li></ul>' +
'</section>');

var divElement = element.find('div');
var liElement = element.find('li');

divElement.triggerHandler({type: 'keydown', which: 13});
liElement.triggerHandler({type: 'keydown', which: 13});
divElement.triggerHandler({type: 'keydown', which: 32});
liElement.triggerHandler({type: 'keydown', which: 32});

expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']);
}
);

they('should not bind to key events if there is existing `ng-$prop`',
['keydown', 'keypress', 'keyup'], function(eventName) {
scope.onKeyEvent = jasmine.createSpy('onKeyEvent');
Expand Down