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

Generalize dataFromJson to allow JSON lists #44

Closed
wants to merge 1 commit into from

Conversation

knuton
Copy link

@knuton knuton commented Mar 12, 2018

I am experimenting with elm-vega and found it counterintuitive that dataFromJson wraps the JSON value into another list.

I think it is better to just assume that the JSON value is already a list. This allows passing complete datasets with all fields defined, which is currently not possible. To support the current usage, single values can simply be wrapped into a JSON list before passing them to dataFromJson.

Please see if this change makes sense to you.

Removes the wrapping of the JSON value into a singleton list. This
allows passing complete datasets with all fields defined. Single values
can simply be wrapped into a JSON list before passing to `dataFromJson`.
@jwoLondon
Copy link
Member

Thanks very much for raising this - much appreciated. I agree with you that this is (unintentionally) counterintuitive. I have made a few other minor changes to data generation functions so will make your suggested changes manually ready for the next release rather than merge the PR.

@jwoLondon jwoLondon closed this Mar 13, 2018
@knuton
Copy link
Author

knuton commented Mar 13, 2018

OK great, thanks

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