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

feat(typeahead): add event object to on select callback #5165

Closed
wants to merge 2 commits into from

Conversation

deeg
Copy link
Contributor

@deeg deeg commented Jan 7, 2016

It would be nice to have access to the event object in the typeahead on select callback.

@@ -202,7 +202,7 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.debounce', 'ui.bootstrap
return false;
};

var getMatchesAsync = function(inputValue) {
var getMatchesAsync = function(inputValue, evt) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some calls to this function which do not seem to have an event object, such as when the typeahead directive inits.

Is it okay for the event object to be undefined in these cases? Any other suggestions in order to always have an event object?

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 is fine - only thing is to perhaps add a note in the documentation that this can be the case.

@wesleycho
Copy link
Contributor

This LGTM, just need that one documentation update and this should be good.

@wesleycho wesleycho added this to the 1.0.0 milestone Jan 8, 2016
@deeg
Copy link
Contributor Author

deeg commented Jan 8, 2016

@wesleycho, thanks just updated the documentation.

Just curious, how often do you release a new version? Do you see this getting added to a new stable version soon?

I noticed some errors happening with typeahead in master, before this change was put in. Should I open a bug for it, or are things in master not yet released still under active development?

@wesleycho
Copy link
Contributor

If there is something wrong, open an issue with reproduction - we may be releasing tomorrow or Saturday, so if there is a real issue, the earlier we can see what is going on, the better

@deeg
Copy link
Contributor Author

deeg commented Jan 8, 2016

@wesleycho, sounds good thanks.

Created issue for it: #5167

@wesleycho wesleycho closed this in 3e876b8 Jan 8, 2016
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.

2 participants