Preserve ui state across Plotly.react redraws

src/components/modebar/buttons.js

@@ -370,53 +370,47 @@ modeBarButtons.hoverClosest3d = {
     click: handleHover3d
 };
 
-function handleHover3d(gd, ev) {
+function getNextHover3d(gd, ev) {
     var button = ev.currentTarget;
-    var val = button._previousVal || false;
-    var layout = gd.layout;
+    var val = button._previousVal;
     var fullLayout = gd._fullLayout;
     var sceneIds = fullLayout._subplots.gl3d;
 
     var axes = ['xaxis', 'yaxis', 'zaxis'];
-    var spikeAttrs = ['showspikes', 'spikesides', 'spikethickness', 'spikecolor'];
 
     // initialize 'current spike' object to be stored in the DOM
     var currentSpikes = {};
-    var axisSpikes = {};
     var layoutUpdate = {};
 
     if(val) {
-        layoutUpdate = Lib.extendDeep(layout, val);
+        layoutUpdate = val;
         button._previousVal = null;
     }
     else {
-        layoutUpdate = {
-            'allaxes.showspikes': false
-        };
-
         for(var i = 0; i < sceneIds.length; i++) {
-            var sceneId = sceneIds[i],
-                sceneLayout = fullLayout[sceneId],
-                sceneSpikes = currentSpikes[sceneId] = {};
+            var sceneId = sceneIds[i];
+            var sceneLayout = fullLayout[sceneId];
 
-            sceneSpikes.hovermode = sceneLayout.hovermode;
-            layoutUpdate[sceneId + '.hovermode'] = false;
+            var hovermodeAStr = sceneId + '.hovermode';
+            currentSpikes[hovermodeAStr] = sceneLayout.hovermode;
+            layoutUpdate[hovermodeAStr] = false;
 
             // copy all the current spike attrs
             for(var j = 0; j < 3; j++) {
                 var axis = axes[j];
-                axisSpikes = sceneSpikes[axis] = {};
-
-                for(var k = 0; k < spikeAttrs.length; k++) {
-                    var spikeAttr = spikeAttrs[k];
-                    axisSpikes[spikeAttr] = sceneLayout[axis][spikeAttr];
-                }
+                var spikeAStr = sceneId + '.' + axis + '.showspikes';
+                layoutUpdate[spikeAStr] = false;
+                currentSpikes[spikeAStr] = sceneLayout[axis].showspikes;
             }
         }
 
-        button._previousVal = Lib.extendDeep({}, currentSpikes);
+        button._previousVal = currentSpikes;
     }
+    return layoutUpdate;
+}
 
+function handleHover3d(gd, ev) {
+    var layoutUpdate = getNextHover3d(gd, ev);
     Registry.call('relayout', gd, layoutUpdate);
 }
 
@@ -501,17 +495,19 @@ modeBarButtons.hoverClosestPie = {
     click: toggleHover
 };
 
-function toggleHover(gd) {
+function getNextHover(gd) {
     var fullLayout = gd._fullLayout;
 
-    var onHoverVal;
+    if(fullLayout.hovermode) return false;
+
     if(fullLayout._has('cartesian')) {
-        onHoverVal = fullLayout._isHoriz ? 'y' : 'x';
+        return fullLayout._isHoriz ? 'y' : 'x';
     }
-    else onHoverVal = 'closest';
-
-    var newHover = gd._fullLayout.hovermode ? false : onHoverVal;
+    return 'closest';
+}
 
+function toggleHover(gd) {
+    var newHover = getNextHover(gd);
     Registry.call('relayout', gd, 'hovermode', newHover);
 }
 
@@ -526,12 +522,10 @@ modeBarButtons.toggleHover = {
     icon: Icons.tooltip_basic,
     gravity: 'ne',
     click: function(gd, ev) {
-        toggleHover(gd);
+        var layoutUpdate = getNextHover3d(gd, ev);
+        layoutUpdate.hovermode = getNextHover(gd);
 
-        // the 3d hovermode update must come
-        // last so that layout.hovermode update does not
-        // override scene?.hovermode?.layout.
-        handleHover3d(gd, ev);
+        Registry.call('relayout', gd, layoutUpdate);
     }
 };
 

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)');