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

Update typeahead to fix #1773 #1793

Closed
wants to merge 3 commits into from

Conversation

shayanelhami
Copy link
Contributor

Used timeout to avoid $rootScope:inprog error

Used timeout to avoid $rootScope:inprog error
@marcghorayeb
Copy link

https://github.com/angular-ui/bootstrap/pull/1744/files
seems i'm not the only one with the issue ;) IE8 is a pain :o

@shayanelhami
Copy link
Contributor Author

Not really! :) it's the way angularjs cycles are designed.

@@ -222,7 +222,8 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
resetMatches();

//return focus to the input element if a mach was selected via a mouse click event
element[0].focus();
// use timeout to avoid $rootScope:inprog error
setTimeout(function() { element[0].focus(); }, 1);
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 using setTimeout, use $timeout with false for invokeApply.

@chrisirhc
Copy link
Contributor

This fix looks good to me, except for the change I highlighted to use $timeout instead.
The idea is to call focus asynchronously and call it out of a digest/apply cycle.

@@ -222,7 +222,8 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
resetMatches();

//return focus to the input element if a mach was selected via a mouse click event
element[0].focus();
// use timeout to avoid $rootScope:inprog error
$timeout(function() { element[0].focus(); }, 1, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does $timeout(function() { element[0].focus(); }, 0, false); work? Was there a reason for picking the values 1 and true (invokeApply = true)?

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 checked and it does work with 0 and false (see http://plnkr.co/FimbQhGviK5dDVarLaRy). I'll give you another commit.

@chrisirhc chrisirhc closed this in d002493 Mar 17, 2014
@chrisirhc
Copy link
Contributor

Thank you, @shayanelhami ! I've added a test and merged it.

@getuliojr
Copy link

Your solution is great @chrisirhc, just tested and it is working. Good to know it will be merged.

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