Common interface to interpret and execute API methods #1016
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,92 +13,63 @@ var Plotly = require('../plotly'); | |
| var Lib = require('../lib'); | ||
|
|
||
| /* | ||
| * Create or update an observer. This function is designed to be | ||
| * idempotent so that it can be called over and over as the component | ||
| * updates, and will attach and detach listeners as needed. | ||
| * | ||
| * @param {optional object} container | ||
| * An object on which the observer is stored. This is the mechanism | ||
| * by which it is idempotent. If it already exists, another won't be | ||
| * added. Each time it's called, the value lookup table is updated. | ||
| * @param {array} commandList | ||
| * An array of commands, following either `buttons` of `updatemenus` | ||
| * or `steps` of `sliders`. | ||
| * @param {function} onchange | ||
| * A listener called when the value is changed. Receives data object | ||
| * with information about the new state. | ||
| */ | ||
| exports.createCommandObserver = function(gd, container, commandList, onchange) { | ||
| var ret = {}; | ||
| var enabled = true; | ||
|
|
||
| if(container && container._commandObserver) { | ||
| ret = container._commandObserver; | ||
| } | ||
|
|
||
| if(!ret.cache) { | ||
| ret.cache = {}; | ||
| } | ||
|
|
||
| // Either create or just recompute this: | ||
| ret.lookupTable = {}; | ||
|
|
||
| var binding = exports.hasSimpleAPICommandBindings(gd, commandList, ret.lookupTable); | ||
|
|
||
| if(container && container._commandObserver) { | ||
| if(!binding) { | ||
| // If container exists and there are no longer any bindings, | ||
| // remove existing: | ||
| if(container._commandObserver.remove) { | ||
| container._commandObserver.remove(); | ||
| container._commandObserver = null; | ||
| return ret; | ||
| } | ||
| } else { | ||
| // If container exists and there *are* bindings, then the lookup | ||
| // table should have been updated and check is already attached, | ||
| // so there's nothing to be done: | ||
| return ret; | ||
|
|
||
|
|
||
| } | ||
| } | ||
|
|
||
| // Determine whether there's anything to do for this binding: | ||
|
|
||
| if(binding) { | ||
| bindingValueHasChanged(gd, binding, ret.cache); | ||
|
|
||
| ret.check = function check() { | ||
| if(!enabled) return; | ||
|
|
||
| var container, value, obj; | ||
|
|
@@ -117,7 +88,7 @@ exports.createCommandObserver = function(gd, commandList, onchange) { | |
|
|
||
| value = Lib.nestedProperty(container, binding.prop).get(); | ||
|
|
||
| obj = ret.cache[binding.type] = ret.cache[binding.type] || {}; | ||
|
|
||
| if(obj.hasOwnProperty(binding.prop)) { | ||
| if(obj[binding.prop] !== value) { | ||
|
|
@@ -130,15 +101,15 @@ exports.createCommandObserver = function(gd, commandList, onchange) { | |
| if(changed && onchange) { | ||
| // Disable checks for the duration of this command in order to avoid | ||
| // infinite loops: | ||
| if(ret.lookupTable[value] !== undefined) { | ||
| ret.disable(); | ||
| Promise.resolve(onchange({ | ||
| value: value, | ||
| type: binding.type, | ||
| prop: binding.prop, | ||
| traces: binding.traces, | ||
| index: ret.lookupTable[value] | ||
| })).then(ret.enable, ret.enable); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -155,32 +126,111 @@ exports.createCommandObserver = function(gd, commandList, onchange) { | |
| ]; | ||
|
There was a problem hiding this comment. @etpinard I refactored this to just hook into a list of existing events. This seems much simpler and more straightforward. There was a problem hiding this comment. Very nicely done. Thank you |
||
|
|
||
| for(var i = 0; i < checkEvents.length; i++) { | ||
| gd._internalOn(checkEvents[i], ret.check); | ||
| } | ||
|
|
||
| ret.remove = function() { | ||
| for(var i = 0; i < checkEvents.length; i++) { | ||
| gd._removeInternalListener(checkEvents[i], ret.check); | ||
| } | ||
| }; | ||
| } else { | ||
| // TODO: It'd be really neat to actually give a *reason* for this, but at least a warning | ||
| // is a start | ||
| Lib.warn('Unable to automatically bind plot updates to API command'); | ||
|
|
||
| ret.lookupTable = {}; | ||
| ret.remove = function() {}; | ||
| } | ||
|
|
||
| ret.disable = function disable() { | ||
| enabled = false; | ||
| }; | ||
|
|
||
| ret.enable = function enable() { | ||
| enabled = true; | ||
| }; | ||
|
|
||
| if(container) { | ||
| container._commandObserver = ret; | ||
| } | ||
|
|
||
| return ret; | ||
| }; | ||
|
|
||
| /* | ||
| * This function checks to see if an array of objects containing | ||
| * method and args properties is compatible with automatic two-way | ||
| * binding. The criteria right now are that | ||
| * | ||
| * 1. multiple traces may be affected | ||
| * 2. only one property may be affected | ||
| * 3. the same property must be affected by all commands | ||
| */ | ||
| exports.hasSimpleAPICommandBindings = function(gd, commandList, bindingsByValue) { | ||
| var n = commandList.length; | ||
|
|
||
| var refBinding; | ||
|
|
||
| for(var i = 0; i < n; i++) { | ||
| var binding; | ||
| var command = commandList[i]; | ||
| var method = command.method; | ||
| var args = command.args; | ||
|
|
||
| // If any command has no method, refuse to bind: | ||
| if(!method) { | ||
| return false; | ||
| } | ||
| var bindings = exports.computeAPICommandBindings(gd, method, args); | ||
|
|
||
| // Right now, handle one and *only* one property being set: | ||
| if(bindings.length !== 1) { | ||
| return false; | ||
| } | ||
|
|
||
| if(!refBinding) { | ||
| refBinding = bindings[0]; | ||
| if(Array.isArray(refBinding.traces)) { | ||
| refBinding.traces.sort(); | ||
| } | ||
| } else { | ||
| binding = bindings[0]; | ||
| if(binding.type !== refBinding.type) { | ||
| return false; | ||
| } | ||
| if(binding.prop !== refBinding.prop) { | ||
| return false; | ||
| } | ||
| if(Array.isArray(refBinding.traces)) { | ||
| if(Array.isArray(binding.traces)) { | ||
| binding.traces.sort(); | ||
| for(var j = 0; j < refBinding.traces.length; j++) { | ||
| if(refBinding.traces[j] !== binding.traces[j]) { | ||
| return false; | ||
| } | ||
| } | ||
| } else { | ||
| return false; | ||
| } | ||
| } else { | ||
| if(binding.prop !== refBinding.prop) { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| binding = bindings[0]; | ||
| var value = binding.value; | ||
| if(Array.isArray(value)) { | ||
| value = value[0]; | ||
| } | ||
| if(bindingsByValue) { | ||
| bindingsByValue[value] = i; | ||
| } | ||
| } | ||
|
|
||
| return refBinding; | ||
| }; | ||
|
|
||
| function bindingValueHasChanged(gd, binding, cache) { | ||
|
|
@@ -213,6 +263,17 @@ function bindingValueHasChanged(gd, binding, cache) { | |
| return changed; | ||
| } | ||
|
|
||
| /* | ||
| * Execute an API command. There's really not much to this; it just provides | ||
| * a common hook so that implementations don't need to be synchronized across | ||
| * multiple components with the ability to invoke API commands. | ||
| * | ||
| * @param {string} method | ||
| * The name of the plotly command to execute. Must be one of 'animate', | ||
| * 'restyle', 'relayout', 'update'. | ||
| * @param {array} args | ||
| * A list of arguments passed to the API command | ||
| */ | ||
| exports.executeAPICommand = function(gd, method, args) { | ||
| var apiMethod = Plotly[method]; | ||
|
|
||
|
|
@@ -223,6 +284,7 @@ exports.executeAPICommand = function(gd, method, args) { | |
|
|
||
| return apiMethod.apply(null, allArgs).catch(function(err) { | ||
| Lib.warn('API call to Plotly.' + method + ' rejected.', err); | ||
| return Promise.reject(err); | ||
| }); | ||
| }; | ||
|
|
||
|
|
@@ -243,9 +305,10 @@ exports.computeAPICommandBindings = function(gd, method, args) { | |
| bindings = computeAnimateBindings(gd, args); | ||
| break; | ||
| default: | ||
|
There was a problem hiding this comment. again here, There was a problem hiding this comment. I guess really it should |
||
| // This is the case where intelligent logic about what affects | ||
| // this command is not implemented. It causes no ill effects. | ||
| // For example, addFrames simply won't bind to a control component. | ||
| bindings = []; | ||
| } | ||
| return bindings; | ||
| }; | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'm big a fan of this new pattern here where one routine creates + updates command observer. This should make the command observer logic more easily reused in the future when we integrate it in other components 👍
One small suggestion, now that
createCommandObsevercan create and update a command observer, maybe we should rename it something likemanageCommandObserversimilar tomanageModebar?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.
Sounds good. I was going to go with
createOrUpdateCommandObserver, but that seemed excessive. Changing now.