-
Notifications
You must be signed in to change notification settings - Fork 958
Persist widgets as screenshots to the notebook json. #314
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
Conversation
This is a great idea. I did not know about html2canvas! Does it handle svg? |
Yes, it should! |
🎉 |
I should mention, this is WIP. |
This is great. The on-save is a really neat idea. |
Wow, does |
@ellisonbg Browserify (via wzrd.in at least) is certainly barfing on it, haven't tried webpack. If so, 😒 |
@jdfreder If you are at Cal Poly later this week, I would love to see it live. 🏄 |
It works in the browser. This PR is for the live notebook.
Not sure how you are importing it, but be aware it isn't an AMD module, it just pollutes the window with a |
Ok, cool. I assume they're just not providing commonjs then. I went to tinker and provide a link to a working sample to Brian, gave up at stack trace. Sorry for the alarm! |
This solves the issue of RTD and GitHub rendering of notebooks which is pretty awesome. Regarding nbviewer, I think that it should actually really support interactive widgets. |
Talked to @ellisonbg , and he suggested adding a render button for doing the HTML -> PNG conversion, since it is asynchronous. I didn't know what icon to use, so I used a truck: Here it is. in action: Here's the output, when saved: https://gist.github.com/jdfreder/067476b62ed0f0042f89 Open to suggestions for the icon, otherwise, this is good to go! |
I already use an auto for |
@jdfreder the problem you have with save event handler that have to be fully synchronous is also going to arise if you want to save the widget state when the notebook is saved. It was not the case before, but since we use (asynchronously loaded) custom serializers, we will have the same issue. Hence, it is probably something we should tackle in the notebook project. |
Yeah I agree, but for now, I think this should be a button. Then, when that is fixed in the notebook project, I can open an accompanying PR here to do it on save. However, one thing I discovered is that DOM -> PNG can be really slow with a lot of widgets, which may be annoying on save. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Atom!
I don't think this should be in the toolbar, just the Menu (Menu should always be a superset of toolbar actions, anyway). Toolbar space seems too precious for something like this. EDIT: Fixed super-confusing mixture of menubar/toolbar. |
A menu is a really good idea, because in the not-so-distant-future we'll need a button for computing possible widget states. Maybe we'll have more things too in the menu, related to managing widget state. |
@ellisonbg thanks for testing this. I think there may be problems with the library I've selected to do the renderings of the widgets. I'll look into alternatives |
I investigated some more, part of the problem was the overflow: scroll attribute of the #site div in the notebook. Temporarily removing that during the rendering seems to work well. Radio buttons, checkboxes, shadows, and select controls styled as a list box do not render. For a complete demo, see https://gist.github.com/jdfreder/a949a96cc677a6244db4 |
Okay, some good news. I updated the html2canvas component to the latest version and a lot of the bugs in the renderings are gone! Radio buttons and checkboxes now render correctly, see: https://gist.github.com/jdfreder/afb72870ef0c3c2f1fcf List boxes still have a bug where only the selected element is rendered, but I've determined this is a bug in html2canvas and will open an issue there. EDIT: Update URL |
and invoke save on snapshot build
I'm totally mystified as to why the Python tests fail in 0b2ff80 , seeing that I don't modify any Python in this PR. @SylvainCorlay I rebased, added the download state action to the menu, and now save immediately after snapshot capture. This is ready for review. If you decide to test it, don't forget to rebuild your CSS! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as soon as we separate jupyter-js-widgets, we should make those npm deps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment so we don't forget
also typo fix
A minor problem. You should probably disable scrolling of the notebook while rendering the snapshots. |
Last commit hides the scrollbars and fixes the scroll position. |
Persist widgets as screenshots to the notebook json.
👍 |
Finally! This PR introduces a work around for the inability to render widgets on GitHub, nbviewer, through nbconvert, etc... A save hook is used to persist canvas renderings of the widget DOM elements to the notebook JSON prior to save.
Ping @SylvainCorlay , @ellisonbg, @captainsafia, @willingc (we can use this for docs), @blink1073 (similar to your export button in the matplotlib widget renderer)