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.

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

Update to Voila 0.5.0a3 #44

Merged
merged 4 commits into from
Mar 23, 2023
Merged

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Mar 23, 2023

References

Follow-up to voila-dashboards/voila#1249

Included in voila==0.5.0a3: https://github.com/voila-dashboards/voila/releases/tag/v0.5.0a3

Code changes

User-facing changes

Backwards-incompatible changes

Should be none.

The custom widget manager is removed but it was not exposed before.

@jtpio jtpio marked this pull request as ready for review March 23, 2023 14:53
Comment on lines +215 to +225
const widgetManager = new KernelWidgetManager(kernel, rendermime);
rendermime.removeMimeType(WIDGET_MIMETYPE);
rendermime.addFactory(
{
safe: false,
mimeTypes: [WIDGET_MIMETYPE],
createRenderer: (options) =>
new WidgetRenderer(options, widgetManager),
},
-10
);
Copy link
Member

Choose a reason for hiding this comment

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

It's unclear to me why we need to do this? Why is the already registered mimetype renderer for widgets not sufficient?

Copy link
Member

Choose a reason for hiding this comment

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

I also don't remember why we needed a custom widget manager in voici since the beginning.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right I am also unsure. This change mimics what is in voila-dashboards/voila#1249. But maybe there is a way to simplify all this.

Copy link
Member Author

@jtpio jtpio Mar 23, 2023

Choose a reason for hiding this comment

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

It's possible we need it because the stock @jupyter-widgets/jupyterlab-manager extension is not able to render the widgets found on the page if they don't come from a notebook (or document context).

Copy link
Member

Choose a reason for hiding this comment

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

Oh right that rings a bell

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Thanks!

@martinRenou martinRenou merged commit d73c5c3 into voila-dashboards:main Mar 23, 2023
@jtpio jtpio deleted the update-voila branch March 23, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants