Select Geocoder text on focus. Fixes #4268. #4464
Conversation
| @@ -70,6 +70,11 @@ value: searchText,\ | |||
| valueUpdate: "afterkeydown",\ | |||
| disable: isSearchInProgress,\ | |||
| css: { "cesium-geocoder-input-wide" : keepExpanded || searchText.length > 0 }'); | |||
| textBox.addEventListener('focus', function() { | |||
| setTimeout(function() { | |||
There was a problem hiding this comment.
What's the purpose of the setTimeout?
There was a problem hiding this comment.
JavaScript cargo-culting :) This StackOverflow answer says "In Chrome the selection via .select() doesn't stick - adding a slight timeout resolves this issue.".
There was a problem hiding this comment.
I just tried the latest version of Chrome/Firefox/IE and everything worked fine without the timeout, so we can remove it to start and add it back if it's ever needed.
The general strategy for Cesium development is to target the latest version of the 5 major browsers (especially since 5 of them are evergreen) and only extra code for older browsers if its really needed and has wide enough use (and we can actually replicate the broken behavior we are fixing first).
There was a problem hiding this comment.
I just tried on the latest version of Safari (10.0 (12602.1.50.0.10)), and it seems that setTimeout is needed.
There was a problem hiding this comment.
Sounds good, thanks for checking. Can you add a comment to the code indicating the setTimeout is needed for Safari, this way someone doesn't accidentally remove it in the future. Thanks
|
Thanks @twpayne! The functionality is great. My only other comment is that we should call |
Done. |
| @@ -164,6 +175,7 @@ cesiumSvgPath: { path: isSearchInProgress ? _stopSearchPath : _startSearchPath, | |||
|
|
|||
| knockout.cleanNode(this._form); | |||
| this._container.removeChild(this._form); | |||
| this._textBox.removeEventListener(this._onTextBoxFocus, false); | |||
There was a problem hiding this comment.
I believe this should be this._textBox.removeEventListener('focus', this._onTextBoxFocus, false);
There was a problem hiding this comment.
Doh! Good catch. Sorry. Fixed.
|
Perfect, thanks @twpayne! |
|
I'll update |
cc @hpinkos
Slightly different code might be required for older iOS devices, see discussion on StackOverflow.