Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Geocoder widget auto-complete not visible at times #4916

Open
hahnd opened this issue Jan 24, 2017 · 8 comments · Fixed by #4931
Open

Geocoder widget auto-complete not visible at times #4916

hahnd opened this issue Jan 24, 2017 · 8 comments · Fixed by #4931

Comments

@hahnd
Copy link

hahnd commented Jan 24, 2017

Greetings,

I had just begun working to add a custom Geocoder service and saw your new feature added in 1.30. I downloaded the latest candidate today (Cesium-1.29.0-master.19674.zip) and tested with that.

Works great, except there is one case with Firefox where the auto-complete window is not visible when it should be. Here are the steps to reproduce:

  1. Click on search box
  2. Type search terms (auto-complete visible as expected)
  3. Hit enter
  4. Map position changes to searched location (as expected)
  5. Press Ctrl-A / Command-A (shortcut key to select all text)
  6. Type other search terms (auto-complete is NOT visible as expected)

A work around is to click the search text field again instead of Ctrl-A / Command-A as in Step 5. Then it will work as expected.

I was not able to reproduce this error in Safari or Chrome. I believe that the reason was that after Step 3, the search text field no longer had focus, and a mouse click was required before additional search terms could be entered.

Here's a demo:
https://www.youtube.com/watch?v=rctgl6pKxK0&feature=youtu.be

@hpinkos
Copy link
Contributor

hpinkos commented Jan 25, 2017

Thanks for reporting this @hahnd!

@erikmaarten
Copy link
Contributor

I'll look into this (probably not today, but later this week). Great bug report by the way! 👍

@hpinkos
Copy link
Contributor

hpinkos commented Jan 30, 2017

Thanks again for reporting this @hahnd! We just merged a fix and it will be included in the Cesium 1.30 release available on February 1st.

@hahnd
Copy link
Author

hahnd commented Feb 2, 2017

Thanks for the quick update and the very timely new feature!

I tested today using the 1.30 release, and indeed after pressing enter in the text field, it loses focus. However, if I press the tab key the text field will regain focus, and suggestions will not be shown while typing.

I would expect suggestions to be visible if the text field has focus and there are suggestions available. Do you agree with this being the expected behavior?

It seems that the suggestions are only visible if the text field has gained focus through a mouse click, but not by other methods (such as tab key). This behavior is consistent in all browsers tested (Firefox, Safari, and Chrome).

I added the line:
document.addEventListener('focus', this._onInputEnd, true);
to Widgets/Geocoder/Geocoder.js:146

This gives me the desired behavior in Firefox and Safari, but not Chrome. I'm not very familiar with knockout, so I'm not sure what might be the best way to do this. Any thoughts?

@hpinkos hpinkos reopened this Feb 2, 2017
@hahnd
Copy link
Author

hahnd commented Feb 2, 2017

I added the same line:
document.addEventListener('focus', this._onInputEnd, true);
to Widgets/Geocoder/Geocoder.js:142

Basically, this event listener is added in either case in the if statement, and now I get the same behavior in Chrome too.

@hpinkos
Copy link
Contributor

hpinkos commented Feb 2, 2017

Thanks @hahnd! Instead of adding another event listener, I think we can fix the problem by adding something like this to GeocoderViewModel

        this.focusTextbox = undefined;
        knockout.defineProperty(this, 'focusTextbox', {
            get : function() {
                return this._focusTextbox;
            },
            set: function(value) {
                if (value) {
                    showSuggestions();
                } else {
                   hideSuggestions();
                }
            }
        });

Then we can use focusTextbox instead of _focusTextbox for data binding in Geocoder.
We can remove the viewModel.hide/showSuggestions() calls in _onInputBegin/End because they will automatically be called whenever the value of focusTextbox changes. (We'd also want to change viewModel._focusTextbox to viewModel.focusTextbox in those two functions)
This should also automatically be called whenever the focus changes since focusTextbox is data-bound and the value will automatically update whenever the textbox focus changes.

If you have a chance to try this, it would be great if you would be interested in submitting a pull request with the fix! See our contributing documentation for information: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/CONTRIBUTING.md#opening-a-pull-request

@hahnd
Copy link
Author

hahnd commented Feb 2, 2017

I like this solution better, but when I tried it, I still only get calls to the property setter on events where the focus is gained or lost by a mouse click. The property setter does not get called when focus is gained or lost by pressing the tab key.

Any ideas why this might be happening? Today is the first day I've ever looked at knockout, so don't assume I know anything. :-)

@hpinkos
Copy link
Contributor

hpinkos commented Feb 3, 2017

Thanks for taking a stab at this @hahnd. I'm not sure why the property wouldn't be set when pressing the tab key, but I'll take a closer look at this next week to see if I can figure it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants