Add support for printing widgets #5850

saulshanabrook · 2019-01-09T16:12:58Z

This closes #1314 by adding the ability to print different widgets, including notebooks.

Features

It adds a new command called Print... to the File menu, which is enabled when the active widget is printable. This is enabled currently for:

images (copies dom nodes)
the inspector (copies dom nodes)
notebooks (loads nbformat URL prints out HTML output)
JSON viewer (copies dom nodes)

This is set to SUPER P by default.

Not implemented

a button for printing the current widget
A keyboard shortcut for printing the current widget

Implementation

The core printing logic is in the apputils/src/printing.tsx file, with the core structure being the IPrintable interface:

export type OptionalAsyncThunk = () => Promise<void> | null;
export const printSymbol = Symbol();
export interface IPrintable {
  [symbol]: () => OptionalAsyncThunk;
}

The command checks to see if the active widget is printable (i.e. it has the printSymbol property and calling that method returns a function) and if so calls that method on it.

There are some helper functions provided to support having widgets defer their ability to print to a child widget and the ability to print a widgets DOM node or a custom URL.

To print a DOM node, or a URL, we create an iframe and copy in a DOM node into it or set the src text to HTML text.

Screenshots

Notebook:

Inspector:

JSON:

Image:

jasongrout · 2019-01-09T18:42:05Z

Are you tying print functionality to document widgets (I notice it seems some functionality is on the docmanager)? It would be great if an arbitrary activity could be printable, even if it wasn't a document widget.

jasongrout · 2019-01-09T18:43:13Z

(An example of an interesting printable non-document widget might be the inspector panel)

saulshanabrook · 2019-01-09T19:03:28Z

Are you tying print functionality to document widgets (I notice it seems some functionality is on the docmanager)? It would be great if an arbitrary activity could be printable, even if it wasn't a document widget.

Nope. It call the printSymbol method on the widget, it isn't tied to the document widget. Check out the latest commit, in it I have printing working for images, the inspector, and JSON.

jasongrout · 2019-01-09T19:30:13Z

Awesome. I'm excited about this work!

Also, I'm glad you are experimenting with using Symbols.

saulshanabrook · 2019-01-09T21:14:05Z

OK this is ready to go from my end.

jasongrout

@ian-r-rose - can you comment on the security of printing via iframe? I notice that https://github.com/joseluisq/printd/blob/master/src/index.ts doesn't seem to use any sandbox attributes.

packages/apputils/src/printing.ts

jasongrout · 2019-01-27T06:12:17Z

packages/apputils/src/printing.ts

+/**
+ * Prints a URL by loading it into an iframe.
+ *
+ * NOTE: When https://github.com/joseluisq/printd/issues/20 is fixed this can be removed.


Our convention is to add a #### Notes section after the parameters for notes like these.

ian-r-rose

I was unable to get this to build, but the overall approach looks good. Looks like it just needs a rebase.

packages/apputils/src/printing.ts

ian-r-rose · 2019-01-28T20:33:31Z

packages/inspector/src/inspector.ts

+   * Print in iframe
+   */
+  [printSymbol]() {
+    printWidget.bind(this)('.p-mod-hidden {display: none;}');


Is there a way to grab more CSS so that this is less ad hoc? I admit this is stretching the bounds of my DOM API knowledge, but if we could do something like node.getAllCSSAsString(), that would be much nicer.

ian-r-rose · 2019-01-28T20:39:37Z

packages/notebook-extension/src/index.ts

  const factory = new NotebookWidgetFactory({
    name: FACTORY,
    fileTypes: ['notebook'],
    modelName: 'notebook',
    defaultFor: ['notebook'],
    preferKernel: true,
+    baseUrl: services.serverSettings.baseUrl,


I think the notebook widget already has access to the serviceManager, so it shouldn't be necessary to pass in the baseUrl individually.

ian-r-rose · 2019-01-28T20:39:49Z

packages/notebook-extension/src/index.ts

@@ -475,12 +470,14 @@ function activateNotebookHandler(
  // An object for tracking the current notebook settings.
  let editorConfig = StaticNotebook.defaultEditorConfig;
  let notebookConfig = StaticNotebook.defaultNotebookConfig;
+  // Pass URL in here.


Out of date comment?

Yep, thanks!

packages/notebook/src/panel.ts

packages/notebook/src/widget.ts

saulshanabrook · 2019-01-28T20:56:17Z

Thanks @ian-r-rose and @jasongrout for the comments, much appreciated.

saulshanabrook · 2019-01-30T18:27:19Z

In reference to @ian-r-rose's question about including the base URL in the NotebookPanel, we had a productive discussion during our meeting today about how to avoid this problem.

Solutions:

Move getNBConvertURL to page config file and import it in notebook panel to define printing. There are other similar URL utilities in page config so it makes some sense there and then we don't have to have it passed in. However, the downside of this is that the nbconvert server might not be running, and should probably be optional, and this exposes it as a global core API.
This thinking encourages us to make nbconvert a separate extension, not tied to the notebook. If this is the case, we need to be able to add the printing logic outside of the NotebookPanel class. We could do this by:
- Keeping existing interface based print API:
  - Monkeypatch NotebookPanel class to add print symbol property
  - Monkeypatch each NotebookPanel instance by registering some notebook widget tracker (@ian-r-rose I forget the actual name) on instantiate.
- Change the print API to also include a dispatch based approach. This would allow the (hypothetical) NB convert plugin to say "I know how to print NotebookPanel widgets and this is how".

I am currently in favor of the last option (dispatching), because it seems the most general, but also allowing widgets to define their own printing. I will keep exploring this and post possible designs here.

ian-r-rose · 2019-01-30T18:33:52Z

    * Monkeypatch each `NotebookPanel` instance by registering some notebook widget tracker (@ian-r-rose I forget the actual name) on instantiate.

I was referring to a DocumentRegistry.IWidgetExtension:

jupyterlab/packages/docregistry/src/registry.ts

Lines 998 to 1005 in 930420a

    
             /** 
        
              * An interface for a widget extension. 
        
              */ 
        
             export interface IWidgetExtension<T extends Widget, U extends IModel> { 
        
               /** 
        
                * Create a new extension for a given widget. 
        
                */ 
        
               createNew(widget: T, context: IContext<U>): IDisposable;

This, for instance, is what the ipywidgets extension uses to add the widgets mimetype to a notebook:
https://github.com/jupyter-widgets/ipywidgets/blob/1f93ad39802d3ee4a6b88bc13064f93f8f796374/packages/jupyterlab-manager/src/plugin.ts#L136

Another option would be to require the INotebookTracker and hook into the widgetAdded signal to patch on the print symbol. This would essentially accomplish the same thing. So much so, that we have been considering removing the DocumentRegistry.IWidgetExtension (#3974).

gnestor · 2019-02-01T03:29:56Z

@saulshanabrook Do you have any insight into what it would require for nbconvert to output static HTML of ipywidget output? There is a conversation about this going on as jupyter/nbconvert#944

saulshanabrook · 2019-02-01T13:00:38Z

@gnestor, I do not, but thanks for the pointer.

bollwyvl · 2019-02-01T13:49:41Z

So the closest thing is probably decktape, which concentrates on PDF output of slides. Uses puppeteer (so chrome only, and does some nasty install stuff). I had done one of these, also targeted at PDF output based on 15th, but haven't revisited since qtwebengine landed... Many things were broken in their old thing. But that's also chromium based. I think if we were to do anything like this as jupyter, it would have to support/prefer Firefox. It's a little "I heard you liked widgets", but it seems like widget packages would also have to include a kernel-side html renderer to enable this without a browser in the loop... For some things, this would just mean packing an AMD package (for now) that nbconvert could find and handle prior to interpreting the widget... unpkg etc is not a robust solution for archival content. some widgets could opt for full server-side rendering. However, lab-forward nbconvert will need a story at some point, which is when the built application situation will need to be resolved for real... I don't think most people will want to wait a couple minutes and have to fiddle with nodejs memory limits to make a static html page after fixing a typo, and nbviewer just can't support that (unless we get REALLY good at caching). The upside is that if we _did_ solve it (e.g. no js on server), the ux of using lab extensions would increase 💯x, and we would get static output for (almost) free: no worse than any PDF approach thus far.

gnestor · 2019-02-02T02:09:13Z

@bollwyvl Yes, it does sound like we would need to use something like decktape that can render the output in a headless browser and convert that to a static HTML, image, PDF, etc.

I think if we were to do anything like this as jupyter, it would have to
support/prefer Firefox.

Why is this?

It's a little "I heard you liked widgets", but it seems like widget
packages would also have to include a kernel-side html renderer to enable
this without a browser in the loop... For some things, this would just mean
packing an AMD package (for now) that nbconvert could find and handle prior
to interpreting the widget... unpkg etc is not a robust solution for
archival content. some widgets could opt for full server-side rendering.

The simple solution is that widgets provide fallback mimetypes in the output, like text/html or image/png. Do you envision the "kernel-side html renderer" providing a text/html output?

bollwyvl · 2019-02-02T03:51:23Z

On Fri, Feb 1, 2019, 21:09 Grant Nestor ***@***.***> wrote: @bollwyvl <https://github.com/bollwyvl> Yes, it does sound like we would need to use something like decktape that can render the output in a headless browser and convert that to a static HTML, image, PDF, etc. I think if we were to do anything like this as jupyter, it would have to support/prefer Firefox. Why is this?

ehm, we shouldn't be contributing to the only-works-on-chrome train wreck. Also, it's much more supportable in the field due to their ESR release.

It's a little "I heard you liked widgets", but it seems like widget packages would also have to include a kernel-side html renderer to enable this without a browser in the loop... For some things, this would just mean packing an AMD package (for now) that nbconvert could find and handle prior to interpreting the widget... unpkg etc is not a robust solution for archival content. some widgets could opt for full server-side rendering. The simple solution is that widgets provide fallback mimetypes in the output, like text/html or image/png. Do you envision the "kernel-side html renderer" providing a text/html output?

Yes, a good archival format should be the ultimate static fallback... Images without embedded fonts, basically. Or plain text. But made with something that isn't on the infinite Merry-go-round of browser churn. However, if you could keep interactivity, you might need to ship some js... Consider pythreejs: you could use freecad (ha), generate the image, and dump that. But if you had two geometry things on the page, you'd only want to embed three js once. Or vrml, lol.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5850 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACxRDncVOcrKp-LEEYjYFzwrxhKpZTiks5vJPNKgaJpZM4Z3zp6> .

gnestor · 2019-02-06T21:55:39Z

However, if you could keep interactivity, you might need to ship some js...

The one issue with this is that widgets typically interact with a kernel (e.g. that's what differentiates the plotlywidget from the plotly-extension), so when I see a widget rendered outside of a notebook but it doesn't behave the way it's expected to because it's not connected to a kernel (like in the ipywidgets docs: https://ipywidgets.readthedocs.io/en/stable/examples/Widget%20Events.html#Linking-traitlets-attributes-in-the-kernel), then I think the result is worse than if it were just a static image.

bollwyvl · 2019-02-06T22:57:39Z

Right, so it would be up to the implementation to decide that, and just emit an image... Or an image in text/html with a tiny bit of interactivity e.g. zoom if it's a vector... Bad example, I guess, as that could just be a feature that all vector image get. But jslink models are way cool.

saulshanabrook · 2019-05-06T20:05:10Z

@ian-r-rose and I did some pair coding on this today and ironed out most of the kinks. Currently, the issue is that one Safari and on Chrome on linux (but not on Mac) the iframe is not being cleaned up because the afterprint event isn't being triggered for some reason.

Ian is gonna look into this a bit more and maybe experiment with watching how the media queries change as a proxy for when we are done printing.

Another option is to only wait for afterprint on firefox, because on chrome it seems to block anyways for long enough to keep the print dialogue open before deleting the iframe.

ian-r-rose · 2019-05-06T21:26:10Z

Should have been obvious to us earlier, but the reason that onafterprint and media queries were not behaving as we expected was due to the fact that we dont 'allow-scripts'.

ian-r-rose · 2019-05-06T21:26:28Z

@saulshanabrook, do you have any thoughts on how to test this in CI?

JS is blocked, we consider it safe to discard of the hidden iframe once event handling has returned to the main content window.

ian-r-rose · 2019-05-06T22:17:15Z

@saulshanabrook I've added what might be considered a huge hack for deciding when to discard the hidden iframe, but it seems to work okay on Ubuntu with Chrome and Firefox. What do you think?

ian-r-rose

I think this is really close. Do you have any thoughts on how we would get this working for codemirror instances?

packages/apputils/src/printing.ts

ian-r-rose · 2019-05-06T22:19:29Z

packages/apputils/src/printing.ts

+   * Copies a node from the base document to the iframe.
+   */
+  function setIFrameNode(iframe: HTMLIFrameElement, node: HTMLElement) {
+    iframe.contentDocument.body.appendChild(node.cloneNode(true));


Does this need to wait for the load event? My experience with the PDF viewer is that it was necessary, but maybe that's not the case here?

I haven't found we needed to wait.

ian-r-rose · 2019-05-06T22:20:09Z

packages/coreutils/src/pageconfig.ts

+   * Returns the URL converting this notebook to a certain
+   * format with nbconvert.
+   */
+  export function getNBConvertURL({


Ping @afshin on this

saulshanabrook · 2019-05-13T11:26:59Z

Should have been obvious to us earlier, but the reason that onafterprint and media queries were not behaving as we expected was due to the fact that we dont 'allow-scripts'.

But they do trigger in Chrome, which is odd...

I've added what might be considered a huge hack for deciding when to discard the hidden iframe, but it seems to work okay on Ubuntu with Chrome and Firefox. What do you think?

Looks good to me! I will test it locally in chrome, firefox and safari to make sure it is cleaned up.

@saulshanabrook, do you have any thoughts on how to test this in CI?

Hah... Selenium doesn't cover printing. Looks like our best bet would be to use puppeteer, Chrome's new browser UX testing framework, but it looks like the print dialogue isn't caught yet (puppeteer/puppeteer#3145). They are also starting a FIrefox compatible puppeteer.

Do you have any thoughts on how we would get this working for codemirror instances?

I will have a go at it.

saulshanabrook · 2019-05-13T11:42:47Z

I will test it locally in chrome, firefox and safari to make sure it is cleaned up.

I tested printing a notebook and the inspector on all three, to test the two code paths. They all printed properly and the iframes were cleaned up, besides in safari, printing the inspector resulted in a blank screen. I am fine to call that "success"

@saulshanabrook, do you have any thoughts on how to test this in CI?

Maybe I could document how to test this for a manual release sort of test? I think there are just some UX tests that cannot be automated currently and the best practice would be to have a list we could go down to verify that they are working. This slightly overlaps with @blink1073's work in #6312. Do you think I should put something like that in the RELEASE.md file? cc @jasongrout and @ellisonbg

saulshanabrook · 2019-05-13T12:36:45Z

Do you have any thoughts on how we would get this working for codemirror instances?

I started trying to get this working, but kept ending up with an unstyled printout and am finding it hard to debug:

Here is the commit that is trying this out. I get rather confused about when I should be waiting for the page to load. If you wanted to try to work through this together this week, I would be game, or happy to merge this as is.

saulshanabrook · 2019-05-13T12:38:52Z

The docs failure is because a github URL is not resolving properly temporarily, it says "too many hits..."

ian-r-rose · 2019-05-13T15:02:46Z

I will test it locally in chrome, firefox and safari to make sure it is cleaned up.

I tested printing a notebook and the inspector on all three, to test the two code paths. They all printed properly and the iframes were cleaned up, besides in safari, printing the inspector resulted in a blank screen. I am fine to call that "success"

Does this mean that printing DOM elements doesn't work in Safari? Did it work with any of the previous iterations of the PR?

packages/apputils-extension/src/index.ts

ian-r-rose · 2019-05-13T16:34:24Z

This looks great, thanks for the perseverance @saulshanabrook!

saulshanabrook changed the title ~~Add supporting for printing widgets~~ WIP: Add supporting for printing widgets Jan 9, 2019

saulshanabrook changed the title ~~WIP: Add supporting for printing widgets~~ WIP: Add support for printing widgets Jan 9, 2019

saulshanabrook added this to the 1.0 milestone Jan 9, 2019

saulshanabrook force-pushed the print-notebook branch from fb7ec2a to 708e465 Compare January 9, 2019 19:09

saulshanabrook changed the title ~~WIP: Add support for printing widgets~~ Add support for printing widgets Jan 9, 2019

jasongrout self-requested a review January 27, 2019 05:56

jasongrout reviewed Jan 27, 2019

View reviewed changes

ian-r-rose reviewed Jan 28, 2019

View reviewed changes

saulshanabrook added 7 commits March 18, 2019 11:41

Add printd

ef8213e

Move printint to apputils and add support docs

a788ee7

Add printing for images, JSON and inspector

ecf0a10

Add documentation to printing module

00349dc

Check if printable again when executing

c58e78c

Add newline back

1b01f68

Allow notebook printing

e441c50

Since we can't use traditional event handlers in the print iframe when

1593668

JS is blocked, we consider it safe to discard of the hidden iframe once event handling has returned to the main content window.

ian-r-rose reviewed May 6, 2019

View reviewed changes

ian-r-rose added pkg:apputils pkg:json pkg:notebook enhancement labels May 10, 2019

saulshanabrook added 4 commits May 13, 2019 07:27

Merge branch 'master' into print-notebook

d225704

Only create server settings once for printing

ca9890d

Linting fix

a406241

Remove unused import

609799b

saulshanabrook requested a review from ian-r-rose May 13, 2019 12:39

ian-r-rose added the tag:Browser Compatibility label May 13, 2019

ian-r-rose reviewed May 13, 2019

View reviewed changes

packages/apputils-extension/src/index.ts Outdated Show resolved Hide resolved

saulshanabrook added 2 commits May 13, 2019 11:18

Remove contextMenuWidget

77cb8da

Get both widget from main app for printing

208aa83

ian-r-rose merged commit 3670ef1 into jupyterlab:master May 13, 2019

saulshanabrook deleted the print-notebook branch May 13, 2019 17:49

lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 7, 2019

lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019

fcollonval removed the status:Needs Review label Mar 2, 2022

Add support for printing widgets #5850

Add support for printing widgets #5850

Conversation

saulshanabrook commented Jan 9, 2019 • edited

Features

Not implemented

Implementation

Screenshots

jasongrout commented Jan 9, 2019

jasongrout commented Jan 9, 2019

saulshanabrook commented Jan 9, 2019

jasongrout commented Jan 9, 2019

saulshanabrook commented Jan 9, 2019

jasongrout left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ian-r-rose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saulshanabrook commented Jan 28, 2019

saulshanabrook commented Jan 30, 2019

ian-r-rose commented Jan 30, 2019 • edited

gnestor commented Feb 1, 2019 • edited

saulshanabrook commented Feb 1, 2019

bollwyvl commented Feb 1, 2019 via email

gnestor commented Feb 2, 2019

bollwyvl commented Feb 2, 2019 via email

gnestor commented Feb 6, 2019

bollwyvl commented Feb 6, 2019 via email

saulshanabrook commented May 6, 2019

ian-r-rose commented May 6, 2019

ian-r-rose commented May 6, 2019

ian-r-rose commented May 6, 2019

ian-r-rose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saulshanabrook commented May 13, 2019 • edited

saulshanabrook commented May 13, 2019

saulshanabrook commented May 13, 2019

saulshanabrook commented May 13, 2019

ian-r-rose commented May 13, 2019

ian-r-rose commented May 13, 2019

saulshanabrook commented Jan 9, 2019 •

edited

ian-r-rose commented Jan 30, 2019 •

edited

gnestor commented Feb 1, 2019 •

edited

saulshanabrook commented May 13, 2019 •

edited