Skip to content

Commit ad864c4

Browse files
etpinardetpinard
authored
Merge pull request #1086 from plotly/container-removal
Consistent container update / removal
2 parents 4bda0bb + 010c792 commit ad864c4

File tree

11 files changed

+261
-81
lines changed

11 files changed

+261
-81
lines changed

‎src/plot_api/helpers.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,3 +430,51 @@ exports.coerceTraceIndices = function(gd, traceIndices) {
430430

431431
return traceIndices;
432432
};
433+
434+
/**
435+
* Manages logic around array container item creation / deletion / update
436+
* that nested property along can't handle.
437+
*
438+
* @param {Object} np
439+
* nested property of update attribute string about trace or layout object
440+
* @param {*} newVal
441+
* update value passed to restyle / relayout / update
442+
* @param {Object} undoit
443+
* undo hash (N.B. undoit may be mutated here).
444+
*
445+
*/
446+
exports.manageArrayContainers = function(np, newVal, undoit) {
447+
var obj = np.obj,
448+
parts = np.parts,
449+
pLength = parts.length,
450+
pLast = parts[pLength - 1];
451+
452+
var pLastIsNumber = isNumeric(pLast);
453+
454+
// delete item
455+
if(pLastIsNumber && newVal === null) {
456+
457+
// Clear item in array container when new value is null
458+
var contPath = parts.slice(0, pLength - 1).join('.'),
459+
cont = Lib.nestedProperty(obj, contPath).get();
460+
cont.splice(pLast, 1);
461+
462+
// Note that nested property clears null / undefined at end of
463+
// array container, but not within them.
464+
}
465+
// create item
466+
else if(pLastIsNumber && np.get() === undefined) {
467+
468+
// When adding a new item, make sure undo command will remove it
469+
if(np.get() === undefined) undoit[np.astr] = null;
470+
471+
np.set(newVal);
472+
}
473+
// update item
474+
else {
475+
476+
// If the last part of attribute string isn't a number,
477+
// np.set is all we need.
478+
np.set(newVal);
479+
}
480+
};

‎src/plot_api/plot_api.js‎

Lines changed: 13 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,9 +1415,6 @@ function _restyle(gd, aobj, _traces) {
14151415
continue;
14161416
}
14171417

1418-
// take no chances on transforms
1419-
if(ai.substr(0, 10) === 'transforms') flags.docalc = true;
1420-
14211418
// set attribute in gd.data
14221419
undoit[ai] = a0();
14231420
for(i = 0; i < traces.length; i++) {
@@ -1557,6 +1554,10 @@ function _restyle(gd, aobj, _traces) {
15571554
}
15581555
helpers.swapXYData(cont);
15591556
}
1557+
else if(Plots.dataArrayContainers.indexOf(param.parts[0]) !== -1) {
1558+
helpers.manageArrayContainers(param, newVal, undoit);
1559+
flags.docalc = true;
1560+
}
15601561
// all the other ones, just modify that one attribute
15611562
else param.set(newVal);
15621563

