From 9643aa5ddb09bd427f36eabb559310e09fc9b170 2014-12-09 01:18:10 From: Jonathan Frederic Date: 2014-12-09 01:18:10 Subject: [PATCH] Address @jasongrout 's review comments, take 2 --- diff --git a/IPython/html/static/widgets/js/manager.js b/IPython/html/static/widgets/js/manager.js index 3bf051d..13b2464 100644 --- a/IPython/html/static/widgets/js/manager.js +++ b/IPython/html/static/widgets/js/manager.js @@ -88,38 +88,32 @@ define([ /** * Displays a view for a particular model. */ - var that = this; - return new Promise(function(resolve, reject) { - var cell = that.get_msg_cell(msg.parent_header.msg_id); - if (cell === null) { - reject(new Error("Could not determine where the display" + - " message was from. Widget will not be displayed")); - } else { - return that.display_view_in_cell(cell, model) - .catch(function(error) { - reject(new utils.WrappedError('View could not be displayed.', error)); - }); - } - }); + var cell = this.get_msg_cell(msg.parent_header.msg_id); + if (cell === null) { + return Promise.reject(new Error("Could not determine where the display" + + " message was from. Widget will not be displayed")); + } else { + return this.display_view_in_cell(cell, model) + .catch(utils.reject('View could not be displayed.', true)); + } }; WidgetManager.prototype.display_view_in_cell = function(cell, model) { // Displays a view in a cell. - var that = this; - return new Promise(function(resolve, reject) { - if (cell.display_widget_view) { - cell.display_widget_view(that.create_view(model, {cell: cell})) - .then(function(view) { - that._handle_display_view(view); - view.trigger('displayed'); - resolve(view); - }, function(error) { - reject(new utils.WrappedError('Could not create or display view', error)); - }); - } else { - reject(new Error('Cell does not have a `display_widget_view` method')); - } - }); + if (cell.display_widget_view) { + var that = this; + return cell.display_widget_view(this.create_view(model, { + cell: cell, + // Only set cell_index when view is displayed as directly. + cell_index: that.notebook.find_cell_index(cell), + })).then(function(view) { + that._handle_display_view(view); + view.trigger('displayed'); + resolve(view); + }).catch(utils.reject('Could not create or display view', true)); + } else { + return Promise.reject(new Error('Cell does not have a `display_widget_view` method')); + } }; WidgetManager.prototype._handle_display_view = function (view) { @@ -315,7 +309,7 @@ define([ // Dictionary of options with the following contents: // only_displayed: (optional) boolean=false // Only return models with one or more displayed views. - // not_alive: (optional) boolean=false + // not_live: (optional) boolean=false // Include models that have comms with severed connections. // // Returns @@ -331,8 +325,8 @@ define([ // If the model has one or more views defined for it, // consider it displayed. var displayed_flag = !(options && options.only_displayed) || Object.keys(model.views).length > 0; - var alive_flag = (options && options.not_alive) || model.comm_alive; - if (displayed_flag && alive_flag) { + var live_flag = (options && options.not_live) || model.comm_live; + if (displayed_flag && live_flag) { state[model_id] = { model_name: model.name, model_module: model.module, @@ -344,13 +338,8 @@ define([ for (var id in model.views) { if (model.views.hasOwnProperty(id)) { var view = model.views[id]; - var cell = view.options.cell; - - // Only store the cell reference if this view is a top level - // child of the cell. - if (cell.widget_views.indexOf(view) != -1) { - var cell_index = that.notebook.find_cell_index(cell); - state[model_id].views.push(cell_index); + if (view.options.cell_index) { + state[model_id].views.push(view.options.cell_index); } } } @@ -358,7 +347,7 @@ define([ } } return state; - }); + }).catch(utils.reject('Could not get state of widget manager', true)); }; WidgetManager.prototype.set_state = function(state) { @@ -383,7 +372,7 @@ define([ kernel.comm_manager.register_comm(new_comm); // Create the model using the recreated comm. When the model is - // created we don't know yet if the comm is valid so set_comm_alive + // created we don't know yet if the comm is valid so set_comm_live // false. Once we receive the first state push from the back-end // we know the comm is alive. var views = kernel.widget_manager.create_model({ @@ -392,12 +381,12 @@ define([ model_module: state[model_id].model_module}) .then(function(model) { - model.set_comm_alive(false); + model.set_comm_live(false); var view_promise = Promise.resolve().then(function() { return model.set_state(state[model.id].state); }).then(function() { model.request_state().then(function() { - model.set_comm_alive(true); + model.set_comm_live(true); }); // Display the views of the model. diff --git a/IPython/html/static/widgets/js/widget.js b/IPython/html/static/widgets/js/widget.js index 7cf0fe1..01f5007 100644 --- a/IPython/html/static/widgets/js/widget.js +++ b/IPython/html/static/widgets/js/widget.js @@ -31,6 +31,7 @@ define(["widgets/js/manager", this.state_lock = null; this.id = model_id; this.views = {}; + this._resolve_received_state = {}; if (comm !== undefined) { // Remember comm associated with the model. @@ -42,9 +43,9 @@ define(["widgets/js/manager", comm.on_msg($.proxy(this._handle_comm_msg, this)); // Assume the comm is alive. - this.set_comm_alive(true); + this.set_comm_live(true); } else { - this.set_comm_alive(false); + this.set_comm_live(false); } return Backbone.Model.apply(this); }, @@ -69,24 +70,24 @@ define(["widgets/js/manager", return; } + var msg_id = this.comm.send({method: 'request_state'}, callbacks || this.widget_manager.callbacks()); + // Promise that is resolved when a state is received // from the back-end. var that = this; var received_state = new Promise(function(resolve) { - that._resolve_received_state = resolve; + that._resolve_received_state[msg_id] = resolve; }); - - this.comm.send({method: 'request_state'}, callbacks || this.widget_manager.callbacks()); return received_state; }, - set_comm_alive: function(alive) { + set_comm_live: function(live) { /** - * Change the comm_alive state of the model. + * Change the comm_live state of the model. */ - if (this.comm_alive === undefined || this.comm_alive != alive) { - this.comm_alive = alive; - this.trigger(alive ? 'comm_is_live' : 'comm_is_dead', {model: this}); + if (this.comm_live === undefined || this.comm_live != live) { + this.comm_live = live; + this.trigger(live ? 'comm:live' : 'comm:dead', {model: this}); } }, @@ -126,9 +127,17 @@ define(["widgets/js/manager", var that = this; switch (method) { case 'update': - this.state_change = this.state_change.then(function() { - return that.set_state(msg.content.data.state); - }).catch(utils.reject("Couldn't process update msg for model id '" + String(that.id) + "'", true)); + this.state_change = this.state_change + .then(function() { + return that.set_state(msg.content.data.state); + }).catch(utils.reject("Couldn't process update msg for model id '" + String(that.id) + "'", true)) + .then(function() { + var parent_id = msg.parent_header.msg_id; + if (that._resolve_received_state[parent_id] !== undefined) { + that._resolve_received_state[parent_id].call(); + delete that._resolve_received_state[parent_id]; + } + }).catch(utils.reject("Couldn't resolve state request promise.", true)); break; case 'custom': this.trigger('msg:custom', msg.content.data.content); @@ -150,11 +159,6 @@ define(["widgets/js/manager", } finally { that.state_lock = null; } - - if (that._resolve_received_state !== undefined) { - that._resolve_received_state(); - } - return Promise.resolve(); }, utils.reject("Couldn't set model state", true)); },