Skip to content

Common interface to interpret and execute API methods #1016

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

Merged
merged 29 commits into from
Oct 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f2d268d
Start implementing command execution wrapper
rreusser Oct 4, 2016
65f618d
Write failing tests for binding computation
rreusser Oct 6, 2016
7ef0404
Add a lot of tests for binding computation
rreusser Oct 10, 2016
edb6773
Implement animate and update binding computation
rreusser Oct 10, 2016
f50fcaa
Switch return format of bindings to structured output
rreusser Oct 11, 2016
60dd634
Merge remote-tracking branch 'origin/master' into lib-commands
rreusser Oct 11, 2016
4bc8387
Command to decide when bindings are simple
rreusser Oct 17, 2016
93b8253
First working version of bindings
rreusser Oct 18, 2016
43dc3d4
Change the method name
rreusser Oct 18, 2016
65d24f3
Fix updatemenus bindings
rreusser Oct 18, 2016
d2696e7
Hook up animations to sliders via bindings
rreusser Oct 18, 2016
7db7f03
Clean up slider positioning code
rreusser Oct 20, 2016
57fcf96
Add mock for bindings
rreusser Oct 20, 2016
7db5f1f
emove custom plotmodified event
rreusser Oct 20, 2016
26ad1ff
Add binding baseline image
rreusser Oct 20, 2016
986b4dc
Fix irritating self-interaction when dragging slider
rreusser Oct 24, 2016
20ac69f
Fix lint issue
rreusser Oct 24, 2016
f993ed7
Change failure modes for command API
rreusser Oct 24, 2016
99d7c8e
Ugh linter again
rreusser Oct 24, 2016
f8c0094
Improve robustness of command bindings
rreusser Oct 24, 2016
6abec15
Test more behavior of bindings
rreusser Oct 24, 2016
5df94a7
Fix linter issue
rreusser Oct 24, 2016
d35ee35
Remove binding test file
rreusser Oct 24, 2016
a554cad
Add equivalent command API test for udpatemenus
rreusser Oct 24, 2016
e5a80ee
DRY up binding change check
rreusser Oct 24, 2016
45717b1
Add note about test failure
rreusser Oct 24, 2016
52de9e4
createCommandObserver --> manageCommandObserver
rreusser Oct 25, 2016
0c40b02
Remove hard-coded updatemenus active default
rreusser Oct 25, 2016
df2d5bb
Revert updatemenus initialization and fix sliders initialization
rreusser Oct 25, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/sliders/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ module.exports = {
active: {
valType: 'number',
role: 'info',
min: -10,
min: 0,
dflt: 0,
description: [
'Determines which button (by index starting from 0) is',
Expand Down
38 changes: 23 additions & 15 deletions src/components/sliders/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

var d3 = require('d3');

var Plotly = require('../../plotly');
var Plots = require('../../plots/plots');
var Lib = require('../../lib');
var Color = require('../color');
Expand Down Expand Up @@ -52,6 +51,9 @@ module.exports = function draw(gd) {
sliderGroups.exit().each(function(sliderOpts) {
d3.select(this).remove();

sliderOpts._commandObserver.remove();
delete sliderOpts._commandObserver;

Plots.autoMargin(gd, constants.autoMarginIdRoot + sliderOpts._index);
});

Expand All @@ -65,12 +67,20 @@ module.exports = function draw(gd) {
// If it has fewer than two options, it's not really a slider:
if(sliderOpts.steps.length < 2) return;

var gSlider = d3.select(this);

computeLabelSteps(sliderOpts);

Plots.manageCommandObserver(gd, sliderOpts, sliderOpts.steps, function(data) {
if(sliderOpts.active === data.index) return;
if(sliderOpts._dragging) return;

setActive(gd, gSlider, sliderOpts, data.index, false, true);
});

drawSlider(gd, d3.select(this), sliderOpts);

// makeInputProxy(gd, d3.select(this), sliderOpts);

});
};

Expand Down Expand Up @@ -227,7 +237,9 @@ function drawSlider(gd, sliderGroup, sliderOpts) {
// Position the rectangle:
Lib.setTranslate(sliderGroup, sliderOpts.lx + sliderOpts.pad.l, sliderOpts.ly + sliderOpts.pad.t);

setActive(gd, sliderGroup, sliderOpts, sliderOpts.active, false, false);
sliderGroup.call(setGripPosition, sliderOpts, sliderOpts.active / (sliderOpts.steps.length - 1), false);
sliderGroup.call(drawCurrentValue, sliderOpts);

}

function drawCurrentValue(sliderGroup, sliderOpts, valueOverride) {
Expand Down Expand Up @@ -371,19 +383,9 @@ function setActive(gd, sliderGroup, sliderOpts, index, doCallback, doTransition)
sliderGroup._nextMethod = {step: step, doCallback: doCallback, doTransition: doTransition};
sliderGroup._nextMethodRaf = window.requestAnimationFrame(function() {
var _step = sliderGroup._nextMethod.step;
var args = _step.args;
if(!_step.method) return;

sliderOpts._invokingCommand = true;
Plotly[_step.method](gd, args[0], args[1], args[2]).then(function() {
sliderOpts._invokingCommand = false;
}, function() {
sliderOpts._invokingCommand = false;

// This is not a disaster. Some methods like `animate` reject if interrupted
// and *should* nicely log a warning.
Lib.warn('Warning: Plotly.' + _step.method + ' was called and rejected.');
});
Plots.executeAPICommand(gd, _step.method, _step.args);

sliderGroup._nextMethod = null;
sliderGroup._nextMethodRaf = null;
Expand All @@ -405,13 +407,15 @@ function attachGripEvents(item, gd, sliderGroup, sliderOpts) {

var normalizedPosition = positionToNormalizedValue(sliderOpts, d3.mouse(node)[0]);
handleInput(gd, sliderGroup, sliderOpts, normalizedPosition, true);
sliderOpts._dragging = true;

$gd.on('mousemove', function() {
var normalizedPosition = positionToNormalizedValue(sliderOpts, d3.mouse(node)[0]);
handleInput(gd, sliderGroup, sliderOpts, normalizedPosition, false);
});

$gd.on('mouseup', function() {
sliderOpts._dragging = false;
grip.call(Color.fill, sliderOpts.bgcolor);
$gd.on('mouseup', null);
$gd.on('mousemove', null);
Expand Down Expand Up @@ -467,8 +471,12 @@ function setGripPosition(sliderGroup, sliderOpts, position, doTransition) {

var x = normalizedValueToPosition(sliderOpts, position);

// If this is true, then *this component* is already invoking its own command
// and has triggered its own animation.
if(sliderOpts._invokingCommand) return;

var el = grip;
if(doTransition && sliderOpts.transition.duration > 0 && !sliderOpts._invokingCommand) {
if(doTransition && sliderOpts.transition.duration > 0) {
el = el.transition()
.duration(sliderOpts.transition.duration)
.ease(sliderOpts.transition.easing);
Expand Down
39 changes: 23 additions & 16 deletions src/components/updatemenus/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

var d3 = require('d3');

var Plotly = require('../../plotly');
var Plots = require('../../plots/plots');
var Lib = require('../../lib');
var Color = require('../color');
Expand All @@ -21,7 +20,6 @@ var anchorUtils = require('../legend/anchor_utils');

var constants = require('./constants');


module.exports = function draw(gd) {
var fullLayout = gd._fullLayout,
menuData = makeMenuData(fullLayout);
Expand Down Expand Up @@ -115,6 +113,11 @@ module.exports = function draw(gd) {
headerGroups.each(function(menuOpts) {
var gHeader = d3.select(this);

var _gButton = menuOpts.type === 'dropdown' ? gButton : null;
Plots.manageCommandObserver(gd, menuOpts, menuOpts.buttons, function(data) {
setActive(gd, menuOpts, menuOpts.buttons[data.index], gHeader, _gButton, data.index, true);
});

if(menuOpts.type === 'dropdown') {
drawHeader(gd, gHeader, gButton, menuOpts);

Expand Down Expand Up @@ -293,21 +296,9 @@ function drawButtons(gd, gHeader, gButton, menuOpts) {
.call(setItemPosition, menuOpts, posOpts);

button.on('click', function() {
// update 'active' attribute in menuOpts
menuOpts._input.active = menuOpts.active = buttonIndex;

// fold up buttons and redraw header
gButton.attr(constants.menuIndexAttrName, '-1');
setActive(gd, menuOpts, buttonOpts, gHeader, gButton, buttonIndex);

if(menuOpts.type === 'dropdown') {
drawHeader(gd, gHeader, gButton, menuOpts);
}

drawButtons(gd, gHeader, gButton, menuOpts);

// call button method
var args = buttonOpts.args;
Plotly[buttonOpts.method](gd, args[0], args[1], args[2]);
Plots.executeAPICommand(gd, buttonOpts.method, buttonOpts.args);
Copy link
Contributor

Choose a reason for hiding this comment

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

much cleaner 🎉

});

button.on('mouseover', function() {
Expand All @@ -326,6 +317,22 @@ function drawButtons(gd, gHeader, gButton, menuOpts) {
Lib.setTranslate(gButton, menuOpts.lx, menuOpts.ly);
}

function setActive(gd, menuOpts, buttonOpts, gHeader, gButton, buttonIndex, isSilentUpdate) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice abstraction

// update 'active' attribute in menuOpts
menuOpts._input.active = menuOpts.active = buttonIndex;

if(menuOpts.type === 'dropdown') {
// fold up buttons and redraw header
gButton.attr(constants.menuIndexAttrName, '-1');

drawHeader(gd, gHeader, gButton, menuOpts);
}

if(!isSilentUpdate || menuOpts.type === 'buttons') {
drawButtons(gd, gHeader, gButton, menuOpts);
}
}

function drawItem(item, menuOpts, itemOpts) {
item.call(drawItemRect, menuOpts)
.call(drawItemText, menuOpts, itemOpts);
Expand Down
18 changes: 10 additions & 8 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2302,14 +2302,7 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) {
var newFrame = trans._currentFrame = trans._frameQueue.shift();

if(newFrame) {
gd.emit('plotly_animatingframe', {
name: newFrame.name,
frame: newFrame.frame,
animation: {
frame: newFrame.frameOpts,
transition: newFrame.transitionOpts,
}
});
gd._fullLayout._currentFrame = newFrame.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice. I'll use that for layout.breakpoints


trans._lastFrameAt = Date.now();
trans._timeToNext = newFrame.frameOpts.duration;
Expand All @@ -2324,6 +2317,15 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) {
newFrame.frameOpts,
newFrame.transitionOpts
);

gd.emit('plotly_animatingframe', {
Copy link
Contributor

Choose a reason for hiding this comment

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

@rreusser can you share some info about why the emit('plotly_animatingframe') changed location?

Copy link
Contributor Author

@rreusser rreusser Oct 21, 2016

Choose a reason for hiding this comment

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

There are a lot of things taking place synchronously and asynchronously so that I wanted to move the event trigger as late as possible to ensure the changes have actually taken place in order to be picked up by bound components. I think maybe a better long-term solution is an event after Plots.transition has supplied defaults (coming from that method itself to ensure correctness) in order to ensure that the changes have absolutely 100% certainly been applied.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the info 👍

name: newFrame.name,
frame: newFrame.frame,
animation: {
frame: newFrame.frameOpts,
transition: newFrame.transitionOpts,
}
});
} else {
// If there are no more frames, then stop the RAF loop:
stopAnimationLoop();
Expand Down
Loading