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

Reorganize maps configuration #900

Merged
merged 5 commits into from
Aug 24, 2018
Merged

Reorganize maps configuration #900

merged 5 commits into from
Aug 24, 2018

Conversation

kpuputti
Copy link
Contributor

@kpuputti kpuputti commented Aug 16, 2018

BREAKING CHANGE

This PR reorganises the maps configuration to work with both Google Maps and Mapbox. The structure of the configuration is different and paves the way for the bigger change of making Mapbox as the default map provider.

NOTE: Can we merge this already? This will create conflicts if people have edited the related settings in config.js. Although multiple smaller breaking changes is better than one huge one when the whole Mapbox feature lands.

  • Reorganise maps config
  • Document the config
  • Test all possible options
  • Enable example default searches with env var
  • Update env docs
  • Update Changelog
  • Update docs (another PR)

@kpuputti kpuputti force-pushed the reorganise-maps-config branch 4 times, most recently from 7b6c989 to 62053d1 Compare August 17, 2018 12:00
@kpuputti kpuputti requested a review from lyyder August 17, 2018 12:02
// obfuscated coordinates are withing a circle that has the same
// radius as the offset.
coordinateOffset: 500,
const maps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could call this map as the application is using one map at a time. But I guess maps makes sense too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, you sort of configure all the maps in the app. And nothing prevents using two or more maps at a time. But mainly maps is a bit more greppable than map.

@lyyder
Copy link
Contributor

lyyder commented Aug 24, 2018

Merging this so that we can base the actual Mapbox changes on top of this.

@lyyder lyyder merged commit 8b70a91 into master Aug 24, 2018
@lyyder lyyder deleted the reorganise-maps-config branch August 24, 2018 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants