fix tests & some edge cases uncovered by them

plotly · alexcjohnson · Nov 21, 2020 · Nov 12, 2020 · Nov 18, 2020 · Nov 19, 2020
commit b707c0f4372f116e996ef07238c5b8bbfdc03c93
diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js
@@ -2055,7 +2055,7 @@ function _relayout(gd, aobj) {
     // we're editing the (auto)range of, so we can tell the others constrained
     // to scale with them that it's OK for them to shrink
     var rangesAltered = {};
-    var axId;
+    var axId, ax;
 
     function recordAlteredAxis(pleafPlus) {
         var axId = Axes.name2id(pleafPlus.split('.')[0]);
@@ -2145,7 +2145,7 @@ function _relayout(gd, aobj) {
         // previously we did this for log <-> not-log, but now only do it
         // for log <-> linear
         if(pleaf === 'type') {
-            var ax = parentIn;
+            ax = parentIn;
             var toLog = parentFull.type === 'linear' && vi === 'log';
             var fromLog = parentFull.type === 'log' && vi === 'linear';
 
@@ -2284,7 +2284,8 @@ function _relayout(gd, aobj) {
 
     // figure out if we need to recalculate axis constraints
     for(axId in rangesAltered) {
-        var group = Axes.getFromId(gd, axId)._constraintGroup;
+        ax = Axes.getFromId(gd, axId);
+        var group = ax && ax._constraintGroup;
         if(group) {
             // Always recalc if we're changing constrained ranges.
             // Otherwise it's possible to violate the constraints by

diff --git a/src/plots/cartesian/constraints.js b/src/plots/cartesian/constraints.js
@@ -55,26 +55,9 @@ exports.handleDefaults = function(layoutIn, layoutOut, opts) {
     }
     stash(matchGroups, '_matchGroup');
 
-    // remove constraint groups that simply duplicate match groups
-    i = 0;
-    while(i < constraintGroups.length) {
-        group = constraintGroups[i];
-        for(axId in group) {
-            axOut = layoutOut[id2name(axId)];
-            if(axOut._matchGroup && Object.keys(axOut._matchGroup).length === Object.keys(group).length) {
-                constraintGroups.splice(i, 1);
-                i--;
-            }
-            break;
-        }
-        i++;
-    }
-
-    // save constraintGroup on each constrained axis
-    stash(constraintGroups, '_constraintGroup');
-
     // If any axis in a constraint group is fixedrange, they all get fixed
-    // This covers matches axes, as they're now in the constraintgroup too.
+    // This covers matches axes, as they're now in the constraintgroup too
+    // and have not yet been removed (if the group is *only* matching)
     for(i = 0; i < constraintGroups.length; i++) {
         group = constraintGroups[i];
         for(axId in group) {
@@ -89,33 +72,53 @@ exports.handleDefaults = function(layoutIn, layoutOut, opts) {
                             'axis in its constraint group has fixedrange true'
                         );
                     }
-                    layoutOut[id2name(axId2)].fixedrange = true;
+                    layoutOut[axName2].fixedrange = true;
                 }
                 break;
             }
         }
     }
 
+    // remove constraint groups that simply duplicate match groups
+    i = 0;
+    while(i < constraintGroups.length) {
+        group = constraintGroups[i];
+        for(axId in group) {
+            axOut = layoutOut[id2name(axId)];
+            if(axOut._matchGroup && Object.keys(axOut._matchGroup).length === Object.keys(group).length) {
+                constraintGroups.splice(i, 1);
+                i--;
+            }
+            break;
+        }
+        i++;
+    }
+
+    // save constraintGroup on each constrained axis
+    stash(constraintGroups, '_constraintGroup');
+
     // make sure `matching` axes share values of necessary attributes
     // Precedence (base axis is the one that doesn't list a `matches`, ie others
     // all point to it):
     // (1) explicitly defined value in the base axis
     // (2) explicitly defined in another axis (arbitrary order)
     // (3) default in the base axis
-    var matchAttrs = {
-        constrain: 1,
-        range: 1,
-        autorange: 1,
-        rangemode: 1,
-        rangebreaks: 1,
-        categoryorder: 1,
-        categoryarray: 1
-    };
+    var matchAttrs = [
+        'constrain',
+        'range',
+        'autorange',
+        'rangemode',
+        'rangebreaks',
+        'categoryorder',
+        'categoryarray'
+    ];
+    var hasRange = false;
     for(i = 0; i < matchGroups.length; i++) {
         group = matchGroups[i];
 
         // find 'matching' range attrs
-        for(var attr in matchAttrs) {
+        for(var j = 0; j < matchAttrs.length; j++) {
+            var attr = matchAttrs[j];
             var val = null;
             var baseAx;
             for(axId in group) {
@@ -138,6 +141,17 @@ exports.handleDefaults = function(layoutIn, layoutOut, opts) {
                     val = axOut[attr];
                 }
             }
+
+            // special logic for coupling of range and autorange
+            // if nobody explicitly specifies autorange, but someone does
+            // explicitly specify range, autorange must be disabled.
+            if(attr === 'range' && val) {
+                hasRange = true;
+            }
+            if(attr === 'autorange' && val === null && hasRange) {
+                val = false;
+            }
+
             if(val === null && attr in baseAx) {
                 // fallback: default value in base axis
                 val = baseAx[attr];
@@ -244,7 +258,7 @@ function handleOneAxDefaults(axIn, axOut, opts) {
 
         // Also include match constraints in the scale groups
         var matchedAx = layoutOut[id2name(matches)];
-        var matchRatio = extent(axOut) / extent(matchedAx);
+        var matchRatio = extent(layoutOut, axOut) / extent(layoutOut, matchedAx);
         if(isX !== (matches.charAt(0) === 'x')) {
             // We don't yet know the actual scale ratio of x/y matches constraints,
             // due to possible automargins, so just leave a placeholder for this:
@@ -277,8 +291,14 @@ function handleOneAxDefaults(axIn, axOut, opts) {
     }
 }
 
-function extent(ax) {
-    return ax.domain[1] - ax.domain[0];
+function extent(layoutOut, ax) {
+    var domain = ax.domain;
+    if(!domain) {
+        // at this point overlaying axes haven't yet inherited their main domains
+        // TODO: constrain: domain with overlaying axes is likely a bug.
+        domain = layoutOut[id2name(ax.overlaying)].domain;
+    }
+    return domain[1] - domain[0];
 }
 
 function getConstraintGroup(groups, thisID) {

diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js
@@ -379,7 +379,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
     // (so we link only axes of the same type) and
     // `fixedrange` (so we can avoid linking from OR TO a fixed axis).
     constraints.handleDefaults(layoutIn, layoutOut, {
-        axIds: allAxisIds.concat(missingMatchedAxisIds),
+        axIds: allAxisIds.concat(missingMatchedAxisIds).sort(axisIds.idSort),
         axHasImage: axHasImage
     });
 };