Skip to content
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

Consistent container update / removal #1086

Merged
merged 5 commits into from
Oct 26, 2016
Merged

Consistent container update / removal #1086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
133bf1a
plots: expose list of data / layout array containers
etpinard Oct 26, 2016
e2524ea
add api helpers for managing array container updates
etpinard Oct 26, 2016
66c3ca0
restyle: call manageArrayContainers on array-container updates
etpinard Oct 26, 2016
2cafbeb
relayout: merge all array container update logic into 1 block
etpinard Oct 26, 2016
010c792
relayout: add support for calls with `null` for annotation / shape
etpinard Oct 26, 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
48 changes: 48 additions & 0 deletions src/plot_api/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,51 @@ exports.coerceTraceIndices = function(gd, traceIndices) {

return traceIndices;
};

/**
* Manages logic around array container item creation / deletion / update
* that nested property along can't handle.
*
* @param {Object} np
* nested property of update attribute string about trace or layout object
* @param {*} newVal
* update value passed to restyle / relayout / update
* @param {Object} undoit
* undo hash (N.B. undoit may be mutated here).
*
*/
exports.manageArrayContainers = function(np, newVal, undoit) {
var obj = np.obj,
parts = np.parts,
pLength = parts.length,
pLast = parts[pLength - 1];

var pLastIsNumber = isNumeric(pLast);

// delete item
if(pLastIsNumber && newVal === null) {

// Clear item in array container when new value is null
var contPath = parts.slice(0, pLength - 1).join('.'),
cont = Lib.nestedProperty(obj, contPath).get();
cont.splice(pLast, 1);

// Note that nested property clears null / undefined at end of
// array container, but not within them.
}
// create item
else if(pLastIsNumber && np.get() === undefined) {

// When adding a new item, make sure undo command will remove it
if(np.get() === undefined) undoit[np.astr] = null;

np.set(newVal);
}
// update item
else {

// If the last part of attribute string isn't a number,
// np.set is all we need.
np.set(newVal);
}
};
54 changes: 13 additions & 41 deletions src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1414,9 +1414,6 @@ function _restyle(gd, aobj, _traces) {
continue;
}

// take no chances on transforms
if(ai.substr(0, 10) === 'transforms') flags.docalc = true;

// set attribute in gd.data
undoit[ai] = a0();
for(i = 0; i < traces.length; i++) {
Expand Down Expand Up @@ -1556,6 +1553,10 @@ function _restyle(gd, aobj, _traces) {
}
helpers.swapXYData(cont);
}
else if(Plots.dataArrayContainers.indexOf(param.parts[0]) !== -1) {
helpers.manageArrayContainers(param, newVal, undoit);
flags.docalc = true;
}
// all the other ones, just modify that one attribute
else param.set(newVal);

