Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

typeahead-focus-first=false option to prevent first match from being focused #2916

Closed
wants to merge 1 commit into from
+108 −4
Closed

typeahead-focus-first=false option to prevent first match from being focused #2916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/typeahead/docs/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,7 @@ The typeahead directives provide several attributes:
* `typeahead-wait-ms` <i class="glyphicon glyphicon-eye-open"></i>
_(Defaults: 0)_ :
Minimal wait time after last character typed before typeahead kicks-in

* `typeahead-focus-first` <i class="glyphicon glyphicon-eye-open"></i>
_(Defaults: true)_ :
Should the first match automatically be focused?
97 changes: 95 additions & 2 deletions src/typeahead/test/typeahead.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ describe('typeahead tests', function () {
this.message = function () {
return 'Expected "' + this.actual + '" to be opened.';
};
return typeaheadEl.length === 1 && typeaheadEl.hasClass('ng-hide') === false && liEls.length === noOfMatches && $(liEls[activeIdx]).hasClass('active');

return (typeaheadEl.length === 1 &&
typeaheadEl.hasClass('ng-hide') === false &&
liEls.length === noOfMatches &&
(activeIdx === -1 ? !$(liEls).hasClass('active') : $(liEls[activeIdx]).hasClass('active'))
);
}
});
});
Expand Down Expand Up @@ -409,7 +414,7 @@ describe('typeahead tests', function () {
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 1);

// Up arrow key goes back to last element
// Up arrow key goes back to first element
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 0);
});
Expand Down Expand Up @@ -670,4 +675,92 @@ describe('typeahead tests', function () {
});
});

describe('focus first', function () {
it('should focus the first element by default', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a test for when pressing enter or tab when activeIdx == -1, nothing should happen and nothing should be selected.

var element = prepareInputEl('<div><input ng-model="result" typeahead="item for item in source | filter:$viewValue"></div>');
changeInputValueTo(element, 'b');
expect(element).toBeOpenWithActive(2, 0);

// Down arrow key
triggerKeyDown(element, 40);
expect(element).toBeOpenWithActive(2, 1);

// Down arrow key goes back to first element
triggerKeyDown(element, 40);
expect(element).toBeOpenWithActive(2, 0);

// Up arrow key goes back to last element
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 1);

// Up arrow key goes back to first element
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 0);
});

it('should not focus the first element until keys are pressed', function () {
var element = prepareInputEl('<div><input ng-model="result" typeahead="item for item in source | filter:$viewValue" typeahead-focus-first="false"></div>');
changeInputValueTo(element, 'b');
expect(element).toBeOpenWithActive(2, -1);

// Down arrow key goes to first element
triggerKeyDown(element, 40);
expect(element).toBeOpenWithActive(2, 0);

// Down arrow key goes to second element
triggerKeyDown(element, 40);
expect(element).toBeOpenWithActive(2, 1);

// Down arrow key goes back to first element
triggerKeyDown(element, 40);
expect(element).toBeOpenWithActive(2, 0);

// Up arrow key goes back to last element
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 1);

// Up arrow key goes back to first element
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 0);

// New input goes back to no focus
changeInputValueTo(element, 'a');
changeInputValueTo(element, 'b');
expect(element).toBeOpenWithActive(2, -1);

// Up arrow key goes to last element
triggerKeyDown(element, 38);
expect(element).toBeOpenWithActive(2, 1);
});
});

it('should not capture enter or tab until an item is focused', function () {
$scope.select_count = 0;
$scope.onSelect = function ($item, $model, $label) {
$scope.select_count = $scope.select_count + 1;
};
var element = prepareInputEl('<div><input ng-model="result" ng-keydown="keyDownEvent = $event" typeahead="item for item in source | filter:$viewValue" typeahead-on-select="onSelect($item, $model, $label)" typeahead-focus-first="false"></div>');
changeInputValueTo(element, 'b');

// enter key should not be captured when nothing is focused
triggerKeyDown(element, 13);
expect($scope.keyDownEvent.isDefaultPrevented()).toBeFalsy();
expect($scope.select_count).toEqual(0);

// tab key should not be captured when nothing is focused
triggerKeyDown(element, 9);
expect($scope.keyDownEvent.isDefaultPrevented()).toBeFalsy();
expect($scope.select_count).toEqual(0);

// down key should be captured and focus first element
triggerKeyDown(element, 40);
expect($scope.keyDownEvent.isDefaultPrevented()).toBeTruthy();
expect(element).toBeOpenWithActive(2, 0);

// enter key should be captured now that something is focused
triggerKeyDown(element, 13);
expect($scope.keyDownEvent.isDefaultPrevented()).toBeTruthy();
expect($scope.select_count).toEqual(1);
});

});
11 changes: 9 additions & 2 deletions src/typeahead/typeahead.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap

var appendToBody = attrs.typeaheadAppendToBody ? originalScope.$eval(attrs.typeaheadAppendToBody) : false;

var focusFirst = originalScope.$eval(attrs.typeaheadFocusFirst) !== false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's safer and more consistent to do same check as above (on line 60, to check whether the attrs expression is truthy/not empty) as that would work with any falsey value.

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 copied this from isEditable on line 50 which is also default-true. Line 60 for appendToBody is default-false. If you prefer different style for default-true attributes, I would suggest updating both isEditable and focusFirst, but I think that's outside of the scope of this pull request.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, in that case this is fine.


//INTERNAL VARIABLES

//model setter executed upon match selection
Expand Down Expand Up @@ -131,7 +133,7 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
if (onCurrentRequest && hasFocus) {
if (matches.length > 0) {

scope.activeIdx = 0;
scope.activeIdx = focusFirst ? 0 : -1;
scope.matches.length = 0;

//transform labels
Expand Down Expand Up @@ -272,14 +274,19 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
return;
}

// if there's nothing selected (i.e. focusFirst) and enter is hit, don't do anything
if (scope.activeIdx == -1 && (evt.which === 13 || evt.which === 9)) {
return;
}

evt.preventDefault();

if (evt.which === 40) {
scope.activeIdx = (scope.activeIdx + 1) % scope.matches.length;
scope.$digest();

} else if (evt.which === 38) {
scope.activeIdx = (scope.activeIdx ? scope.activeIdx : scope.matches.length) - 1;
scope.activeIdx = (scope.activeIdx > 0 ? scope.activeIdx : scope.matches.length) - 1;
scope.$digest();

} else if (evt.which === 13 || evt.which === 9) {
Expand Down