-
Notifications
You must be signed in to change notification settings - Fork 300
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
Dynamically load IPyWidget 8 scripts #12608
Conversation
65a4d5c
to
5fa050d
Compare
const notebookComms = editors | ||
.filter((editor) => this.notebookCommunications.has(editor)) | ||
.map((editor) => this.notebookCommunications.get(editor)!); | ||
notebookComms.forEach((comm) => comm.changeController(e.controller)); |
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.
Why these changes?
@@ -118,18 +124,15 @@ export class NotebookIPyWidgetCoordinator implements IExtensionSyncActivationSer | |||
.forEach((editor) => { | |||
const comms = this.notebookCommunications.get(editor); | |||
this.notebookCommunications.delete(editor); | |||
if (comms) { | |||
comms.dispose(); | |||
if (comms && comms.controller !== e.controller.controller) { |
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.
Why these changes?
'ipywidgets.js' | ||
) | ||
); | ||
// scripts.push( |
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.
Update build scripts to ensure dev compile and release modes have the same paths, as this is a debt item and we'd like to remove this debt now.
5fa050d
to
b795a10
Compare
e41584b
to
e6b6782
Compare
For #8552
Related PR in widgets npm microsoft/vscode-jupyter-ipywidgets#19
TODO: