diff --git a/examples/web1/index.js b/examples/web1/index.js index ee9d39e15d..fffa20d06b 100644 --- a/examples/web1/index.js +++ b/examples/web1/index.js @@ -31,7 +31,7 @@ document.addEventListener('DOMContentLoaded', function(event) { }, console.error.bind(console)) .then(function(view) { console.log(widgetType + ' view created'); - manager.display_view(null, view); + manager.display_view(view); return view; }, console.error.bind(console)); } diff --git a/examples/web1/manager.js b/examples/web1/manager.js index 36d31c8948..1bec8186fc 100644 --- a/examples/web1/manager.js +++ b/examples/web1/manager.js @@ -46,13 +46,10 @@ class WidgetManager extends ManagerBase { }); } - display_view(msg, view, options) { + display_view(view) { var that = this; - return Promise.resolve(view).then(function(view) { + return Promise.resolve(view).then(view => { LuminoWidget.attach(view.pWidget, that.el); - view.on('remove', function() { - console.log('View removed', view); - }); return view; }); } diff --git a/examples/web2/index.js b/examples/web2/index.js index 6f9e078e53..4020de196f 100644 --- a/examples/web2/index.js +++ b/examples/web2/index.js @@ -21,12 +21,14 @@ document.addEventListener('DOMContentLoaded', function(event) { var state = require('./widget_state.json'); var widgetarea = document.getElementsByClassName('widgetarea')[0]; var manager = new WidgetManager(widgetarea); - manager.set_state(state).then(function(models) { - manager.display_model( - undefined, - models.find(function(element) { - return element.model_id == '8621699ecc804983a612f09b7dfe806b'; - }) - ); - }); + manager + .set_state(state) + .then(models => + manager.create_view( + models.find( + element => element.model_id == '8621699ecc804983a612f09b7dfe806b' + ) + ) + ) + .then(view => manager.display_view(view)); }); diff --git a/examples/web2/manager.js b/examples/web2/manager.js index 36d31c8948..125527fba3 100644 --- a/examples/web2/manager.js +++ b/examples/web2/manager.js @@ -46,13 +46,10 @@ class WidgetManager extends ManagerBase { }); } - display_view(msg, view, options) { + display_view(view) { var that = this; return Promise.resolve(view).then(function(view) { LuminoWidget.attach(view.pWidget, that.el); - view.on('remove', function() { - console.log('View removed', view); - }); return view; }); } diff --git a/examples/web3/src/index.ts b/examples/web3/src/index.ts index eaab9cbd77..75568b37a9 100644 --- a/examples/web3/src/index.ts +++ b/examples/web3/src/index.ts @@ -3,6 +3,7 @@ import 'codemirror/lib/codemirror.css'; import 'codemirror/mode/python/python'; import 'font-awesome/css/font-awesome.css'; import { WidgetManager } from './manager'; +import * as lmWidget from '@lumino/widgets'; import { KernelManager, @@ -48,22 +49,21 @@ document.addEventListener('DOMContentLoaded', async function(event) { const widgetarea = document.getElementsByClassName( 'widgetarea' )[0] as HTMLElement; - const manager = new WidgetManager(kernel, widgetarea); + const manager = new WidgetManager(kernel); // Run backend code to create the widgets. You could also create the // widgets in the frontend, like the other widget examples demonstrate. const execution = kernel.requestExecute({ code: code }); - execution.onIOPub = (msg): void => { + execution.onIOPub = async (msg): Promise => { // If we have a display message, display the widget. if (KernelMessage.isDisplayDataMsg(msg)) { const widgetData: any = msg.content.data['application/vnd.jupyter.widget-view+json']; if (widgetData !== undefined && widgetData.version_major === 2) { - const model = manager.get_model(widgetData.model_id); + const model = await manager.get_model(widgetData.model_id); if (model !== undefined) { - model.then(model => { - manager.display_model(msg, model); - }); + const view = await manager.create_view(model); + lmWidget.Widget.attach(view.pWidget, widgetarea); } } } diff --git a/examples/web3/src/manager.ts b/examples/web3/src/manager.ts index d99d275ee6..8aa6e25a22 100644 --- a/examples/web3/src/manager.ts +++ b/examples/web3/src/manager.ts @@ -1,18 +1,15 @@ import * as base from '@jupyter-widgets/base'; -import * as pWidget from '@lumino/widgets'; import { Kernel } from '@jupyterlab/services'; import { HTMLManager } from '@jupyter-widgets/html-manager'; import './widgets.css'; -import { DOMWidgetView } from '@jupyter-widgets/base'; export class WidgetManager extends HTMLManager { - constructor(kernel: Kernel.IKernelConnection, el: HTMLElement) { + constructor(kernel: Kernel.IKernelConnection) { super(); this.kernel = kernel; - this.el = el; kernel.registerCommTarget(this.comm_target_name, async (comm, msg) => { const oldComm = new base.shims.services.Comm(comm); @@ -20,21 +17,6 @@ export class WidgetManager extends HTMLManager { }); } - display_view( - msg: any, - view: DOMWidgetView, - options: any - ): Promise { - return Promise.resolve(view).then(view => { - pWidget.Widget.attach(view.pWidget, this.el); - view.on('remove', function() { - console.log('view removed', view); - }); - // We will resolve this lie in another PR. - return view as any; - }); - } - /** * Create a comm. */ @@ -61,5 +43,4 @@ export class WidgetManager extends HTMLManager { } kernel: Kernel.IKernelConnection; - el: HTMLElement; } diff --git a/packages/base-manager/src/manager-base.ts b/packages/base-manager/src/manager-base.ts index 313c293cd3..7c429b601e 100644 --- a/packages/base-manager/src/manager-base.ts +++ b/packages/base-manager/src/manager-base.ts @@ -55,33 +55,7 @@ export interface IBase64Buffers extends PartialJSONObject { /** * Manager abstract base class */ -export abstract class ManagerBase implements IWidgetManager { - /** - * Display a DOMWidgetView for a particular model. - */ - display_model( - msg: services.KernelMessage.IMessage | null, - model: DOMWidgetModel, - options: any = {} - ): Promise { - return this.create_view(model, options) - .then(view => this.display_view(msg, view, options)) - .catch(reject('Could not create view', true)); - } - - /** - * Display a DOMWidget view. - * - * #### Notes - * This must be implemented by a subclass. The implementation must trigger the view's displayed - * event after the view is on the page: `view.trigger('displayed')` - */ - abstract display_view( - msg: services.KernelMessage.IMessage | null, - view: DOMWidgetView, - options: any - ): Promise; - +export abstract class ManagerBase implements IWidgetManager { /** * Modifies view options. Generally overloaded in custom widget manager * implementations. @@ -93,8 +67,12 @@ export abstract class ManagerBase implements IWidgetManager { /** * Creates a promise for a view of a given model * + * #### Notes + * The implementation must trigger the Lumino 'after-attach' and 'after-show' events when appropriate, which in turn will trigger the view's 'displayed' events. + * * Make sure the view creation is not out of order with * any state updates. + * */ create_view( model: DOMWidgetModel, @@ -108,38 +86,38 @@ export abstract class ManagerBase implements IWidgetManager { model: WidgetModel, options = {} ): Promise { - const viewPromise = (model.state_change = model.state_change.then(() => { - return this.loadClass( - model.get('_view_name'), - model.get('_view_module'), - model.get('_view_module_version') - ) - .then((ViewType: typeof WidgetView) => { - const view: WidgetView = new ViewType({ + const id = uuid(); + const viewPromise = (model.state_change = model.state_change.then( + async () => { + try { + const ViewType = (await this.loadClass( + model.get('_view_name'), + model.get('_view_module'), + model.get('_view_module_version') + )) as typeof WidgetView; + const view = new ViewType({ model: model, options: this.setViewOptions(options) }); view.listenTo(model, 'destroy', view.remove); - return Promise.resolve(view.render()).then(() => { - return view; + await view.render(); + + // This presumes the view is added to the list of model views below + view.once('remove', () => { + delete model.views[id]; }); - }) - .catch( - reject('Could not create a view for model id ' + model.model_id, true) - ); - })); - const id = uuid(); + + return view; + } catch (e) { + console.error( + `Could not create a view for model id ${model.model_id}` + ); + throw e; + } + } + )); model.views[id] = viewPromise; - viewPromise.then(view => { - view.once( - 'remove', - () => { - delete view.model.views[id]; - }, - this - ); - }); - return model.state_change; + return viewPromise; } /** diff --git a/packages/base-manager/test/src/dummy-manager.ts b/packages/base-manager/test/src/dummy-manager.ts index f705de8026..492412b516 100644 --- a/packages/base-manager/test/src/dummy-manager.ts +++ b/packages/base-manager/test/src/dummy-manager.ts @@ -2,7 +2,6 @@ // Distributed under the terms of the Modified BSD License. import * as widgets from '@jupyter-widgets/base'; -import * as services from '@jupyterlab/services'; import * as Backbone from 'backbone'; import { ManagerBase } from '../../lib'; @@ -139,26 +138,12 @@ const testWidgets = { BinaryWidgetView }; -export class DummyManager extends ManagerBase { +export class DummyManager extends ManagerBase { constructor() { super(); this.el = window.document.createElement('div'); } - display_view( - msg: services.KernelMessage.IMessage, - view: Backbone.View, - options: any - ): Promise { - // TODO: make this a spy - // TODO: return an html element - return Promise.resolve(view).then(view => { - this.el.appendChild(view.el); - view.on('remove', () => console.log('view removed', view)); - return view.el; - }); - } - protected loadClass( className: string, moduleName: string, diff --git a/packages/base-manager/test/src/manager_test.ts b/packages/base-manager/test/src/manager_test.ts index 871d1b1327..2d0f558fff 100644 --- a/packages/base-manager/test/src/manager_test.ts +++ b/packages/base-manager/test/src/manager_test.ts @@ -33,26 +33,6 @@ describe('ManagerBase', function() { }); }); - describe('display_model', function() { - it('exists', function() { - expect(this.managerBase.display_model).to.not.be.undefined; - }); - it('calls create_view and display_view', async function() { - const manager = this.managerBase; - sinon.spy(manager, 'create_view'); - sinon.spy(manager, 'display_view'); - const msg = { msg: true }; - const viewOptions = { viewOptions: true }; - const model = await manager.new_model(this.modelOptions); - await manager.display_model(msg, model, viewOptions); - expect(manager.create_view.calledWith(model, viewOptions)).to.be.true; - expect(manager.display_view.calledWith(msg)).to.be.true; - expect(manager.display_view.getCall(0).args[2]).to.deep.equal( - viewOptions - ); - }); - }); - describe('setViewOptions', function() { it('returns an object', function() { expect(this.managerBase.setViewOptions()).to.deep.equal({}); diff --git a/packages/controls/test/src/dummy-manager.ts b/packages/controls/test/src/dummy-manager.ts index 639a784db1..b59313fb4f 100644 --- a/packages/controls/test/src/dummy-manager.ts +++ b/packages/controls/test/src/dummy-manager.ts @@ -2,7 +2,6 @@ // Distributed under the terms of the Modified BSD License. import * as widgets from '../../lib'; -import * as services from '@jupyterlab/services'; import * as Backbone from 'backbone'; import * as base from '@jupyter-widgets/base'; import { WidgetModel, WidgetView } from '@jupyter-widgets/base'; @@ -83,26 +82,12 @@ class TestWidgetView extends base.WidgetView { const testWidgets = { TestWidget, TestWidgetView }; -export class DummyManager extends ManagerBase { +export class DummyManager extends ManagerBase { constructor() { super(); this.el = window.document.createElement('div'); } - display_view( - msg: services.KernelMessage.IMessage, - view: Backbone.View, - options: any - ): Promise { - // TODO: make this a spy - // TODO: return an html element - return Promise.resolve(view).then(view => { - this.el.appendChild(view.el); - view.on('remove', () => console.log('view removed', view)); - return view.el; - }); - } - protected loadClass( className: string, moduleName: string, diff --git a/packages/html-manager/src/htmlmanager.ts b/packages/html-manager/src/htmlmanager.ts index b7567a1368..c9c70649e5 100644 --- a/packages/html-manager/src/htmlmanager.ts +++ b/packages/html-manager/src/htmlmanager.ts @@ -13,9 +13,9 @@ import { } from '@jupyterlab/rendermime'; import { WidgetRenderer, WIDGET_MIMETYPE } from './output_renderers'; -import { WidgetModel, WidgetView } from '@jupyter-widgets/base'; +import { WidgetModel, WidgetView, DOMWidgetView } from '@jupyter-widgets/base'; -export class HTMLManager extends ManagerBase { +export class HTMLManager extends ManagerBase { constructor(options?: { loader?: (moduleName: string, moduleVersion: string) => Promise; }) { @@ -37,18 +37,11 @@ export class HTMLManager extends ManagerBase { * Display the specified view. Element where the view is displayed * is specified in the `options.el` argument. */ - display_view( - msg: any, - view: any, - options: { el: HTMLElement } - ): Promise { - return Promise.resolve(view).then(view => { - LuminoWidget.Widget.attach(view.pWidget, options.el); - view.on('remove', () => { - console.log('View removed', view); - }); - return view; - }); + async display_view( + view: Promise | DOMWidgetView, + el: HTMLElement + ): Promise { + LuminoWidget.Widget.attach((await view).pWidget, el); } /** diff --git a/packages/html-manager/src/libembed.ts b/packages/html-manager/src/libembed.ts index 97123d5e1a..a5a3b114ad 100644 --- a/packages/html-manager/src/libembed.ts +++ b/packages/html-manager/src/libembed.ts @@ -31,16 +31,18 @@ const view_validate = ajv.compile(widget_view_schema); * @param managerFactory A function that returns a new HTMLManager * @param element (default document.documentElement) The document element in which to process for widget state. */ -export function renderWidgets( +export async function renderWidgets( managerFactory: () => HTMLManager, element: HTMLElement = document.documentElement -): void { +): Promise { const tags = element.querySelectorAll( 'script[type="application/vnd.jupyter.widget-state+json"]' ); - for (let i = 0; i != tags.length; ++i) { - renderManager(element, JSON.parse(tags[i].innerHTML), managerFactory); - } + await Promise.all( + Array.from(tags).map(async t => + renderManager(element, JSON.parse(t.innerHTML), managerFactory) + ) + ); } /** @@ -57,47 +59,44 @@ export function renderWidgets( * Additionally, if the script tag has a prior img sibling with class * 'jupyter-widget', then that img tag is deleted. */ -function renderManager( +async function renderManager( element: HTMLElement, widgetState: any, managerFactory: () => HTMLManager -): void { +): Promise { const valid = model_validate(widgetState); if (!valid) { - console.error('Model state has errors.', model_validate.errors); + throw new Error(`Model state has errors: ${model_validate.errors}`); } const manager = managerFactory(); - manager.set_state(widgetState).then(function(models) { - const tags = element.querySelectorAll( - 'script[type="application/vnd.jupyter.widget-view+json"]' - ); - for (let i = 0; i != tags.length; ++i) { - const viewtag = tags[i]; + const models = await manager.set_state(widgetState); + const tags = element.querySelectorAll( + 'script[type="application/vnd.jupyter.widget-view+json"]' + ); + await Promise.all( + Array.from(tags).map(async viewtag => { const widgetViewObject = JSON.parse(viewtag.innerHTML); const valid = view_validate(widgetViewObject); if (!valid) { - console.error('View state has errors.', view_validate.errors); + throw new Error(`View state has errors: ${view_validate.errors}`); } const model_id: string = widgetViewObject.model_id; - // Find the model id in the models. We should use .find, but IE - // doesn't support .find - const model = models.filter(item => { - return item.model_id == model_id; - })[0]; - if (model !== undefined) { + const model = models.find(item => item.model_id == model_id); + if (model !== undefined && viewtag.parentElement !== null) { const prev = viewtag.previousElementSibling; if ( prev && prev.tagName === 'img' && prev.classList.contains('jupyter-widget') ) { - viewtag.parentElement?.removeChild(prev); + viewtag.parentElement.removeChild(prev); } const widgetTag = document.createElement('div'); widgetTag.className = 'widget-subarea'; - viewtag.parentElement?.insertBefore(widgetTag, viewtag); - manager.display_model(null, model, { el: widgetTag }); + viewtag.parentElement.insertBefore(widgetTag, viewtag); + const view = await manager.create_view(model); + manager.display_view(view, widgetTag); } - } - }); + }) + ); } diff --git a/packages/html-manager/src/output_renderers.ts b/packages/html-manager/src/output_renderers.ts index 16ebf08ad9..5239caca3b 100644 --- a/packages/html-manager/src/output_renderers.ts +++ b/packages/html-manager/src/output_renderers.ts @@ -23,7 +23,8 @@ export class WidgetRenderer extends Widget implements IRenderMime.IRenderer { if (modelPromise) { try { const wModel = await modelPromise; - await this._manager.display_model(null, wModel, { el: this.node }); + const wView = await this._manager.create_view(wModel); + Widget.attach(wView.pWidget, this.node); } catch (err) { console.log('Error displaying widget'); console.log(err); @@ -33,7 +34,6 @@ export class WidgetRenderer extends Widget implements IRenderMime.IRenderer { } else { this.node.textContent = 'Error creating widget: could not find model'; this.addClass('jupyter-widgets'); - return Promise.resolve(); } } diff --git a/packages/html-manager/test/src/output_test.ts b/packages/html-manager/test/src/output_test.ts index 8cd7b35d93..008cb7533c 100644 --- a/packages/html-manager/test/src/output_test.ts +++ b/packages/html-manager/test/src/output_test.ts @@ -20,7 +20,7 @@ const newWidget = async (modelState: any): Promise => { model_module_version: '*' }; const model = await manager.new_model(modelCreate, modelState); - await manager.display_model(null, model, { el: widgetTag }); + await manager.display_view(manager.create_view(model), widgetTag); return widgetTag; }; @@ -130,10 +130,12 @@ describe('Output widget', function() { model_module_version: '*' }; const model = await manager.new_model(modelCreate, modelState); - await manager.display_model(null, model, { el: elt }); - // Pause one more time to give the asynchronous output renderer time - // to render the widgets. - await Promise.resolve(); + await manager.display_view(manager.create_view(model), elt); + + // Give the widget time to render + await new Promise(resolve => { + setTimeout(resolve, 20); + }); expect(elt.querySelectorAll('.slider').length).to.equal(1); }); @@ -192,7 +194,7 @@ describe('Output widget', function() { ] }; const model = await manager.new_model(modelCreate, modelState); - await manager.display_model(null, model, { el: widgetTag }); + await manager.display_view(manager.create_view(model), widgetTag); expect(widgetTag.innerText).to.equal('something different'); }); }); diff --git a/packages/jupyterlab-manager/src/manager.ts b/packages/jupyterlab-manager/src/manager.ts index c1b306339a..b2ea06247e 100644 --- a/packages/jupyterlab-manager/src/manager.ts +++ b/packages/jupyterlab-manager/src/manager.ts @@ -1,9 +1,6 @@ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. -// Just need typings -import * as Backbone from 'backbone'; - import { shims, IClassicComm, @@ -26,8 +23,6 @@ import { IDisposable } from '@lumino/disposable'; import { PromiseDelegate, ReadonlyPartialJSONValue } from '@lumino/coreutils'; -import { Widget } from '@lumino/widgets'; - import { INotebookModel } from '@jupyterlab/notebook'; import { IRenderMimeRegistry } from '@jupyterlab/rendermime'; @@ -53,43 +48,10 @@ export const WIDGET_VIEW_MIMETYPE = 'application/vnd.jupyter.widget-view+json'; export const WIDGET_STATE_MIMETYPE = 'application/vnd.jupyter.widget-state+json'; -/** - * The class name added to an BackboneViewWrapper widget. - */ -const BACKBONEVIEWWRAPPER_CLASS = 'jp-BackboneViewWrapper'; - -export class BackboneViewWrapper extends Widget { - /** - * Construct a new `Backbone` wrapper widget. - * - * @param view - The `Backbone.View` instance being wrapped. - */ - constructor(view: Backbone.View) { - super(); - this._view = view; - view.on('remove', () => { - this.dispose(); - }); - this.addClass(BACKBONEVIEWWRAPPER_CLASS); - this.node.appendChild(view.el); - } - - onAfterAttach(msg: any): void { - this._view.trigger('displayed'); - } - - dispose(): void { - this._view = null!; - super.dispose(); - } - - private _view: Backbone.View; -} - /** * A widget manager that returns Lumino widgets. */ -export abstract class LabWidgetManager extends ManagerBase +export abstract class LabWidgetManager extends ManagerBase implements IDisposable { constructor(rendermime: IRenderMimeRegistry) { super(); @@ -219,17 +181,6 @@ export abstract class LabWidgetManager extends ManagerBase ); } - /** - * Return a Lumino widget representing the view - */ - async display_view( - msg: any, - view: Backbone.View, - options: any - ): Promise { - return (view as any).pWidget || new BackboneViewWrapper(view); - } - /** * Create a comm. */ diff --git a/packages/jupyterlab-manager/src/renderer.ts b/packages/jupyterlab-manager/src/renderer.ts index a8a8e90076..f9d410f42e 100644 --- a/packages/jupyterlab-manager/src/renderer.ts +++ b/packages/jupyterlab-manager/src/renderer.ts @@ -5,12 +5,12 @@ import { PromiseDelegate } from '@lumino/coreutils'; import { IDisposable } from '@lumino/disposable'; -import { Panel, Widget } from '@lumino/widgets'; +import { Panel, Widget as LuminoWidget } from '@lumino/widgets'; import { IRenderMime } from '@jupyterlab/rendermime-interfaces'; import { WidgetManager } from './manager'; -import { WidgetModel } from '@jupyter-widgets/base'; +import { DOMWidgetModel } from '@jupyter-widgets/base'; /** * A renderer for widgets. @@ -47,9 +47,10 @@ export class WidgetRenderer extends Panel return Promise.resolve(); } - let wModel: WidgetModel; + let wModel: DOMWidgetModel; try { - wModel = await manager.get_model(source.model_id); + // Presume we have a DOMWidgetModel. Should we check for sure? + wModel = (await manager.get_model(source.model_id)) as DOMWidgetModel; } catch (err) { if (manager.restoredStatus) { // The manager has been restored, so this error won't be going away. @@ -67,9 +68,9 @@ export class WidgetRenderer extends Panel // Successful getting the model, so we don't need to try to rerender. this._rerenderMimeModel = null; - let widget: Widget; + let widget: LuminoWidget; try { - widget = await manager.display_model(null, wModel, undefined); + widget = (await manager.create_view(wModel)).pWidget; } catch (err) { this.node.textContent = 'Error displaying widget'; this.addClass('jupyter-widgets'); diff --git a/widgetsnbextension/src/extension.js b/widgetsnbextension/src/extension.js index a2f398dc8c..f9c24ffc36 100644 --- a/widgetsnbextension/src/extension.js +++ b/widgetsnbextension/src/extension.js @@ -127,7 +127,7 @@ function register_events(Jupyter, events, outputarea) { if (model) { model .then(function(model) { - return manager.display_model(void 0, model, { output: output }); + return manager.create_view(model, { output: output }); }) .then(function(view) { var id = view.cid; diff --git a/widgetsnbextension/src/manager.js b/widgetsnbextension/src/manager.js index 78dd66eb49..5bbc446b59 100644 --- a/widgetsnbextension/src/manager.js +++ b/widgetsnbextension/src/manager.js @@ -322,10 +322,6 @@ export class WidgetManager extends ManagerBase { return item; } - display_view(msg, view, options) { - return Promise.resolve(view); - } - _create_comm(comm_target_name, comm_id, data, metadata, buffers) { var that = this; return this._get_connected_kernel().then(function(kernel) {