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

Commit 89538d6

Browse files
topherfangiommalerba
authored andcommitted
fix(select): Allow non-english characters for keyboard selection. (#8893)
Our previous method for checking whether or not the typed character matched the first letter of an `<md-option>` was constrained to the letters `[a-zA-Z]`. Fix by allowing more characters and checking the validity via the `String.fromCharCode()` method. Add tests for Spanish and Simplified Chinese, as well as not swallowing certain keys like function keys (F3), meta, ctrl and comma (`,`). Fixes #7730.
1 parent f0facb2 commit 89538d6

File tree

3 files changed

+78
-6
lines changed

3 files changed

+78
-6
lines changed

src/components/select/select.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ function SelectDirective($mdSelect, $mdUtil, $mdConstant, $mdTheming, $mdAria, $
567567
e.preventDefault();
568568
openSelect(e);
569569
} else {
570-
if ($mdConstant.isInputKey(e) || $mdConstant.isNumPadKey(e)) {
570+
if (shouldHandleKey(e, $mdConstant)) {
571571
e.preventDefault();
572572

573573
var node = selectMenuCtrl.optNodeForKeyboardSearch(e);
@@ -1396,7 +1396,7 @@ function SelectProvider($$interimElementProvider) {
13961396
$mdUtil.nextTick($mdSelect.hide, true);
13971397
break;
13981398
default:
1399-
if ($mdConstant.isInputKey(ev) || $mdConstant.isNumPadKey(ev)) {
1399+
if (shouldHandleKey(ev, $mdConstant)) {
14001400
var optNode = dropDown.controller('mdSelectMenu').optNodeForKeyboardSearch(ev);
14011401
opts.focusedNode = optNode || opts.focusedNode;
14021402
optNode && optNode.focus();
@@ -1673,4 +1673,13 @@ function SelectProvider($$interimElementProvider) {
16731673
}
16741674
return isScrollable;
16751675
}
1676+
1677+
}
1678+
1679+
function shouldHandleKey(ev, $mdConstant) {
1680+
var char = String.fromCharCode(ev.keyCode);
1681+
var isNonUsefulKey = (ev.keyCode <= 31);
1682+
1683+
return (char && char.length && !isNonUsefulKey &&
1684+
!$mdConstant.isMetaKey(ev) && !$mdConstant.isFnLockKey(ev) && !$mdConstant.hasModifierKey(ev));
16761685
}

src/components/select/select.spec.js

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,6 +1294,62 @@ describe('<md-select>', function() {
12941294
expect($rootScope.someModel).toBe(2);
12951295
});
12961296

1297+
it('supports typing non-english option names', inject(function($document, $rootScope) {
1298+
var words = ['algebra', 'álgebra'];
1299+
var el = setupSelect('ng-model="someModel"', words).find('md-select');
1300+
1301+
pressKey(el, words[1].charCodeAt(0));
1302+
expect($rootScope.someModel).toBe(words[1]);
1303+
}));
1304+
1305+
it('supports typing unicode option names', inject(function($document, $rootScope) {
1306+
var words = ['algebra', '太阳'];
1307+
var el = setupSelect('ng-model="someModel"', words).find('md-select');
1308+
1309+
pressKey(el, words[1].charCodeAt(0));
1310+
expect($rootScope.someModel).toBe(words[1]);
1311+
}));
1312+
1313+
// Note, this test is designed to check the shouldHandleKey() method which is the default
1314+
// method if the keypress doesn't match one of the KNOWN keys such as up/down/enter/escape/etc.
1315+
it('does not swallow useful keys (fn, arrow, etc)', function() {
1316+
var keyCodes = [17, 92, 113]; // ctrl, comma (`,`), and F3
1317+
var customEvent = {
1318+
type: 'keydown',
1319+
preventDefault: jasmine.createSpy('preventDefault')
1320+
};
1321+
1322+
var words = ['algebra', 'math', 'science'];
1323+
var el = setupSelect('ng-model="someModel"', words).find('md-select');
1324+
1325+
keyCodes.forEach(function(code) {
1326+
customEvent.keyCode = code;
1327+
pressKey(el, null, customEvent);
1328+
expect(customEvent.preventDefault).not.toHaveBeenCalled();
1329+
});
1330+
});
1331+
1332+
it('does not swallow modifier keys', function() {
1333+
var customEvent = {
1334+
type: 'keydown',
1335+
preventDefault: jasmine.createSpy('preventDefault')
1336+
};
1337+
1338+
var words = ['algebra', 'math', 'science'];
1339+
var el = setupSelect('ng-model="someModel"', words).find('md-select');
1340+
1341+
customEvent.keyCode = 70;
1342+
customEvent.ctrlKey = true;
1343+
pressKey(el, null, customEvent);
1344+
expect(customEvent.preventDefault).not.toHaveBeenCalled();
1345+
1346+
customEvent.keyCode = 82;
1347+
customEvent.ctrlKey = false;
1348+
customEvent.metaKey = true;
1349+
pressKey(el, null, customEvent);
1350+
expect(customEvent.preventDefault).not.toHaveBeenCalled();
1351+
});
1352+
12971353
it('disallows selection of disabled options', function() {
12981354
var optsTemplate =
12991355
'<md-option value="1">1</md-option>' +
@@ -1385,11 +1441,13 @@ describe('<md-select>', function() {
13851441
}
13861442

13871443

1388-
function pressKey(el, code) {
1389-
el.triggerHandler({
1444+
function pressKey(el, code, customEvent) {
1445+
var event = customEvent || {
13901446
type: 'keydown',
13911447
keyCode: code
1392-
});
1448+
};
1449+
1450+
el.triggerHandler(event);
13931451
}
13941452

13951453
function clickOption(select, index) {

src/core/util/constant.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,16 @@ function MdConstantFactory() {
4646

4747
var self = {
4848
isInputKey : function(e) { return (e.keyCode >= 31 && e.keyCode <= 90); },
49-
isNumPadKey : function (e){ return (3 === e.location && e.keyCode >= 97 && e.keyCode <= 105); },
49+
isNumPadKey : function(e) { return (3 === e.location && e.keyCode >= 97 && e.keyCode <= 105); },
50+
isMetaKey: function(e) { return (e.keyCode >= 91 && e.keyCode <= 93); },
51+
isFnLockKey: function(e) { return (e.keyCode >= 112 && e.keyCode <= 145); },
5052
isNavigationKey : function(e) {
5153
var kc = self.KEY_CODE, NAVIGATION_KEYS = [kc.SPACE, kc.ENTER, kc.UP_ARROW, kc.DOWN_ARROW];
5254
return (NAVIGATION_KEYS.indexOf(e.keyCode) != -1);
5355
},
56+
hasModifierKey: function(e) {
57+
return e.ctrlKey || e.metaKey || e.altKey;
58+
},
5459

5560
/**
5661
* Maximum size, in pixels, that can be explicitly set to an element. The actual value varies

0 commit comments

Comments
 (0)