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.

Sign up for GitHub

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

Add optional custom geocoder callback function #6915

Merged
merged 5 commits into from
Aug 14, 2018
Merged

Add optional custom geocoder callback function #6915

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c0b9c77
Add optional custom geocoder command
Aug 13, 2018
d382b44
Merge branch 'master' into geocoder-command
Aug 14, 2018
3860234
CHANGES.md
Aug 14, 2018
1feacbd
geocoder command -> function
Aug 14, 2018
32bd0c4
doc
Aug 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Source/Widgets/Geocoder/Geocoder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ define([
* @param {GeocoderService[]} [options.geocoderServices] The geocoder services to be used
* @param {Boolean} [options.autoComplete = true] True if the geocoder should query as the user types to autocomplete
* @param {Number} [options.flightDuration=1.5] The duration of the camera flight to an entered location, in seconds.
* @param {Geocoder~DestinationFoundFunction} [options.destinationFound] A callback function that is called after a successful geocode. If not supplied, the default behavior is to fly the camera to the result destination.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused about the use case here. If a user needs to geocode and then do something with the results, they should just be calling the underlying Geocoder service directly. Why would they be using the Geocoder widget if they didn't want to fly to the destination (which is the main point of the widget)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's useful to be able to use the UI without necessarily imposing the flyto behavior. For example, say I wanted to use the search box to add billboards at certain addresses on the globe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps expose GeocoderViewModel.flyToDestination as a documented function and assign it as the default value here so that it's more obvious that changing it to something else will remove that functionality? I think that would work better than trying to explain it in that last sentence.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, does it make sense for this to be a Command? It's not called through the knockout bindings right? If that's the case a simple function callback makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it be passed in as a Command? I use that callback function to create a command: https://github.com/AnalyticalGraphicsInc/cesium/blob/386023433ddb1879cef4ef1eb89e0a7759d4be4b/Source/Widgets/Geocoder/GeocoderViewModel.js#L175-L179

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, what I meant was does it make sense for this.destinationFoundCommand to be a command at all? Why not just have it stay a callback function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh gotcha. Yeah, I'll change it

*/
function Geocoder(options) {
//>>includeStart('debug', pragmas.debug);
Expand Down Expand Up @@ -210,5 +211,12 @@ css: { active: $data === $parent._selectedSuggestion }');
return destroyObject(this);
};

/**
* A function that handles the result of a successful geocode.
* @callback Geocoder~DestinationFoundFunction
* @param {GeocoderViewModel} viewModel The view model.
* @param {Cartesian3|Rectangle} destination The destination result of the geocode.
*/

return Geocoder;
});
26 changes: 18 additions & 8 deletions Source/Widgets/Geocoder/GeocoderViewModel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ define([
* If more than one are supplied, suggestions will be gathered for the geocoders that support it,
* and if no suggestion is selected the result from the first geocoder service wil be used.
* @param {Number} [options.flightDuration] The duration of the camera flight to an entered location, in seconds.
* @param {Geocoder~DestinationFoundFunction} [options.destinationFound] A callback function that is called after a successful geocode. If not supplied, the default behavior is to fly the camera to the result destination.
*/
function GeocoderViewModel(options) {
//>>includeStart('debug', pragmas.debug);
Expand Down Expand Up @@ -77,9 +78,7 @@ define([
this._suggestions = [];
this._selectedSuggestion = undefined;
this._showSuggestions = true;
this._updateCamera = updateCamera;
this._adjustSuggestionsScroll = adjustSuggestionsScroll;
this._updateSearchSuggestions = updateSearchSuggestions;

this._handleArrowDown = handleArrowDown;
this._handleArrowUp = handleArrowUp;

Expand Down Expand Up @@ -140,7 +139,7 @@ define([
that._searchText = data.displayName;
var destination = data.destination;
clearSuggestions(that);
updateCamera(that, destination);
that.destinationFoundCommand(that, destination);
};

this.hideSuggestions = function () {
Expand Down Expand Up @@ -173,14 +172,20 @@ define([
*/
this.autoComplete = defaultValue(options.autocomplete, true);

/**
* Gets and sets the command called when a geocode destination is found
* @type {Command}
*/
this.destinationFoundCommand = createCommand(defaultValue(options.destinationFound, GeocoderViewModel._updateCamera));

this._focusTextbox = false;

knockout.track(this, ['_searchText', '_isSearchInProgress', 'keepExpanded', '_suggestions', '_selectedSuggestion', '_showSuggestions', '_focusTextbox']);

var searchTextObservable = knockout.getObservable(this, '_searchText');
searchTextObservable.extend({ rateLimit: { timeout: 500 } });
this._suggestionSubscription = searchTextObservable.subscribe(function() {
updateSearchSuggestions(that);
GeocoderViewModel._updateSearchSuggestions(that);
});
/**
* Gets a value indicating whether a search is currently in progress. This property is observable.
Expand Down Expand Up @@ -326,7 +331,7 @@ define([
}
next = currentIndex - 1;
viewModel._selectedSuggestion = viewModel._suggestions[next];
adjustSuggestionsScroll(viewModel, next);
GeocoderViewModel._adjustSuggestionsScroll(viewModel, next);
}

function handleArrowDown(viewModel) {
Expand All @@ -338,7 +343,7 @@ define([
var next = (currentIndex + 1) % numberOfSuggestions;
viewModel._selectedSuggestion = viewModel._suggestions[next];

adjustSuggestionsScroll(viewModel, next);
GeocoderViewModel._adjustSuggestionsScroll(viewModel, next);
}

function computeFlyToLocationForCartographic(cartographic, terrainProvider) {
Expand Down Expand Up @@ -445,7 +450,7 @@ define([
var geocoderResults = result.value;
if (result.state === 'fulfilled' && defined(geocoderResults) && geocoderResults.length > 0) {
viewModel._searchText = geocoderResults[0].displayName;
updateCamera(viewModel, geocoderResults[0].destination);
viewModel.destinationFoundCommand(viewModel, geocoderResults[0].destination);
return;
}
viewModel._searchText = query + ' (not found)';
Expand Down Expand Up @@ -521,5 +526,10 @@ define([
});
}

//exposed for testing
GeocoderViewModel._updateCamera = updateCamera;
GeocoderViewModel._updateSearchSuggestions = updateSearchSuggestions;
GeocoderViewModel._adjustSuggestionsScroll = adjustSuggestionsScroll;

return GeocoderViewModel;
});
54 changes: 5 additions & 49 deletions Specs/Widgets/Geocoder/GeocoderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,6 @@ defineSuite([

var scene;

var mockDestination = new Cartesian3(1.0, 2.0, 3.0);
var geocoderResults = [{
displayName: 'a',
destination: mockDestination
}, {
displayName: 'b',
destination: mockDestination
}, {
displayName: 'c',
destination: mockDestination
}];

var customGeocoderOptions = {
autoComplete : true,
geocode : function (input) {
return when.resolve(geocoderResults);
}
};
beforeEach(function() {
scene = createScene();
});
Expand All @@ -40,16 +22,20 @@ defineSuite([

it('constructor sets expected properties', function() {
var flightDuration = 1234;
var destinationFound = jasmine.createSpy();

var geocoder = new Geocoder({
container : document.body,
scene : scene,
flightDuration : flightDuration
flightDuration : flightDuration,
destinationFound : destinationFound
});

var viewModel = geocoder.viewModel;
expect(viewModel.scene).toBe(scene);
expect(viewModel.flightDuration).toBe(flightDuration);
viewModel.destinationFoundCommand();
expect(destinationFound).toHaveBeenCalled();
geocoder.destroy();
});

Expand Down Expand Up @@ -96,34 +82,4 @@ defineSuite([
});
}).toThrowDeveloperError();
});

it('automatic suggestions can be navigated by arrow up/down keys', function() {
var container = document.createElement('div');
container.id = 'testContainer';
document.body.appendChild(container);
var geocoder = new Geocoder({
container : 'testContainer',
scene : scene,
geocoderServices : [customGeocoderOptions]
});
var viewModel = geocoder._viewModel;
viewModel._searchText = 'some_text';
viewModel._updateSearchSuggestions(viewModel);

expect(viewModel._selectedSuggestion).toEqual(undefined);
viewModel._handleArrowDown(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('a');
viewModel._handleArrowDown(viewModel);
viewModel._handleArrowDown(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('c');
viewModel._handleArrowDown(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('a');
viewModel._handleArrowDown(viewModel);
viewModel._handleArrowUp(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('a');
viewModel._handleArrowUp(viewModel);
expect(viewModel._selectedSuggestion).toBeUndefined();
document.body.removeChild(container);
});

}, 'WebGL');
68 changes: 56 additions & 12 deletions Specs/Widgets/Geocoder/GeocoderViewModelSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ defineSuite([
}, {
displayName: 'b',
destination: mockDestination
}, {
displayName: 'c',
destination: mockDestination
}];
var customGeocoderOptions = {
autoComplete: true,
Expand Down Expand Up @@ -158,8 +161,8 @@ defineSuite([
geocoderServices : [customGeocoderOptions]
});
geocoder._searchText = 'some_text';
geocoder._updateSearchSuggestions(geocoder);
expect(geocoder._suggestions.length).toEqual(2);
GeocoderViewModel._updateSearchSuggestions(geocoder);
expect(geocoder._suggestions.length).toEqual(3);
});

it('update search suggestions results in empty list if the query is empty', function() {
Expand All @@ -168,35 +171,38 @@ defineSuite([
geocoderServices : [customGeocoderOptions]
});
geocoder._searchText = '';
spyOn(geocoder, '_adjustSuggestionsScroll');
geocoder._updateSearchSuggestions(geocoder);

GeocoderViewModel._updateSearchSuggestions(geocoder);
expect(geocoder._suggestions.length).toEqual(0);
});

it('can activate selected search suggestion', function () {
spyOn(GeocoderViewModel, '_updateCamera');
var destination = new Rectangle(0.0, -0.1, 0.1, 0.1);
var geocoder = new GeocoderViewModel({
scene : scene,
geocoderServices : [customGeocoderOptions]
});
spyOn(geocoder, '_updateCamera');
spyOn(geocoder, '_adjustSuggestionsScroll');

var suggestion = { displayName: 'a', destination: new Rectangle(0.0, -0.1, 0.1, 0.1) };
var suggestion = { displayName: 'a', destination: destination };
geocoder._selectedSuggestion = suggestion;
geocoder.activateSuggestion(suggestion);
expect(geocoder._searchText).toEqual('a');
expect(GeocoderViewModel._updateCamera).toHaveBeenCalledWith(geocoder, destination);
});

it('if more than one geocoder service is provided, use first result from first geocode in array order', function () {
spyOn(GeocoderViewModel, '_updateCamera');

var geocoder = new GeocoderViewModel({
scene : scene,
geocoderServices : [noResultsGeocoder, customGeocoderOptions2]
});
geocoder._searchText = 'sthsnth'; // an empty query will prevent geocoding
spyOn(geocoder, '_updateCamera');
spyOn(geocoder, '_adjustSuggestionsScroll');

geocoder.search();
expect(geocoder._searchText).toEqual(geocoderResults2[0].displayName);
expect(GeocoderViewModel._updateCamera).toHaveBeenCalledWith(geocoder, mockDestination);
});

it('can update autoComplete suggestions list using multiple geocoders', function () {
Expand All @@ -205,10 +211,48 @@ defineSuite([
geocoderServices : [customGeocoderOptions, customGeocoderOptions2]
});
geocoder._searchText = 'sthsnth'; // an empty query will prevent geocoding
spyOn(geocoder, '_updateCamera');
spyOn(geocoder, '_adjustSuggestionsScroll');
geocoder._updateSearchSuggestions(geocoder);
GeocoderViewModel._updateSearchSuggestions(geocoder);
expect(geocoder._suggestions.length).toEqual(geocoderResults1.length + geocoderResults2.length);
});

it('uses custom destination found callback', function () {
spyOn(GeocoderViewModel, '_updateCamera');

var destinationFound = jasmine.createSpy();
var geocoder = new GeocoderViewModel({
scene : scene,
geocoderServices : [noResultsGeocoder, customGeocoderOptions2],
destinationFound: destinationFound
});
geocoder._searchText = 'sthsnth'; // an empty query will prevent geocoding
geocoder.search();

expect(geocoder._searchText).toEqual(geocoderResults2[0].displayName);
expect(GeocoderViewModel._updateCamera).not.toHaveBeenCalled();
expect(destinationFound).toHaveBeenCalledWith(geocoder, mockDestination);
});

it('automatic suggestions can be navigated by arrow up/down keys', function() {
spyOn(GeocoderViewModel, '_adjustSuggestionsScroll');
var viewModel = new GeocoderViewModel({
scene : scene,
geocoderServices : [customGeocoderOptions]
});
viewModel._searchText = 'some_text';
GeocoderViewModel._updateSearchSuggestions(viewModel);

expect(viewModel._selectedSuggestion).toEqual(undefined);
viewModel._handleArrowDown(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('a');
viewModel._handleArrowDown(viewModel);
viewModel._handleArrowDown(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('c');
viewModel._handleArrowDown(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('a');
viewModel._handleArrowDown(viewModel);
viewModel._handleArrowUp(viewModel);
expect(viewModel._selectedSuggestion.displayName).toEqual('a');
viewModel._handleArrowUp(viewModel);
expect(viewModel._selectedSuggestion).toBeUndefined();
});
}, 'WebGL');