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.

Sign up for GitHub

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

Jump to bottom

Add embed option exportDataSet to set the data used when opening source/opening in editor #720

Closed
wants to merge 1 commit into from
Closed

Add embed option exportDataSet to set the data used when opening source/opening in editor #720

wants to merge 1 commit into from

Conversation

mcintyre94
Copy link

Closes #719

This PR adds a new optional embed option exportDataSet. If it is set, then when the "Open Source" and "Open in Vega Editor" actions are used the data set given is used as the data.values of the spec before it is opened.

This means that it is possible to use Vega's changeset functionality to modify the data, and then open source/editor to eg. extract the latest data or to iterate on the chart in the editor with the latest data available.

I've also added an example file test-changeset.html which demonstrates this feature (and changesets more widely). This is based on the streaming demo: https://vega.github.io/vega-lite/tutorials/streaming.html

Here's a short video showing this working. Note that without this PR the data would be empty in both cases and the editor wouldn't render any data.

vega-embed-export-dataset.mov

domoritz
domoritz requested changes Jul 20, 2021
View reviewed changes
Copy link
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. I think this solution is too brittle as it only works for Vega-Lite.

Instead, we should send the view state via postMessage to the editor.

@mcintyre94
Copy link
Author

Instead, we should send the view state via postMessage to the editor.

Thanks for taking a look! Isn't this what the post function that I'm sending the data in is already doing though? It looks like its implementation is opening the editor window and then sending the data with postMessage. Are you saying that it should send the spec unmodified, and then send a secondary message to update it? Or am I updating the wrong thing by updating the spec?

@mcintyre94
Copy link
Author

mcintyre94 commented Jul 21, 2021 •
edited
Loading

I've just pushed an updated version which does at least work with test-vg.html:

  • The data pushed to the editor now has a name as well as the values
  • When mode is vega, the data is put in an array of length 1. When it's vega-lite it stays as an object.

Hopefully this makes it less brittle, but I'm happy to change the way it sends the message to the editor as you mentioned - I'm just not clear exactly what I need to change there yet.

feat: add embed option exportDataSet
a38ab2e 
If set, this is used to populate the data when opening in Vega editor or viewing Vega source
domoritz
domoritz requested changes Jul 21, 2021
View reviewed changes
Copy link
Member

@domoritz domoritz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This isn't an easy issue so I don't want you to get too hung up when working on it and I will help where I can.

When I say send the state, I mean use https://vega.github.io/vega/docs/api/view/#view_getState in embed and https://vega.github.io/vega/docs/api/view/#view_setState in the editor. You will need to send a pull request to the editor as well.

I had issues with serializing state before so maybe we also need to change Vega to make this approach work.

src/embed.ts
Comment on lines +288 to +291
if (mode === 'vega') {
exportSpec.data = [exportSpecData];
} else if (mode === 'vega-lite') {
exportSpec.data = exportSpecData;
Copy link
Member

Choose a reason for hiding this comment

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

Data can be nested in a spec so this code won't work in general.

@hydrosquall hydrosquall changed the base branch from master to next September 16, 2021 23:56
@domoritz
Copy link
Member

Please send a new pull request.

@domoritz domoritz closed this Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domoritz domoritz domoritz requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is it possible to have changesets update the data property in Vega Editor?
2 participants
@mcintyre94 @domoritz