-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 1 commit
f2d268d
65f618d
7ef0404
edb6773
f50fcaa
60dd634
4bc8387
93b8253
43dc3d4
65d24f3
d2696e7
7db7f03
57fcf96
7db5f1f
26ad1ff
986b4dc
20ac69f
f993ed7
99d7c8e
f8c0094
6abec15
5df94a7
d35ee35
a554cad
e5a80ee
45717b1
52de9e4
0c40b02
df2d5bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,8 @@ module.exports = function updateMenusDefaults(layoutIn, layoutOut) { | |
| // used to determine object constancy | ||
| menuOut._index = i; | ||
|
|
||
| menuOut._input.active = menuOut.active; | ||
|
|
||
| contOut.push(menuOut); | ||
| } | ||
| }; | ||
|
|
@@ -48,7 +50,9 @@ function menuDefaults(menuIn, menuOut, layoutOut) { | |
| var visible = coerce('visible', buttons.length > 0); | ||
| if(!visible) return; | ||
|
|
||
| coerce('active'); | ||
| // Default to zero since active must be *something*: | ||
|
||
| coerce('active', 0); | ||
|
|
||
| coerce('direction'); | ||
| coerce('type'); | ||
| coerce('showactive'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -525,21 +525,21 @@ describe('attaching component bindings', function() { | |
| destroyGraphDiv(gd); | ||
| }); | ||
|
|
||
| it('attaches bindings when events are added', function(done) { | ||
| it('attaches and updates bindings for sliders', function(done) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @etpinard See tests. It checks:
|
||
| expect(gd._internalEv._events.plotly_animatingframe).toBeUndefined(); | ||
|
|
||
| Plotly.relayout(gd, { | ||
| sliders: [{ | ||
| // This one gets bindings: | ||
| steps: [ | ||
| {label: 'first', method: 'restyle', args: ['marker.color', 'red']}, | ||
| {label: 'first', method: 'restyle', args: ['marker.color', 'blue']}, | ||
| {label: 'second', method: 'restyle', args: ['marker.color', 'blue']}, | ||
| ] | ||
| }, { | ||
| // This one does *not*: | ||
| steps: [ | ||
| {label: 'first', method: 'restyle', args: ['line.color', 'red']}, | ||
| {label: 'first', method: 'restyle', args: ['marker.color', 'blue']}, | ||
| {label: 'second', method: 'restyle', args: ['marker.color', 'blue']}, | ||
| ] | ||
| }] | ||
| }).then(function() { | ||
|
|
@@ -577,4 +577,57 @@ describe('attaching component bindings', function() { | |
| expect(gd._internalEv._events.plotly_animatingframe).toBeUndefined(); | ||
| }).catch(fail).then(done); | ||
| }); | ||
|
|
||
| it('attaches and updates bindings for updatemenus', function(done) { | ||
| expect(gd._internalEv._events.plotly_animatingframe).toBeUndefined(); | ||
|
|
||
| Plotly.relayout(gd, { | ||
| updatemenus: [{ | ||
| // This one gets bindings: | ||
| buttons: [ | ||
| {label: 'first', method: 'restyle', args: ['marker.color', 'red']}, | ||
| {label: 'second', method: 'restyle', args: ['marker.color', 'blue']}, | ||
| ] | ||
| }, { | ||
| // This one does *not*: | ||
| buttons: [ | ||
| {label: 'first', method: 'restyle', args: ['line.color', 'red']}, | ||
| {label: 'second', method: 'restyle', args: ['marker.color', 'blue']}, | ||
| ] | ||
| }] | ||
| }).then(function() { | ||
| // Check that it has attached a listener: | ||
| expect(typeof gd._internalEv._events.plotly_animatingframe).toBe('function'); | ||
|
|
||
| // Confirm the first position is selected: | ||
| expect(gd.layout.updatemenus[0].active).toBe(0); | ||
|
|
||
| // Modify the plot | ||
| return Plotly.restyle(gd, {'marker.color': 'blue'}); | ||
| }).then(function() { | ||
| // Confirm that this has changed the slider position: | ||
| expect(gd.layout.updatemenus[0].active).toBe(1); | ||
|
|
||
| // Swap the values of the components: | ||
| return Plotly.relayout(gd, { | ||
| 'updatemenus[0].buttons[0].args[1]': 'green', | ||
| 'updatemenus[0].buttons[1].args[1]': 'red' | ||
| }); | ||
| }).then(function() { | ||
| return Plotly.restyle(gd, {'marker.color': 'green'}); | ||
| }).then(function() { | ||
| // Confirm that the lookup table has been updated: | ||
| expect(gd.layout.updatemenus[0].active).toBe(0); | ||
|
|
||
| // Check that it still has one attached listener: | ||
| expect(typeof gd._internalEv._events.plotly_animatingframe).toBe('function'); | ||
|
|
||
| // Change this to a non-simple binding: | ||
| return Plotly.relayout(gd, {'updatemenus[0].buttons[0].args[0]': 'line.color'}); | ||
| }).then(function() { | ||
| // Bindings are no longer simple, so check to ensure they have | ||
| // been removed | ||
| expect(gd._internalEv._events.plotly_animatingframe).toBeUndefined(); | ||
| }).catch(fail).then(done); | ||
| }); | ||
| }); | ||
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.
Hmm. Why do we need this?
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.
Without this, there's an intermediate state in which this value is updated when changes occur, but is
undefinedbefore that.I ran into this while testing: I checked this to see if the external value was updated when component interactions occurred. That worked fine, but before changes were made, the value was
undefined, which was perhaps desired but did not reflect the actual state.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.
Removed this line.