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

Pass flyTo options to map upon selection #214

Merged
merged 6 commits into from
Mar 19, 2019
Merged

Conversation

scottsfarley93
Copy link

@scottsfarley93 scottsfarley93 commented Mar 18, 2019

This PR addresses #51 by passing the flyTo object in the constructor options to the flyTo method on the map when a result selection has been made.

  • If the property is an object, the object will be passed to the map to specify a custom animation

  • If the property is true, the map will animate using the default animation parameters

  • If the property is false, the map will not animate to the selected result

  • briefly describe the changes in this PR

  • write tests for all new functionality

  • update CHANGELOG.md with changes under master heading before merging

Copy link
Contributor

@katydecorah katydecorah left a comment

Choose a reason for hiding this comment

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

Nice work figuring out those tests! I left two comments to address a small text change

API.md Outdated
@@ -20,7 +20,7 @@ A geocoder component using Mapbox Geocoding API
- `options.accessToken` **[String](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String)** Required.
- `options.origin` **[String](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String)** Use to set a custom API origin. Defaults to <https://api.mapbox.com>.
- `options.zoom` **[Number](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number)** On geocoded result what zoom level should the map animate to when a `bbox` isn't found in the response. If a `bbox` is found the map will fit to the `bbox`. (optional, default `16`)
- `options.flyTo` **[Boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** If false, animating the map to a selected result is disabled. (optional, default `true`)
- `options.flyTo` **([Boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean) \| [Object](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object))?** If false, animating the map to a selected result is disabled. If true, animating the map with use the default animation parameters. If an object, the object will be passed to the flyTo map method to specify a custom animation.
Copy link
Contributor

Choose a reason for hiding this comment

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

If true, animating the map with use the default

Should this read:

If true, animating the map will use the default

lib/index.js Outdated
@@ -17,7 +17,7 @@ var geocoderService;
* @param {String} options.accessToken Required.
* @param {String} options.origin Use to set a custom API origin. Defaults to https://api.mapbox.com.
* @param {Number} [options.zoom=16] On geocoded result what zoom level should the map animate to when a `bbox` isn't found in the response. If a `bbox` is found the map will fit to the `bbox`.
* @param {Boolean} [options.flyTo=true] If false, animating the map to a selected result is disabled.
* @param {Boolean|Object} [options.flyTo] If false, animating the map to a selected result is disabled. If true, animating the map with use the default animation parameters. If an object, the object will be passed to the flyTo map method to specify a custom animation.
Copy link
Contributor

Choose a reason for hiding this comment

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

If true, animating the map with use the default

Should this read:

If true, animating the map will use the default

flyTo: true
});

var mapFlyMethod = sinon.spy(map, "flyTo");
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!!

@scottsfarley93 scottsfarley93 changed the base branch from master to version4 March 19, 2019 14:44
@scottsfarley93 scottsfarley93 merged commit 36147da into version4 Mar 19, 2019
@scottsfarley93 scottsfarley93 deleted the fly-to-options branch March 19, 2019 14:49
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.

2 participants