Preserve ui state across Plotly.react redraws

src/plot_api/plot_api.js

@@ -15,6 +15,8 @@ var isNumeric = require('fast-isnumeric');
 var hasHover = require('has-hover');
 
 var Lib = require('../lib');
+var nestedProperty = Lib.nestedProperty;
+
 var Events = require('../lib/events');
 var Queue = require('../lib/queue');
 
@@ -842,7 +844,7 @@ function getExtendProperties(gd, update, indices, maxPoints) {
              * instance that references the key and value for this particular trace.
              */
             trace = gd.data[indices[j]];
-            prop = Lib.nestedProperty(trace, key);
+            prop = nestedProperty(trace, key);
 
             /*
              * Target is the existing gd.data.trace.dataArray value like "x" or "marker.size"
@@ -1420,6 +1422,16 @@ function _restyle(gd, aobj, traces) {
 
     function rangeAttr(axName) { return 'LAYOUT' + axName + '.range'; }
 
+    function getFullTrace(traceIndex) {
+        // usually fullData maps 1:1 onto data, but with groupby transforms
+        // the fullData index can be greater. Take the *first* matching trace.
+        for(var j = traceIndex; j < fullData.length; j++) {
+            if(fullData[j]._input === data[traceIndex]) return fullData[j];
+        }
+        // should never get here - and if we *do* it should cause an error
+        // later on undefined fullTrace is passed to nestedProperty.
+    }
+
     // for attrs that interact (like scales & autoscales), save the
     // old vals before making the change
     // val=undefined will not set a value, just record what the value was.
@@ -1507,7 +1519,7 @@ function _restyle(gd, aobj, traces) {
         undoit[ai] = a0();
         for(i = 0; i < traces.length; i++) {
             cont = data[traces[i]];
-            contFull = fullData[traces[i]];
+            contFull = getFullTrace(traces[i]);
             param = Lib.nestedProperty(cont, ai);
             oldVal = param.get();
             newVal = Array.isArray(vi) ? vi[i % vi.length] : vi;
@@ -1518,7 +1530,7 @@ function _restyle(gd, aobj, traces) {
             var prefix = ai.substr(0, ai.length - finalPart.length - 1);
             var prefixDot = prefix ? prefix + '.' : '';
             var innerContFull = prefix ?
-                Lib.nestedProperty(contFull, prefix).get() : contFull;
+                nestedProperty(contFull, prefix).get() : contFull;
 
             valObject = PlotSchema.getTraceValObject(contFull, param.parts);
 
@@ -1565,14 +1577,14 @@ function _restyle(gd, aobj, traces) {
                 Lib.swapAttrs(cont, ['?', '?src'], 'values', valuesTo);
 
                 if(oldVal === 'pie') {
-                    Lib.nestedProperty(cont, 'marker.color')
-                        .set(Lib.nestedProperty(cont, 'marker.colors').get());
+                    nestedProperty(cont, 'marker.color')
+                        .set(nestedProperty(cont, 'marker.colors').get());
 
                     // super kludgy - but if all pies are gone we won't remove them otherwise
                     fullLayout._pielayer.selectAll('g.trace').remove();
                 } else if(Registry.traceIs(cont, 'cartesian')) {
-                    Lib.nestedProperty(cont, 'marker.colors')
-                        .set(Lib.nestedProperty(cont, 'marker.color').get());
+                    nestedProperty(cont, 'marker.colors')
+                        .set(nestedProperty(cont, 'marker.color').get());
                 }
             }
 
@@ -1643,7 +1655,7 @@ function _restyle(gd, aobj, traces) {
 
         // swap hovermode if set to "compare x/y data"
         if(ai === 'orientationaxes') {
-            var hovermode = Lib.nestedProperty(gd.layout, 'hovermode');
+            var hovermode = nestedProperty(gd.layout, 'hovermode');
             if(hovermode.get() === 'x') {
                 hovermode.set('y');
             } else if(hovermode.get() === 'y') {
@@ -1898,8 +1910,8 @@ function _relayout(gd, aobj) {
         var pleafPlus = p.parts[pend - 1] + '.' + pleaf;
         // trunk nodes (everything except the leaf)
         var ptrunk = p.parts.slice(0, pend).join('.');
-        var parentIn = Lib.nestedProperty(gd.layout, ptrunk).get();
-        var parentFull = Lib.nestedProperty(fullLayout, ptrunk).get();
+        var parentIn = nestedProperty(gd.layout, ptrunk).get();
+        var parentFull = nestedProperty(fullLayout, ptrunk).get();
         var vOld = p.get();
 
         if(vi === undefined) continue;
@@ -1944,20 +1956,20 @@ function _relayout(gd, aobj) {
         // check autorange vs range
         else if(pleafPlus.match(AX_RANGE_RE)) {
             recordAlteredAxis(pleafPlus);
-            Lib.nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
+            nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
         }
         else if(pleafPlus.match(AX_AUTORANGE_RE)) {
             recordAlteredAxis(pleafPlus);
-            Lib.nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
-            var axFull = Lib.nestedProperty(fullLayout, ptrunk).get();
+            nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
+            var axFull = nestedProperty(fullLayout, ptrunk).get();
             if(axFull._inputDomain) {
                 // if we're autoranging and this axis has a constrained domain,
                 // reset it so we don't get locked into a shrunken size
                 axFull._input.domain = axFull._inputDomain.slice();
             }
         }
         else if(pleafPlus.match(AX_DOMAIN_RE)) {
-            Lib.nestedProperty(fullLayout, ptrunk + '._inputDomain').set(null);
+            nestedProperty(fullLayout, ptrunk + '._inputDomain').set(null);
         }
 
         // toggling axis type between log and linear: we need to convert
@@ -2026,10 +2038,10 @@ function _relayout(gd, aobj) {
                 doextra(ptrunk + '.autorange', true);
                 doextra(ptrunk + '.range', null);
             }
-            Lib.nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
+            nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
         }
         else if(pleaf.match(AX_NAME_PATTERN)) {
-            var fullProp = Lib.nestedProperty(fullLayout, ai).get(),
+            var fullProp = nestedProperty(fullLayout, ai).get(),
                 newType = (vi || {}).type;
 
             // This can potentially cause strange behavior if the autotype is not
@@ -2051,8 +2063,6 @@ function _relayout(gd, aobj) {
             arrayStr = containerArrayMatch.array;
             i = containerArrayMatch.index;
             var propStr = containerArrayMatch.property;
-            var componentArray = Lib.nestedProperty(layout, arrayStr);
-            var obji = (componentArray || [])[i] || {};
             var updateValObject = valObject || {editType: 'calc'};
 
             if(i !== '' && propStr === '') {
@@ -2062,7 +2072,7 @@ function _relayout(gd, aobj) {
                 if(manageArrays.isAddVal(vi)) {
                     undoit[ai] = null;
                 } else if(manageArrays.isRemoveVal(vi)) {
-                    undoit[ai] = obji;
+                    undoit[ai] = (nestedProperty(layout, arrayStr).get() || [])[i];
                 } else {
                     Lib.warn('unrecognized full object value', aobj);
                 }
@@ -2106,7 +2116,7 @@ function _relayout(gd, aobj) {
     // now we've collected component edits - execute them all together
     for(arrayStr in arrayEdits) {
         var finished = manageArrays.applyContainerArrayChanges(gd,
-            Lib.nestedProperty(layout, arrayStr), arrayEdits[arrayStr], flags);
+            nestedProperty(layout, arrayStr), arrayEdits[arrayStr], flags);
         if(!finished) flags.plot = true;
     }
 

src/traces/histogram/attributes.js

@@ -172,11 +172,6 @@ module.exports = {
         dflt: null,
         role: 'style',
         editType: 'calc',
-        impliedEdits: {
-            'ybins.start': undefined,
-            'ybins.end': undefined,
-            'ybins.size': undefined
-        },
         description: [
             'Obsolete: since v1.42 each bin attribute is auto-determined',
             'separately and `autobiny` is not needed. However, we accept',

test/jasmine/assets/custom_assertions.js

@@ -59,9 +59,7 @@ exports.assertHoverLabelStyle = function(g, expectation, msg, textSelector) {
     expect(textStyle.fill).toBe(expectation.fontColor, msg + ': font.color');
 };
 
-function assertLabelContent(label, expectation, msg) {
-    if(!expectation) expectation = '';
-
+function getLabelContent(label) {
     var lines = label.selectAll('tspan.line');
     var content = [];
 
@@ -77,8 +75,15 @@ function assertLabelContent(label, expectation, msg) {
     } else {
         fill(label);
     }
+    return content.join('\n');
+}
+
+function assertLabelContent(label, expectation, msg) {
+    if(!expectation) expectation = '';
+
+    var content = getLabelContent(label);
 
-    expect(content.join('\n')).toBe(expectation, msg + ': text content');
+    expect(content).toBe(expectation, msg + ': text content');
 }
 
 function count(selector) {
@@ -130,35 +135,63 @@ exports.assertHoverLabelContent = function(expectation, msg) {
         expect(ptCnt)
             .toBe(expectation.name.length, ptMsg + ' # of visible labels');
 
-        var bboxes = [];
+        var observed = [];
+        var expected = expectation.nums.map(function(num, i) {
+            return {
+                num: num,
+                name: expectation.name[i],
+                order: (expectation.hOrder || expectation.vOrder || []).indexOf(i)
+            };
+        });
         d3.selectAll(ptSelector).each(function(_, i) {
             var g = d3.select(this);
             var numsSel = g.select('text.nums');
             var nameSel = g.select('text.name');
 
-            assertLabelContent(numsSel, expectation.nums[i], ptMsg + ' (nums ' + i + ')');
-            assertLabelContent(nameSel, expectation.name[i], ptMsg + ' (name ' + i + ')');
+            // Label selection can be out of order... dunno why, but on AJ's Mac,
+            // just for certain box and violin cases, the order looks correct but
+            // it's different from what we see in CI (and presumably on
+            // other systems) which looks wrong.
+            // Anyway we don't *really* care about the order within the selection,
+            // we just care that each label is correct. So collect all the info
+            // about each label, and sort both observed and expected identically.
+            observed.push({
+                num: getLabelContent(numsSel),
+                name: getLabelContent(nameSel),
+                bbox: this.getBoundingClientRect(),
+                order: -1
+            });
 
             if('isRotated' in expectation) {
                 expect(g.attr('transform').match(reRotate))
                     .negateIf(expectation.isRotated)
                     .toBe(null, ptMsg + ' ' + i + ' should be rotated');
             }
-
-            bboxes.push({bbox: this.getBoundingClientRect(), index: i});
         });
-        if(expectation.vOrder) {
-            bboxes.sort(function(a, b) {
-                return (a.bbox.top + a.bbox.bottom - b.bbox.top - b.bbox.bottom) / 2;
+        if(expectation.vOrder || expectation.hOrder) {
+            var o2 = observed.slice();
+            o2.sort(function(a, b) {
+                return expectation.vOrder ?
+                    (a.bbox.top + a.bbox.bottom - b.bbox.top - b.bbox.bottom) / 2 :
+                    (b.bbox.left + b.bbox.right - a.bbox.left - a.bbox.right) / 2;
             });
-            expect(bboxes.map(function(d) { return d.index; })).toEqual(expectation.vOrder);
-        }
-        if(expectation.hOrder) {
-            bboxes.sort(function(a, b) {
-                return (b.bbox.left + b.bbox.right - a.bbox.left - a.bbox.right) / 2;
+            o2.forEach(function(item, i) {
+                item.order = i;
+                delete item.bbox;
             });
-            expect(bboxes.map(function(d) { return d.index; })).toEqual(expectation.hOrder);
         }
+        observed.sort(labelSorter);
+        expected.sort(labelSorter);
+        // don't use .toEqual here because we want the message
+        expect(observed.length).toBe(expected.length, ptMsg);
+        observed.forEach(function(obsi, i) {
+            var expi = expected[i];
+            expect(obsi.num).toBe(expi.num, ptMsg + ' (nums ' + i + ')');
+            expect(obsi.name).toBe(expi.name, ptMsg + ' (name ' + i + ')');
+            if(expectation.vOrder || expectation.hOrder) {
+                expect(obsi.order).toBe(expi.order, ptMsg + ' (order ' + i + ')');
+            }
+        });
     } else {
         if(expectation.nums) {
             fail(ptMsg + ': expecting *nums* labels, did not find any.');
@@ -181,6 +214,12 @@ exports.assertHoverLabelContent = function(expectation, msg) {
     }
 };
 
+function labelSorter(a, b) {
+    if(a.name !== b.name) return a.name > b.name ? 1 : -1;
+    if(a.num !== b.num) return a.num > b.num ? 1 : -1;
+    return a.order - b.order;
+}
+
 exports.assertClip = function(sel, isClipped, size, msg) {
     expect(sel.size()).toBe(size, msg + ' clip path (selection size)');
 

test/jasmine/tests/annotations_test.js

@@ -1,6 +1,7 @@
 var Annotations = require('@src/components/annotations');
 
 var Plotly = require('@lib/index');
+var Queue = require('@src/lib/queue');
 var Plots = require('@src/plots/plots');
 var Lib = require('@src/lib');
 var Loggers = require('@src/lib/loggers');
@@ -191,11 +192,20 @@ describe('annotations relayout', function() {
         Plotly.plot(gd, mockData, mockLayout).then(done);
 
         spyOn(Loggers, 'warn');
+
+        Plotly.setPlotConfig({queueLength: 3});
     });
 
-    afterEach(destroyGraphDiv);
+    afterEach(function() {
+        destroyGraphDiv();
+        Plotly.setPlotConfig({queueLength: 0});
+    });
 
     function countAnnotations() {
+        // also check that no annotations are empty objects
+        (gd.layout.annotations || []).forEach(function(ann, i) {
+            expect(JSON.stringify(ann)).not.toBe(JSON.stringify({}), i);
+        });
         return d3.selectAll('g.annotation').size();
     }
 
@@ -220,11 +230,31 @@ describe('annotations relayout', function() {
         .then(function() {
             expect(countAnnotations()).toEqual(len);
 
+            return Queue.undo(gd);
+        })
+        .then(function() {
+            expect(countAnnotations()).toBe(len + 1);
+
+            return Queue.redo(gd);
+        })
+        .then(function() {
+            expect(countAnnotations()).toBe(len);
+
             return Plotly.relayout(gd, 'annotations[0]', null);
         })
         .then(function() {
             expect(countAnnotations()).toEqual(len - 1);
 
+            return Queue.undo(gd);
+        })
+        .then(function() {
+            expect(countAnnotations()).toBe(len);
+
+            return Queue.redo(gd);
+        })
+        .then(function() {
+            expect(countAnnotations()).toBe(len - 1);
+
             return Plotly.relayout(gd, 'annotations[0].visible', false);
         })
         .then(function() {

test/jasmine/tests/lib_test.js

@@ -2631,9 +2631,8 @@ describe('Queue', function() {
             return Plotly.relayout(gd, 'updatemenus[0]', null);
         })
         .then(function() {
-            // buttons have been stripped out because it's an empty container array...
             expect(gd.undoQueue.queue[1].undo.args[0][1])
-                .toEqual({ 'updatemenus[0]': {} });
+                .toEqual({ 'updatemenus[0]': { buttons: [] } });
             expect(gd.undoQueue.queue[1].redo.args[0][1])
                 .toEqual({ 'updatemenus[0]': null });