Skip to content

Screenshot support#25

Merged
tomstitt merged 3 commits intomasterfrom
screenshot
Sep 3, 2021
Merged

Screenshot support#25
tomstitt merged 3 commits intomasterfrom
screenshot

Conversation

@tomstitt
Copy link
Copy Markdown
Member

@tomstitt tomstitt commented Aug 25, 2021

Note: this requires a yet-to-be released version of glvis from npm that I'm working with locally. We can update glvis once GLVis/glvis-js#11 & GLVis/glvis#195 are merged.

Right now the screenshot is triggered from JavaScript so the usual browser download rules apply. Here is what things look like:

TODO:

  • update glvis dependency

Screen Shot 2021-08-25 at 3 31 26 PM

And the actual output: star(4)

Another option would be to get the screen state back to Python and write the file from there. We wouldn't be "downloading" it in the same way.

I think I like the Python approach more because there wouldn't be a download dialog (then again Chrome just downloads the files without a dialog) but I need to try a few more things to get it working.

Is there anything else you were hoping for @rw-anderson, @tzanio , @kmittal2? (I saw you all participated in some way or another on #5)

addresses: #5

@tomstitt tomstitt added the enhancement New feature or request label Aug 25, 2021
@tomstitt tomstitt requested review from rw-anderson and tzanio August 25, 2021 22:38
@tomstitt tomstitt self-assigned this Aug 25, 2021
@tomstitt tomstitt requested a review from kmittal2 August 25, 2021 22:38
@tzanio tzanio mentioned this pull request Aug 26, 2021
15 tasks
Copy link
Copy Markdown
Member

@tzanio tzanio left a comment

Choose a reason for hiding this comment

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

This looks great to me, thanks @tomstitt !

@rw-anderson
Copy link
Copy Markdown

rw-anderson commented Aug 26, 2021

"Another option would be to get the screen state back to Python and write the file from there. We wouldn't be "downloading" it in the same way."

This seems preferable for the use case that raised this issue in the first place for me. I wanted to create "galleries" of examples programmatically, maybe 20 at a time or something in that ballpark. If I had to navigate 20 download dialogs, that would be a big bottleneck.

But reading more carefully, if chrome just downloads without a dialog, that seems fine, too.

@tomstitt
Copy link
Copy Markdown
Member Author

I added an option to download via Python with the caveat that since the messaging is asynchronous we don't (can't easily) block so there is either no feedback when you .screenshot or there is an output but it is under the widget not the cell you ran screenshot; a little weird for sure but I tested with ex9 and it saved all the images

@tomstitt tomstitt merged commit ef02be3 into master Sep 3, 2021
@tzanio tzanio deleted the screenshot branch September 5, 2021 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request ready-for-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants