Refactor image embedding logic #406
Conversation
|
|
martinRenou
force-pushed
the
image_embedding_refactor
branch
5 times, most recently
from
d149e3e to
7c997b4
Compare
martinRenou
force-pushed
the
image_embedding_refactor
branch
from
7c997b4 to
9aad6fc
Compare
| </div> | ||
| """.format( | ||
| self._figure_label, self._data_url, width | ||
| ) | ||
|
|
||
| # Update the widget model properly for HTML embedding | ||
| self._size = (width, height) |
There was a problem hiding this comment.
This is one important change here.
We need to make the plot size part of the widget model in the case of nbconvert --to html otherwise there is no registered size and the canvas would be 0 sized.
| @@ -275,17 +287,49 @@ export class MPLCanvasModel extends DOMWidgetModel { | |||
| this.image = new Image(); | |||
|
|
|||
| this.image.onload = () => { | |||
| // In case of an embedded widget, the initial size is not correct | |||
| // and we are not receiving any resize event from the server | |||
| if (this.disabled) { | |||
There was a problem hiding this comment.
This is the second important change in this PR.
Here, we need a special case for when the widget is disabled (no counterpart in the kernel, or no kernel at all), where we display the image that is part of the model.
There was a problem hiding this comment.
Am I right that this will come up in many situations, not just nbconvert, right?
There was a problem hiding this comment.
Yes. You can trigger this situation by saving the widget model in the page, shutdown the kernel, then refresh the page.
| @@ -184,8 +185,7 @@ class Canvas(DOMWidget, FigureCanvasWebAggCore): | |||
| # This will still be used by ipywidgets in the case of embedding. | |||
| _data_url = Any(None).tag(sync=True) | |||
|
|
|||
| _width = CInt().tag(sync=True) | |||
| _height = CInt().tag(sync=True) | |||
| _size = Tuple([0, 0]).tag(sync=True) | |||
There was a problem hiding this comment.
Replacing the two private traits _width and _height into _size so that the resize happens in one comm message
|
Haven't tried it out, but this all looks pretty sensible to me. |
|
Thanks for looking into it! I will merge and make a patch release with this soonish. |
Refactor the image embedding logic.
This fixes an issue in the case of
nbconvert --to htmlthat was preventing the plot image to render.