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

Add geocoding to directions.inputControl #74

Merged
merged 4 commits into from
May 26, 2015
Merged

Add geocoding to directions.inputControl #74

merged 4 commits into from
May 26, 2015

Conversation

tmcw
Copy link
Contributor

@tmcw tmcw commented May 22, 2015

closes #73

return;
}

resp = resp || err;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's what I suggest for the remainder of this method:

if (err) return cb(err);

try {
   resp = JSON.parse(resp.responseText);
} catch (e) {
    return cb(e);
}

return cb(null, resp);

So:

  • Pass the corslite or parsing error directly to the callback, reserving this.fire('error') for actual direction request errors.
  • Let the callback receive the full response. It can take care of extracting the first result, or showing a list of results if desired.

@lbud
Copy link
Contributor

lbud commented May 23, 2015

@jfirebaugh I think this is ready — mind taking a quick pass before merge?

@@ -114,7 +146,7 @@ var Directions = L.Class.extend({
token = this.options.accessToken || L.mapbox.accessToken,
profile = this.getProfile(),
points = [this.origin].concat(this._waypoints).concat([this.destination]).map(function (point) {
return point.properties.query || point.geometry.coordinates;
return point.geometry.coordinates || point.properties.query;
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be safe to remove the || fallback entirely now.

@jfirebaugh
Copy link
Contributor

This looks good from a code standpoint but I'm concerned about issues like https://github.com/mapbox/geocoder-feedback/issues/60 -- if I'm zoomed out looking at California and search for "San Francisco" to "Chico", I'm not expecting to start from "San Francisco St, Yountville, 94599, California, United States".

@ingalls, anything you can suggest to get more predictable results? Setting the precision of the proximity parameter based on the map zoom doesn't seem to have the effect I would hope for.

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

Successfully merging this pull request may close these issues.

inputControl appears to accept text input
3 participants