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

fix(typeahead): close dropdown on tab with focus-first="false" #3340

Closed
wants to merge 1 commit into from
Closed

fix(typeahead): close dropdown on tab with focus-first="false" #3340

wants to merge 1 commit into from

Conversation

dlukez
Copy link
Contributor

@dlukez dlukez commented Feb 26, 2015

When an item isn't selected and you hit "tab", the dropdown stays open even though the focus moves to the second field. An alternative solution might handle this using the 'blur' event. Happy to discuss this.

See this plnkr: http://plnkr.co/edit/zR0HYGVSlMdLgRvCEIZv?p=preview

@fernandoneira
Copy link

This is good stuff. I'd actually add the same behavior for the Enter key press, since it is basically the same issue in my opinion, i.e. the user has no intent in modifying the input anymore, either because he changed to a different input by pressing Tab or because he wants to submit the form by pressing Enter.

Either you can add this or I'll do a pull request with that change later on if my opinion is not shared.

@@ -275,7 +275,14 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
}

// 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)) {
if (scope.activeIdx == -1 && evt.which === 13) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use ===.

@wesleycho wesleycho modified the milestones: 0.13.1, Backlog Apr 5, 2015
@wesleycho
Copy link
Contributor

This is mostly there, just fix those two things up :) .

@dlukez
Copy link
Contributor Author

dlukez commented Apr 7, 2015

@wesleycho I deliberately left the == comparisons as they were. I'm not sure of the ramifications of changing them... Perhaps somewhere they are being set as strings and the coercion is necessary? I don't think they should be changed as a part of this PR, since this PR is concerned with fixing the issue mentioned above rather than refactoring existing code which will carry more risk of introducing a bug.

@dlukez
Copy link
Contributor Author

dlukez commented Apr 7, 2015

@fernandoneira I agree with you, however the comment suggested this was by design. If others are in support, I'd be happy to make it work like this for the "enter" keypress too.

@wesleycho
Copy link
Contributor

I will be merging this shortly, along with the change to === - I did an investigation and it looks like it is only being set to integers, so this should be safe.

@wesleycho wesleycho closed this in 493510d Jul 19, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants