Skip to content

Commit b134a52

Browse files
committed
fix obscure undo/redo bug with container array relayout removal
1 parent 082ac38 commit b134a52

File tree

3 files changed

+33
-6
lines changed

3 files changed

+33
-6
lines changed

src/plot_api/plot_api.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -2063,8 +2063,6 @@ function _relayout(gd, aobj) {
20632063
arrayStr = containerArrayMatch.array;
20642064
i = containerArrayMatch.index;
20652065
var propStr = containerArrayMatch.property;
2066-
var componentArray = Lib.nestedProperty(layout, arrayStr);
2067-
var obji = (componentArray || [])[i] || {};
20682066
var updateValObject = valObject || {editType: 'calc'};
20692067

20702068
if(i !== '' && propStr === '') {
@@ -2074,7 +2072,7 @@ function _relayout(gd, aobj) {
20742072
if(manageArrays.isAddVal(vi)) {
20752073
undoit[ai] = null;
20762074
} else if(manageArrays.isRemoveVal(vi)) {
2077-
undoit[ai] = obji;
2075+
undoit[ai] = (nestedProperty(layout, arrayStr).get() || [])[i];
20782076
} else {
20792077
Lib.warn('unrecognized full object value', aobj);
20802078
}

test/jasmine/tests/annotations_test.js

+31-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
var Annotations = require('@src/components/annotations');
22

33
var Plotly = require('@lib/index');
4+
var Queue = require('@src/lib/queue');
45
var Plots = require('@src/plots/plots');
56
var Lib = require('@src/lib');
67
var Loggers = require('@src/lib/loggers');
@@ -191,11 +192,20 @@ describe('annotations relayout', function() {
191192
Plotly.plot(gd, mockData, mockLayout).then(done);
192193

193194
spyOn(Loggers, 'warn');
195+
196+
Plotly.setPlotConfig({queueLength: 3});
194197
});
195198

196-
afterEach(destroyGraphDiv);
199+
afterEach(function() {
200+
destroyGraphDiv();
201+
Plotly.setPlotConfig({queueLength: 0});
202+
});
197203

198204
function countAnnotations() {
205+
// also check that no annotations are empty objects
206+
(gd.layout.annotations || []).forEach(function(ann, i) {
207+
expect(JSON.stringify(ann)).not.toBe(JSON.stringify({}), i);
208+
});
199209
return d3.selectAll('g.annotation').size();
200210
}
201211

@@ -220,11 +230,31 @@ describe('annotations relayout', function() {
220230
.then(function() {
221231
expect(countAnnotations()).toEqual(len);
222232

233+
return Queue.undo(gd);
234+
})
235+
.then(function() {
236+
expect(countAnnotations()).toBe(len + 1);
237+
238+
return Queue.redo(gd);
239+
})
240+
.then(function() {
241+
expect(countAnnotations()).toBe(len);
242+
223243
return Plotly.relayout(gd, 'annotations[0]', null);
224244
})
225245
.then(function() {
226246
expect(countAnnotations()).toEqual(len - 1);
227247

248+
return Queue.undo(gd);
249+
})
250+
.then(function() {
251+
expect(countAnnotations()).toBe(len);
252+
253+
return Queue.redo(gd);
254+
})
255+
.then(function() {
256+
expect(countAnnotations()).toBe(len - 1);
257+
228258
return Plotly.relayout(gd, 'annotations[0].visible', false);
229259
})
230260
.then(function() {

test/jasmine/tests/lib_test.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -2631,9 +2631,8 @@ describe('Queue', function() {
26312631
return Plotly.relayout(gd, 'updatemenus[0]', null);
26322632
})
26332633
.then(function() {
2634-
// buttons have been stripped out because it's an empty container array...
26352634
expect(gd.undoQueue.queue[1].undo.args[0][1])
2636-
.toEqual({ 'updatemenus[0]': {} });
2635+
.toEqual({ 'updatemenus[0]': { buttons: [] } });
26372636
expect(gd.undoQueue.queue[1].redo.args[0][1])
26382637
.toEqual({ 'updatemenus[0]': null });
26392638

0 commit comments

Comments
 (0)