Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

add input id to search <input> if present on ui-select directive as input-id #1220

Merged
merged 2 commits into from
Oct 3, 2015
Merged

add input id to search <input> if present on ui-select directive as input-id #1220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f87765
add input id to search <input> if present on ui-select directive as i…
Sep 30, 2015
7917e56
add unit tests (for multiple and single)
Oct 1, 2015
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
13 changes: 8 additions & 5 deletions src/uiSelectDirective.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@ uis.directive('uiSelect',

//Multiple or Single depending if multiple attribute presence
if (angular.isDefined(tAttrs.multiple))
tElement.append("<ui-select-multiple/>").removeAttr('multiple');
tElement.append('<ui-select-multiple/>').removeAttr('multiple');
else
tElement.append("<ui-select-single/>");
tElement.append('<ui-select-single/>');

if (tAttrs.inputId)
tElement.querySelectorAll('input.ui-select-search')[0].id = tAttrs.inputId;

return function(scope, element, attrs, ctrls, transcludeFn) {

Expand All @@ -43,7 +46,7 @@ uis.directive('uiSelect',

$select.onSelectCallback = $parse(attrs.onSelect);
$select.onRemoveCallback = $parse(attrs.onRemove);

//Limit the number of selections allowed
$select.limit = (angular.isDefined(attrs.limit)) ? parseInt(attrs.limit, 10) : undefined;

Expand All @@ -56,8 +59,8 @@ uis.directive('uiSelect',

if(attrs.tabindex){
attrs.$observe('tabindex', function(value) {
$select.focusInput.attr("tabindex", value);
element.removeAttr("tabindex");
$select.focusInput.attr('tabindex', value);
element.removeAttr('tabindex');
});
}

Expand Down
38 changes: 27 additions & 11 deletions test/select.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('ui-select tests', function() {
{ name: 'Nicole', email: '[email protected]', group: 'bar', age: 43 },
{ name: 'Natasha', email: '[email protected]', group: 'Baz', age: 54 }
];

scope.peopleObj = {
'1' : { name: 'Adam', email: '[email protected]', age: 12, country: 'United States' },
'2' : { name: 'Amalie', email: '[email protected]', age: 12, country: 'Argentina' },
Expand Down Expand Up @@ -127,6 +127,7 @@ describe('ui-select tests', function() {
if (attrs.title !== undefined) { attrsHtml += ' title="' + attrs.title + '"'; }
if (attrs.appendToBody !== undefined) { attrsHtml += ' append-to-body="' + attrs.appendToBody + '"'; }
if (attrs.allowClear !== undefined) { matchAttrsHtml += ' allow-clear="' + attrs.allowClear + '"';}
if (attrs.inputId !== undefined) { attrsHtml += ' input-id="' + attrs.inputId + '"'; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of inputId what about only id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't have two elements with the same id and I felt weird about removing the id from the ui-select because people might be using that to select their css rules.

}

return compileTemplate(
Expand Down Expand Up @@ -207,7 +208,7 @@ describe('ui-select tests', function() {
//uisRepeatParser

it('should parse simple repeat syntax', function() {

var locals = {};
locals.people = [{name: 'Wladimir'}, {name: 'Samantha'}];
locals.person = locals.people[0];
Expand All @@ -226,7 +227,7 @@ describe('ui-select tests', function() {
});

it('should parse simple repeat syntax', function() {

var locals = {};
locals.people = [{name: 'Wladimir'}, {name: 'Samantha'}];
locals.person = locals.people[0];
Expand All @@ -239,7 +240,7 @@ describe('ui-select tests', function() {
});

it('should parse simple property binding repeat syntax', function() {

var locals = {};
locals.people = [{name: 'Wladimir'}, {name: 'Samantha'}];
locals.person = locals.people[0];
Expand All @@ -252,7 +253,7 @@ describe('ui-select tests', function() {
});

it('should parse (key, value) repeat syntax', function() {

var locals = {};
locals.people = { 'WC' : {name: 'Wladimir'}, 'SH' : {name: 'Samantha'}};
locals.person = locals.people[0];
Expand All @@ -272,7 +273,7 @@ describe('ui-select tests', function() {
});

it('should parse simple property binding with (key, value) repeat syntax', function() {

var locals = {};
locals.people = { 'WC' : {name: 'Wladimir'}, 'SH' : {name: 'Samantha'}};
locals.person = locals.people['WC'];
Expand All @@ -286,7 +287,7 @@ describe('ui-select tests', function() {
});

it('should should accept a "collection expresion" only if its not (key, value) repeat syntax', function() {

var locals = {};
locals.people = { 'WC' : {name: 'Wladimir'}, 'SH' : {name: 'Samantha'}};
locals.person = locals.people['WC'];
Expand All @@ -299,7 +300,7 @@ describe('ui-select tests', function() {
});

it('should should throw if "collection expresion" used and (key, value) repeat syntax', function() {

var locals = {};
locals.people = { 'WC' : {name: 'Wladimir'}, 'SH' : {name: 'Samantha'}};
locals.person = locals.people['WC'];
Expand Down Expand Up @@ -339,7 +340,7 @@ describe('ui-select tests', function() {

expect(getMatchLabel(el)).toEqual('Adam');
});

it('should correctly render initial state with track by feature', function() {
var el = compileTemplate(
'<ui-select ng-model="selection.selected"> \
Expand Down Expand Up @@ -447,13 +448,13 @@ describe('ui-select tests', function() {
it('should toggle allow-clear directive', function() {
scope.selection.selected = scope.people[0];
scope.isClearAllowed = false;

var el = createUiSelect({theme : 'select2', allowClear: '{{isClearAllowed}}'});
var $select = el.scope().$select;

expect($select.allowClear).toEqual(false);
expect(el.find('.select2-search-choice-close').length).toEqual(0);

// Turn clear on
scope.isClearAllowed = true;
scope.$digest();
Expand Down Expand Up @@ -1506,6 +1507,7 @@ describe('ui-select tests', function() {
if (attrs.closeOnSelect !== undefined) { attrsHtml += ' close-on-select="' + attrs.closeOnSelect + '"'; }
if (attrs.tagging !== undefined) { attrsHtml += ' tagging="' + attrs.tagging + '"'; }
if (attrs.taggingTokens !== undefined) { attrsHtml += ' tagging-tokens="' + attrs.taggingTokens + '"'; }
if (attrs.inputId !== undefined) { attrsHtml += ' input-id="' + attrs.inputId + '"'; }
}

return compileTemplate(
Expand Down Expand Up @@ -2093,6 +2095,20 @@ describe('ui-select tests', function() {

expect($(el).scope().$select.selected.length).toBe(5);
});

it('should add an id to the search input field', function () {
var el = createUiSelectMultiple({inputId: 'inid'});
var searchEl = $(el).find('input.ui-select-search');
expect(searchEl.length).toEqual(1);
expect(searchEl[0].id).toEqual('inid');
});
});

it('should add an id to the search input field', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like same test is repeated twice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to test both single and multiple because they are very different beasts (even though they both have the same text input name right now). The it name is the same because the describe names will disambiguate them. I'm happy to remove one if you want.
I'd prefer to keep the multiple test because that's the one I care about most (and that search field is always on display so much more likely to be used for <label for="...">.

var el = createUiSelect({inputId: 'inid'});
var searchEl = $(el).find('input.ui-select-search');
expect(searchEl.length).toEqual(1);
expect(searchEl[0].id).toEqual('inid');
});

describe('default configuration via uiSelectConfig', function() {
Expand Down