Expand Down Expand Up @@ -1815,8 +1816,7 @@ function _relayout(gd, aobj) {
// trunk nodes (everything except the leaf)
ptrunk = p.parts.slice(0, pend).join('.'),
parentIn = Lib.nestedProperty(gd.layout, ptrunk).get(),
parentFull = Lib.nestedProperty(fullLayout, ptrunk).get(),
diff;
parentFull = Lib.nestedProperty(fullLayout, ptrunk).get();

if(vi === undefined) continue;

Expand Down Expand Up @@ -1921,6 +1921,9 @@ function _relayout(gd, aobj) {
objList = layout[objType] || [],
obji = objList[objNum] || {};

// new API, remove annotation / shape with `null`
if(vi === null) aobj[ai] = 'remove';

// if p.parts is just an annotation number, and val is either
// 'add' or an entire annotation to add, the undo is 'remove'
// if val is 'remove' then undo is the whole annotation object
Expand Down Expand Up @@ -1950,42 +1953,11 @@ function _relayout(gd, aobj) {
drawOne(gd, objNum, p.parts.slice(2).join('.'), aobj[ai]);
delete aobj[ai];
}
else if(p.parts[0] === 'images') {
var update = Lib.objectFromPath(ai, vi);
Lib.extendDeepAll(gd.layout, update);

Registry.getComponentMethod('images', 'supplyLayoutDefaults')(gd.layout, gd._fullLayout);
Registry.getComponentMethod('images', 'draw')(gd);
}
else if(p.parts[0] === 'mapbox' && p.parts[1] === 'layers') {
Lib.extendDeepAll(gd.layout, Lib.objectFromPath(ai, vi));

// append empty container to mapbox.layers
// so that relinkPrivateKeys does not complain

var fullLayers = (gd._fullLayout.mapbox || {}).layers || [];
diff = (p.parts[2] + 1) - fullLayers.length;

for(i = 0; i < diff; i++) fullLayers.push({});

flags.doplot = true;
}
else if(p.parts[0] === 'updatemenus') {
Lib.extendDeepAll(gd.layout, Lib.objectFromPath(ai, vi));

var menus = gd._fullLayout.updatemenus || [];
diff = (p.parts[2] + 1) - menus.length;

for(i = 0; i < diff; i++) menus.push({});
flags.doplot = true;
}
else if(p.parts[0] === 'sliders') {
Lib.extendDeepAll(gd.layout, Lib.objectFromPath(ai, vi));

var sliders = gd._fullLayout.sliders || [];
diff = (p.parts[2] + 1) - sliders.length;

for(i = 0; i < diff; i++) sliders.push({});
else if(
Plots.layoutArrayContainers.indexOf(p.parts[0]) !== -1 ||
(p.parts[0] === 'mapbox' && p.parts[1] === 'layers')
) {
helpers.manageArrayContainers(p, vi, undoit);
flags.doplot = true;
}
// alter gd.layout
Expand Down
13 changes: 5 additions & 8 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,9 @@ plots.extendObjectWithContainers = function(dest, src, containerPaths) {
return dest;
};

plots.dataArrayContainers = ['transforms'];
plots.layoutArrayContainers = ['annotations', 'shapes', 'images', 'sliders', 'updatemenus'];

/*
* Extend a trace definition. This method:
*
Expand All @@ -1433,7 +1436,7 @@ plots.extendObjectWithContainers = function(dest, src, containerPaths) {
* The result is the original object reference with the new contents merged in.
*/
plots.extendTrace = function(destTrace, srcTrace) {
return plots.extendObjectWithContainers(destTrace, srcTrace, ['transforms']);
return plots.extendObjectWithContainers(destTrace, srcTrace, plots.dataArrayContainers);
};

/*
Expand All @@ -1446,13 +1449,7 @@ plots.extendTrace = function(destTrace, srcTrace) {
* The result is the original object reference with the new contents merged in.
*/
plots.extendLayout = function(destLayout, srcLayout) {
return plots.extendObjectWithContainers(destLayout, srcLayout, [
'annotations',
'shapes',
'images',
'sliders',
'updatemenus'
]);
return plots.extendObjectWithContainers(destLayout, srcLayout, plots.layoutArrayContainers);
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it. Nice refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I thought exposing plots.layoutArrayContainers was a nice compromise.

Next step would be to auto-generate that list using the plot-schema. (later ...)

};

/**
Expand Down
11 changes: 9 additions & 2 deletions test/jasmine/tests/annotations_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,18 @@ describe('annotations relayout', function() {
expect(countAnnotations()).toEqual(len + 1);

return Plotly.relayout(gd, 'annotations[' + 0 + ']', 'remove');
}).then(function() {
})
.then(function() {
expect(countAnnotations()).toEqual(len);

return Plotly.relayout(gd, 'annotations[' + 0 + ']', null);
})
.then(function() {
expect(countAnnotations()).toEqual(len - 1);

return Plotly.relayout(gd, { annotations: [] });
}).then(function() {
})
.then(function() {
expect(countAnnotations()).toEqual(0);

done();
Expand Down
10 changes: 7 additions & 3 deletions test/jasmine/tests/layout_images_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,22 @@ describe('Layout images', function() {
return Plotly.relayout(gd, 'images[2]', makeImage(pythonLogo, 0.2, 0.5));
}).then(function() {
assertImages(3);
expect(gd.layout.images.length).toEqual(3);

return Plotly.relayout(gd, 'images[2]', 'remove');
return Plotly.relayout(gd, 'images[2]', null);
}).then(function() {
assertImages(2);
expect(gd.layout.images.length).toEqual(2);

return Plotly.relayout(gd, 'images[1]', 'remove');
return Plotly.relayout(gd, 'images[1]', null);
}).then(function() {
assertImages(1);
expect(gd.layout.images.length).toEqual(1);

return Plotly.relayout(gd, 'images[0]', 'remove');
return Plotly.relayout(gd, 'images[0]', null);
}).then(function() {
assertImages(0);
expect(gd.layout.images).toEqual([]);

done();
});
Expand Down
44 changes: 40 additions & 4 deletions test/jasmine/tests/lib_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1561,23 +1561,27 @@ describe('Queue', function() {

Plotly.plot(gd, [{
y: [2, 1, 2]
}]).then(function() {
}])
.then(function() {
expect(gd.undoQueue).toBeUndefined();

return Plotly.restyle(gd, 'marker.color', 'red');
}).then(function() {
})
.then(function() {
expect(gd.undoQueue.index).toEqual(1);
expect(gd.undoQueue.queue[0].undo.args[0][1]['marker.color']).toEqual([undefined]);
expect(gd.undoQueue.queue[0].redo.args[0][1]['marker.color']).toEqual('red');

return Plotly.relayout(gd, 'title', 'A title');
}).then(function() {
})
.then(function() {
expect(gd.undoQueue.index).toEqual(2);
expect(gd.undoQueue.queue[1].undo.args[0][1].title).toEqual(undefined);
expect(gd.undoQueue.queue[1].redo.args[0][1].title).toEqual('A title');

return Plotly.restyle(gd, 'mode', 'markers');
}).then(function() {
})
.then(function() {
expect(gd.undoQueue.index).toEqual(2);
expect(gd.undoQueue.queue[2]).toBeUndefined();

Expand All @@ -1587,6 +1591,38 @@ describe('Queue', function() {
expect(gd.undoQueue.queue[0].undo.args[0][1].title).toEqual(undefined);
expect(gd.undoQueue.queue[0].redo.args[0][1].title).toEqual('A title');

return Plotly.restyle(gd, 'transforms[0]', { type: 'filter' });
})
.then(function() {
expect(gd.undoQueue.queue[1].undo.args[0][1])
.toEqual({ 'transforms[0]': null });
expect(gd.undoQueue.queue[1].redo.args[0][1])
.toEqual({ 'transforms[0]': { type: 'filter' } });

return Plotly.relayout(gd, 'updatemenus[0]', { buttons: [] });
})
.then(function() {
expect(gd.undoQueue.queue[1].undo.args[0][1])
.toEqual({ 'updatemenus[0]': null });
expect(gd.undoQueue.queue[1].redo.args[0][1])
.toEqual({ 'updatemenus[0]': { buttons: [] } });

return Plotly.relayout(gd, 'updatemenus[0]', null);
})
.then(function() {
expect(gd.undoQueue.queue[1].undo.args[0][1])
.toEqual({ 'updatemenus[0]': { buttons: []} });
expect(gd.undoQueue.queue[1].redo.args[0][1])
.toEqual({ 'updatemenus[0]': null });

return Plotly.restyle(gd, 'transforms[0]', null);
})
.then(function() {
expect(gd.undoQueue.queue[1].undo.args[0][1])
.toEqual({ 'transforms[0]': [ { type: 'filter' } ]});
expect(gd.undoQueue.queue[1].redo.args[0][1])
.toEqual({ 'transforms[0]': null });

done();
});
});
Expand Down
45 changes: 33 additions & 12 deletions test/jasmine/tests/mapbox_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,56 +548,77 @@ describe('mapbox plots', function() {
expect(countVisibleLayers(gd)).toEqual(0);

Plotly.relayout(gd, 'mapbox.layers[0]', layer0).then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(1);
expect(countVisibleLayers(gd)).toEqual(1);

return Plotly.relayout(gd, 'mapbox.layers[1]', layer1);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(2);
expect(countVisibleLayers(gd)).toEqual(2);

return Plotly.relayout(gd, mapUpdate);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(2);
expect(countVisibleLayers(gd)).toEqual(2);

return Plotly.relayout(gd, styleUpdate0);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(2);
expect(countVisibleLayers(gd)).toEqual(2);

return assertLayerStyle(gd, {
'fill-color': [1, 0, 0, 1],
'fill-outline-color': [0, 0, 1, 1],
'fill-opacity': 0.3
}, 0);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(2);
expect(countVisibleLayers(gd)).toEqual(2);

return Plotly.relayout(gd, styleUpdate1);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(2);
expect(countVisibleLayers(gd)).toEqual(2);

return assertLayerStyle(gd, {
'line-width': 3,
'line-color': [0, 0, 1, 1],
'line-opacity': 0.6
}, 1);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(2);
expect(countVisibleLayers(gd)).toEqual(2);

return Plotly.relayout(gd, 'mapbox.layers[1]', 'remove');
}).then(function() {
return Plotly.relayout(gd, 'mapbox.layers[1]', null);
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(1);
expect(countVisibleLayers(gd)).toEqual(1);

return Plotly.relayout(gd, 'mapbox.layers[0]', 'remove');
}).then(function() {
return Plotly.relayout(gd, 'mapbox.layers[0]', null);
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(0);
expect(countVisibleLayers(gd)).toEqual(0);

return Plotly.relayout(gd, 'mapbox.layers[0]', {});
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers).toEqual([]);
expect(countVisibleLayers(gd)).toEqual(0);

// layer with no source are not drawn

return Plotly.relayout(gd, 'mapbox.layers[0].source', layer0.source);
}).then(function() {
})
.then(function() {
expect(gd.layout.mapbox.layers.length).toEqual(1);
expect(countVisibleLayers(gd)).toEqual(1);

done();
Expand Down
Loading