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.

Sign up for GitHub

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

Common interface to interpret and execute API methods #1016

Show file tree
Hide file tree
Changes from 11 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
12 changes: 11 additions & 1 deletion src/components/sliders/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,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 +68,19 @@ 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);

if(!sliderOpts._commandObserver) {
sliderOpts._commandObserver = Plots.createCommandObserver(gd, sliderOpts.steps, function(data) {
setActive(gd, gSlider, sliderOpts, data.index, false, true);
});
}

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

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

});
};

Expand Down
36 changes: 24 additions & 12 deletions src/components/updatemenus/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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 +114,13 @@ module.exports = function draw(gd) {
headerGroups.each(function(menuOpts) {
var gHeader = d3.select(this);

if(!menuOpts._commandObserver) {
var _gButton = menuOpts.type === 'dropdown' ? gButton : null;
menuOpts._commandObserver = Plots.createCommandObserver(gd, menuOpts.buttons, function(data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@rreusser if I understand correctly, _commandObserver:

  • is attached once
  • relinked by relinkPrivateKeys in supply-defaults on update
  • knows how to update itself when menuOpts.buttons is modified e.g. via Plotly.relayout(gd, 'updatemenus[0].buttons[0].label', 'new label');

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,17 +299,7 @@ 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');

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

drawButtons(gd, gHeader, gButton, menuOpts);
setActive(gd, menuOpts, buttonOpts, gHeader, gButton, buttonIndex);

// call button method
var args = buttonOpts.args;
Expand All @@ -326,6 +322,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
7 changes: 7 additions & 0 deletions src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) {

return plotDone.then(function() {
gd.emit('plotly_restyle', specs.eventData);
gd.emit('plotly_plotmodified');
return gd;
});
};
Expand Down Expand Up @@ -1710,6 +1711,7 @@ Plotly.relayout = function relayout(gd, astr, val) {

return plotDone.then(function() {
gd.emit('plotly_relayout', specs.eventData);
gd.emit('plotly_plotmodified');
return gd;
});
};
Expand Down Expand Up @@ -2124,6 +2126,7 @@ Plotly.update = function update(gd, traceUpdate, layoutUpdate, traces) {
data: restyleSpecs.eventData,
layout: relayoutSpecs.eventData
});
gd.emit('plotly_plotmodified');
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just subscribe to the existing plotly_afterplot ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know about it! Just need to add that event in a couple more places, but that should be perfect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, other reason, now that I think about it:

Should afterplot be emitted between frames? The exact sequence definitely matters, so the other logic was that a unique event would free us from tight coupling.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should afterplot be emitted between frames?

Hmm. Very good question. I'd say no. plotly_afterplot should be the fired just before the promise is returned. But, I can be convinced otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it should be a separate event for this purpose? Again, my fear was coupling the component update code with events that serve other purposes. But I can try to use plotly_afterplot if you prefer. (though, again, it would need to be called during animation)

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no strong opinion here. I'll leave it up to you.


return gd;
});
Expand Down Expand Up @@ -2302,6 +2305,8 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) {
var newFrame = trans._currentFrame = trans._frameQueue.shift();

if(newFrame) {
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


gd.emit('plotly_animatingframe', {
name: newFrame.name,
frame: newFrame.frame,
Expand All @@ -2324,6 +2329,8 @@ Plotly.animate = function(gd, frameOrGroupNameOrFrameList, animationOpts) {
newFrame.frameOpts,
newFrame.transitionOpts
);

gd.emit('plotly_plotmodified');
} else {
// If there are no more frames, then stop the RAF loop:
stopAnimationLoop();
Expand Down
Loading