ngAutocomplete icon indicating copy to clipboard operation
ngAutocomplete copied to clipboard

Changed watch enter to not got to server

Open leobispo opened this issue 11 years ago • 7 comments

  • Simulating an arrow down and enter on the listbox, so the app don't need to do one extra server call to get the information.

    This change will also fix the lost focus issue, where the AutoComplete is reverting the changes when user press enter.

leobispo avatar Apr 16 '14 14:04 leobispo

First, can you tell me if the issue you are talking about is fixed in the example here? http://plnkr.co/edit/GF3nM3XfYX9El2w11pGo?p=preview I've added jQuery to the dependencies, I've been struggling with an issue where the autocomplete result isn't updating correctly, this fixes the issue I've been seeing, but I'm wondering if you are referring to the same one.

To see the issue I'm talking about, take out jQuery from the example I linked, then type in "a" to the autocomplete, press enter, then type in "s", and click on one of the answers, it won't update properly. I'm not sure why having jQuery as a dependency fixes this but it seems to work.

Originally I was going to implement the enter functionality the way you have, with it simulating the down arrows and enter keys, but I wanted to implement it in a way that doesn't break if Google changes the autocomplete's CSS layout, and I'm a bit wary of simulating keys. The current implementation relies on an extra server call, but it uses Google's public API's which are less likely to change, and doesn't rely on the CSS layout.

So it's a bit of a tradeoff, more robust <-----> less server calls

I'm really interested in hearing more of your thoughts on this, and I really appreciate the PR's.

wpalahnuk avatar Apr 16 '14 18:04 wpalahnuk

I was planning to use your component without any changes. The problem is that I found some bugs in my use cases :(.

I am using the watchEnter functionality and whenever the component loose the focus, the text is returning to the state when I hit ENTER. I debug the code and I found that the lostfocus event that you bound to the element was occurring before the lostfocus event of google's component. So In my case, this workaround was not working.

Another problem is, since you are using the PlaceService API, the result returned by this API is not the same as the first element from the AutoComplete list. In my case (Germany), I was searching for Alexander Platz using (al) and when I hit ENTER it returned me Germany (and it should return Alexander Platz, Germany).

With the changes that I did, both bugs are gone.

IMO we shouldn't care about changes on Googles's API. We can always set which API version we want to use. So we must provide in the documentation which google's API version we tested with.

PS: Browser Tested - Chrome, Safari and Firefox

leobispo avatar Apr 16 '14 19:04 leobispo

I am using the watchEnter functionality and whenever the component loose the focus, the text is returning to the state when I hit ENTER. I debug the code and I found that the lostfocus event that you bound to the element was occurring before the lostfocus event of google's component. So In my case, this workaround was not working.

This is concerning, could you link your use case or put up an example of this? I'd really like to look more closely at this.

IMO we shouldn't care about changes on Googles's API. We can always set which API version we want to use. So we must provide in the documentation which google's API version we tested with.

It's not the API I'm worried about changing, but rather the non API things like the autocomplete's HTML/CSS layout, which the directive now depends on because of the $(".pac-item-selected") part of the code. Google is expecting you to utilize it's public APIs but it is not expecting you to use autocomplete's HTML/CSS layout.

Currently the directive only utilizes the public APIs, this change would also make it reliant on HTML/CSS layout of the autocomplete widget, which might be subject to undocumented change.

Another problem is, since you are using the PlaceService API, the result returned by this API is not the same as the first element from the AutoComplete list. In my case (Germany), I was searching for Alexander Platz using (al) and when I hit ENTER it returned me Germany (and it should return Alexander Platz, Germany).

This is definitely an issue, I was under the impression they returned the same results, but apparently not always.

wpalahnuk avatar Apr 16 '14 20:04 wpalahnuk

Just try:

http://plnkr.co/edit/GF3nM3XfYX9El2w11pGo?p=preview

al (ENTER) (TAB)

Tested on: Chrome and Firefox

leobispo avatar Apr 16 '14 20:04 leobispo

Ok, I see what you mean. I need to look at this some more.

wpalahnuk avatar Apr 16 '14 20:04 wpalahnuk

NP. Take your Time. I think the check $(".pac-item-selected").length is not needed.

I will do some tests and update the PR.

leobispo avatar Apr 16 '14 20:04 leobispo

So,

What is the status of this PR?

leobispo avatar Apr 26 '14 14:04 leobispo