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 dataJson option in geographyConfig #121

Merged
merged 2 commits into from
Aug 20, 2014
Merged

Conversation

ramnathv
Copy link
Contributor

This PR allows the use dataJson in geographyConfig to specify a custom topojson file as a string, instead of having to use dataUrl and link to an external file. This is especially useful in generating standalone html files with no dependencies.

This PR allows the use `dataJson` in `geographyConfig` to specify a custom topojson file as a string, instead of having to use `dataUrl` and link to an external file. This is especially useful in generating standalone html files with no dependencies.
@markmarkoh
Copy link
Owner

Thanks for this, I think this is a good feature to add.

Since topojson is valid JSON, can we ditch the JSON.parse requirement so that a user can send in normal JSON?

In the event they do have just a string representation, they could always do the JSON.parse external to the DataMaps instantiation.

What do you think?

@ramnathv
Copy link
Contributor Author

Yes. That will work since the JSON.parse can always be done externally. Do you want me to make that change, or you prefer to merge and make it yourself?

@markmarkoh
Copy link
Owner

I'd prefer you make the change and push back to this PR.

@ramnathv
Copy link
Contributor Author

I have made the change.

markmarkoh added a commit that referenced this pull request Aug 20, 2014
Add dataJson option in geographyConfig
@markmarkoh markmarkoh merged commit 8283e4b into markmarkoh:master Aug 20, 2014
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.

None yet

2 participants