@@ -1816,8 +1817,7 @@ function _relayout(gd, aobj) {
18161817
// trunk nodes (everything except the leaf)
18171818
ptrunk = p.parts.slice(0, pend).join('.'),
18181819
parentIn = Lib.nestedProperty(gd.layout, ptrunk).get(),
1819-
parentFull = Lib.nestedProperty(fullLayout, ptrunk).get(),
1820-
diff;
1820+
parentFull = Lib.nestedProperty(fullLayout, ptrunk).get();
18211821

18221822
if(vi === undefined) continue;
18231823

@@ -1922,6 +1922,9 @@ function _relayout(gd, aobj) {
19221922
objList = layout[objType] || [],
19231923
obji = objList[objNum] || {};
19241924

1925+
// new API, remove annotation / shape with `null`
1926+
if(vi === null) aobj[ai] = 'remove';
1927+
19251928
// if p.parts is just an annotation number, and val is either
19261929
// 'add' or an entire annotation to add, the undo is 'remove'
19271930
// if val is 'remove' then undo is the whole annotation object
@@ -1951,42 +1954,11 @@ function _relayout(gd, aobj) {
19511954
drawOne(gd, objNum, p.parts.slice(2).join('.'), aobj[ai]);
19521955
delete aobj[ai];
19531956
}
1954-
else if(p.parts[0] === 'images') {
1955-
var update = Lib.objectFromPath(ai, vi);
1956-
Lib.extendDeepAll(gd.layout, update);
1957-
1958-
Registry.getComponentMethod('images', 'supplyLayoutDefaults')(gd.layout, gd._fullLayout);
1959-
Registry.getComponentMethod('images', 'draw')(gd);
1960-
}
1961-
else if(p.parts[0] === 'mapbox' && p.parts[1] === 'layers') {
1962-
Lib.extendDeepAll(gd.layout, Lib.objectFromPath(ai, vi));
1963-
1964-
// append empty container to mapbox.layers
1965-
// so that relinkPrivateKeys does not complain
1966-
1967-
var fullLayers = (gd._fullLayout.mapbox || {}).layers || [];
1968-
diff = (p.parts[2] + 1) - fullLayers.length;
1969-
1970-
for(i = 0; i < diff; i++) fullLayers.push({});
1971-
1972-
flags.doplot = true;
1973-
}
1974-
else if(p.parts[0] === 'updatemenus') {
1975-
Lib.extendDeepAll(gd.layout, Lib.objectFromPath(ai, vi));
1976-
1977-
var menus = gd._fullLayout.updatemenus || [];
1978-
diff = (p.parts[2] + 1) - menus.length;
1979-
1980-
for(i = 0; i < diff; i++) menus.push({});
1981-
flags.doplot = true;
1982-
}
1983-
else if(p.parts[0] === 'sliders') {
1984-
Lib.extendDeepAll(gd.layout, Lib.objectFromPath(ai, vi));
1985-
1986-
var sliders = gd._fullLayout.sliders || [];
1987-
diff = (p.parts[2] + 1) - sliders.length;
1988-
1989-
for(i = 0; i < diff; i++) sliders.push({});
1957+
else if(
1958+
Plots.layoutArrayContainers.indexOf(p.parts[0]) !== -1 ||
1959+
(p.parts[0] === 'mapbox' && p.parts[1] === 'layers')
1960+
) {
1961+
helpers.manageArrayContainers(p, vi, undoit);
19901962
flags.doplot = true;
19911963
}
19921964
// alter gd.layout

‎src/plots/plots.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,6 +1437,9 @@ plots.extendObjectWithContainers = function(dest, src, containerPaths) {
14371437
return dest;
14381438
};
14391439

1440+
plots.dataArrayContainers = ['transforms'];
1441+
plots.layoutArrayContainers = ['annotations', 'shapes', 'images', 'sliders', 'updatemenus'];
1442+
14401443
/*
14411444
* Extend a trace definition. This method:
14421445
*
@@ -1446,7 +1449,7 @@ plots.extendObjectWithContainers = function(dest, src, containerPaths) {
14461449
* The result is the original object reference with the new contents merged in.
14471450
*/
14481451
plots.extendTrace = function(destTrace, srcTrace) {
1449-
return plots.extendObjectWithContainers(destTrace, srcTrace, ['transforms']);
1452+
return plots.extendObjectWithContainers(destTrace, srcTrace, plots.dataArrayContainers);
14501453
};
14511454

14521455
/*
@@ -1459,13 +1462,7 @@ plots.extendTrace = function(destTrace, srcTrace) {
14591462
* The result is the original object reference with the new contents merged in.
14601463
*/
14611464
plots.extendLayout = function(destLayout, srcLayout) {
1462-
return plots.extendObjectWithContainers(destLayout, srcLayout, [
1463-
'annotations',
1464-
'shapes',
1465-
'images',
1466-
'sliders',
1467-
'updatemenus'
1468-
]);
1465+
return plots.extendObjectWithContainers(destLayout, srcLayout, plots.layoutArrayContainers);
14691466
};
14701467

14711468
/**

‎test/jasmine/tests/annotations_test.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,18 @@ describe('annotations relayout', function() {
6969
expect(countAnnotations()).toEqual(len + 1);
7070

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

76+
return Plotly.relayout(gd, 'annotations[' + 0 + ']', null);
77+
})
78+
.then(function() {
79+
expect(countAnnotations()).toEqual(len - 1);
80+
7581
return Plotly.relayout(gd, { annotations: [] });
76-
}).then(function() {
82+
})
83+
.then(function() {
7784
expect(countAnnotations()).toEqual(0);
7885

7986
done();

‎test/jasmine/tests/layout_images_test.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,18 +329,22 @@ describe('Layout images', function() {
329329
return Plotly.relayout(gd, 'images[2]', makeImage(pythonLogo, 0.2, 0.5));
330330
}).then(function() {
331331
assertImages(3);
332+
expect(gd.layout.images.length).toEqual(3);
332333

333-
return Plotly.relayout(gd, 'images[2]', 'remove');
334+
return Plotly.relayout(gd, 'images[2]', null);
334335
}).then(function() {
335336
assertImages(2);
337+
expect(gd.layout.images.length).toEqual(2);
336338

337-
return Plotly.relayout(gd, 'images[1]', 'remove');
339+
return Plotly.relayout(gd, 'images[1]', null);
338340
}).then(function() {
339341
assertImages(1);
342+
expect(gd.layout.images.length).toEqual(1);
340343

341-
return Plotly.relayout(gd, 'images[0]', 'remove');
344+
return Plotly.relayout(gd, 'images[0]', null);
342345
}).then(function() {
343346
assertImages(0);
347+
expect(gd.layout.images).toEqual([]);
344348

345349
done();
346350
});

‎test/jasmine/tests/lib_test.js‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,23 +1561,27 @@ describe('Queue', function() {
15611561

15621562
Plotly.plot(gd, [{
15631563
y: [2, 1, 2]
1564-
}]).then(function() {
1564+
}])
1565+
.then(function() {
15651566
expect(gd.undoQueue).toBeUndefined();
15661567

15671568
return Plotly.restyle(gd, 'marker.color', 'red');
1568-
}).then(function() {
1569+
})
1570+
.then(function() {
15691571
expect(gd.undoQueue.index).toEqual(1);
15701572
expect(gd.undoQueue.queue[0].undo.args[0][1]['marker.color']).toEqual([undefined]);
15711573
expect(gd.undoQueue.queue[0].redo.args[0][1]['marker.color']).toEqual('red');
15721574

15731575
return Plotly.relayout(gd, 'title', 'A title');
1574-
}).then(function() {
1576+
})
1577+
.then(function() {
15751578
expect(gd.undoQueue.index).toEqual(2);
15761579
expect(gd.undoQueue.queue[1].undo.args[0][1].title).toEqual(undefined);
15771580
expect(gd.undoQueue.queue[1].redo.args[0][1].title).toEqual('A title');
15781581

15791582
return Plotly.restyle(gd, 'mode', 'markers');
1580-
}).then(function() {
1583+
})
1584+
.then(function() {
15811585
expect(gd.undoQueue.index).toEqual(2);
15821586
expect(gd.undoQueue.queue[2]).toBeUndefined();
15831587

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

1594+
return Plotly.restyle(gd, 'transforms[0]', { type: 'filter' });
1595+
})
1596+
.then(function() {
1597+
expect(gd.undoQueue.queue[1].undo.args[0][1])
1598+
.toEqual({ 'transforms[0]': null });
1599+
expect(gd.undoQueue.queue[1].redo.args[0][1])
1600+
.toEqual({ 'transforms[0]': { type: 'filter' } });
1601+
1602+
return Plotly.relayout(gd, 'updatemenus[0]', { buttons: [] });
1603+
})
1604+
.then(function() {
1605+
expect(gd.undoQueue.queue[1].undo.args[0][1])
1606+
.toEqual({ 'updatemenus[0]': null });
1607+
expect(gd.undoQueue.queue[1].redo.args[0][1])
1608+
.toEqual({ 'updatemenus[0]': { buttons: [] } });
1609+
1610+
return Plotly.relayout(gd, 'updatemenus[0]', null);
1611+
})
1612+
.then(function() {
1613+
expect(gd.undoQueue.queue[1].undo.args[0][1])
1614+
.toEqual({ 'updatemenus[0]': { buttons: []} });
1615+
expect(gd.undoQueue.queue[1].redo.args[0][1])
1616+
.toEqual({ 'updatemenus[0]': null });
1617+
1618+
return Plotly.restyle(gd, 'transforms[0]', null);
1619+
})
1620+
.then(function() {
1621+
expect(gd.undoQueue.queue[1].undo.args[0][1])
1622+
.toEqual({ 'transforms[0]': [ { type: 'filter' } ]});
1623+
expect(gd.undoQueue.queue[1].redo.args[0][1])
1624+
.toEqual({ 'transforms[0]': null });
1625+
15901626
done();
15911627
});
15921628
});

‎test/jasmine/tests/mapbox_test.js‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -548,56 +548,77 @@ describe('mapbox plots', function() {
548548
expect(countVisibleLayers(gd)).toEqual(0);
549549

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

553554
return Plotly.relayout(gd, 'mapbox.layers[1]', layer1);
554-
}).then(function() {
555+
})
556+
.then(function() {
557+
expect(gd.layout.mapbox.layers.length).toEqual(2);
555558
expect(countVisibleLayers(gd)).toEqual(2);
556559

557560
return Plotly.relayout(gd, mapUpdate);
558-
}).then(function() {
561+
})
562+
.then(function() {
563+
expect(gd.layout.mapbox.layers.length).toEqual(2);
559564
expect(countVisibleLayers(gd)).toEqual(2);
560565

561566
return Plotly.relayout(gd, styleUpdate0);
562-
}).then(function() {
567+
})
568+
.then(function() {
569+
expect(gd.layout.mapbox.layers.length).toEqual(2);
563570
expect(countVisibleLayers(gd)).toEqual(2);
564571

565572
return assertLayerStyle(gd, {
566573
'fill-color': [1, 0, 0, 1],
567574
'fill-outline-color': [0, 0, 1, 1],
568575
'fill-opacity': 0.3
569576
}, 0);
570-
}).then(function() {
577+
})
578+
.then(function() {
579+
expect(gd.layout.mapbox.layers.length).toEqual(2);
571580
expect(countVisibleLayers(gd)).toEqual(2);
572581

573582
return Plotly.relayout(gd, styleUpdate1);
574-
}).then(function() {
583+
})
584+
.then(function() {
585+
expect(gd.layout.mapbox.layers.length).toEqual(2);
575586
expect(countVisibleLayers(gd)).toEqual(2);
576587

577588
return assertLayerStyle(gd, {
578589
'line-width': 3,
579590
'line-color': [0, 0, 1, 1],
580591
'line-opacity': 0.6
581592
}, 1);
582-
}).then(function() {
593+
})
594+
.then(function() {
595+
expect(gd.layout.mapbox.layers.length).toEqual(2);
583596
expect(countVisibleLayers(gd)).toEqual(2);
584597

585-
return Plotly.relayout(gd, 'mapbox.layers[1]', 'remove');
586-
}).then(function() {
598+
return Plotly.relayout(gd, 'mapbox.layers[1]', null);
599+
})
600+
.then(function() {
601+
expect(gd.layout.mapbox.layers.length).toEqual(1);
587602
expect(countVisibleLayers(gd)).toEqual(1);
588603

589-
return Plotly.relayout(gd, 'mapbox.layers[0]', 'remove');
590-
}).then(function() {
604+
return Plotly.relayout(gd, 'mapbox.layers[0]', null);
605+
})
606+
.then(function() {
607+
expect(gd.layout.mapbox.layers.length).toEqual(0);
591608
expect(countVisibleLayers(gd)).toEqual(0);
592609

593610
return Plotly.relayout(gd, 'mapbox.layers[0]', {});
594-
}).then(function() {
611+
})
612+
.then(function() {
613+
expect(gd.layout.mapbox.layers).toEqual([]);
595614
expect(countVisibleLayers(gd)).toEqual(0);
596615

597616
// layer with no source are not drawn
598617

599618
return Plotly.relayout(gd, 'mapbox.layers[0].source', layer0.source);
600-
}).then(function() {
619+
})
620+
.then(function() {
621+
expect(gd.layout.mapbox.layers.length).toEqual(1);
601622
expect(countVisibleLayers(gd)).toEqual(1);
602623

603624
done();

0 commit comments

Comments
 (0)