Merge pull request #5268 from plotly/fix5267-cartesian-category-values

Fixup categoryorder for missing values in cartesian traces

Author: archmoj

src/plots/plots.js

@@ -3098,6 +3098,9 @@ function sortAxisCategoriesByValue(axList, gd) {
             var aggregator = match[1];
             var order = match[2];
 
+            var axLetter = ax._id.charAt(0);
+            var isX = axLetter === 'x';
+
             // Store values associated with each category
             var categoriesValue = [];
             for(j = 0; j < ax._categories.length; j++) {
@@ -3108,7 +3111,6 @@ function sortAxisCategoriesByValue(axList, gd) {
             for(j = 0; j < ax._traceIndices.length; j++) {
                 var traceIndex = ax._traceIndices[j];
                 var fullTrace = gd._fullData[traceIndex];
-                var axLetter = ax._id.charAt(0);
 
                 // Skip over invisible traces
                 if(fullTrace.visible !== true) continue;
@@ -3118,27 +3120,28 @@ function sortAxisCategoriesByValue(axList, gd) {
                     delete fullTrace._xautoBinFinished;
                     delete fullTrace._yautoBinFinished;
                 }
+                var isSplom = type === 'splom';
+                var isScattergl = type === 'scattergl';
 
                 var cd = gd.calcdata[traceIndex];
                 for(k = 0; k < cd.length; k++) {
                     var cdi = cd[k];
-                    var cat, catIndex, value;
+                    var catIndex, value;
 
-                    if(type === 'splom') {
+                    if(isSplom) {
                         // If `splom`, collect values across dimensions
                         // Find which dimension the current axis is representing
                         var currentDimensionIndex = fullTrace._axesDim[ax._id];
 
                         // Apply logic to associated x axis if it's defined
-                        if(axLetter === 'y') {
+                        if(!isX) {
                             var associatedXAxisID = fullTrace._diag[currentDimensionIndex][0];
                             if(associatedXAxisID) ax = gd._fullLayout[axisIDs.id2name(associatedXAxisID)];
                         }
 
                         var categories = cdi.trace.dimensions[currentDimensionIndex].values;
                         for(l = 0; l < categories.length; l++) {
-                            cat = categories[l];
-                            catIndex = ax._categoriesMap[cat];
+                            catIndex = ax._categoriesMap[categories[l]];
 
                             // Collect associated values at index `l` over all other dimensions
                             for(o = 0; o < cdi.trace.dimensions.length; o++) {
@@ -3147,18 +3150,14 @@ function sortAxisCategoriesByValue(axList, gd) {
                                 categoriesValue[catIndex][1].push(dimension.values[l]);
                             }
                         }
-                    } else if(type === 'scattergl') {
+                    } else if(isScattergl) {
                         // If `scattergl`, collect all values stashed under cdi.t
                         for(l = 0; l < cdi.t.x.length; l++) {
-                            if(axLetter === 'x') {
-                                cat = cdi.t.x[l];
-                                catIndex = cat;
+                            if(isX) {
+                                catIndex = cdi.t.x[l];
                                 value = cdi.t.y[l];
-                            }
-
-                            if(axLetter === 'y') {
-                                cat = cdi.t.y[l];
-                                catIndex = cat;
+                            } else {
+                                catIndex = cdi.t.y[l];
                                 value = cdi.t.x[l];
                             }
                             categoriesValue[catIndex][1].push(value);
@@ -3181,16 +3180,19 @@ function sortAxisCategoriesByValue(axList, gd) {
                         }
                     } else {
                         // For all other 2d cartesian traces
-                        if(axLetter === 'x') {
-                            cat = cdi.p + 1 ? cdi.p : cdi.x;
-                            value = cdi.s || cdi.v || cdi.y;
-                        } else if(axLetter === 'y') {
-                            cat = cdi.p + 1 ? cdi.p : cdi.y;
-                            value = cdi.s || cdi.v || cdi.x;
+                        catIndex = cdi.p;
+                        if(catIndex === undefined) catIndex = cdi[axLetter];
+
+                        value = cdi.s;
+                        if(value === undefined) value = cdi.v;
+                        if(value === undefined) value = isX ? cdi.y : cdi.x;
+
+                        if(!Array.isArray(value)) {
+                            if(value === undefined) value = [];
+                            else value = [value];
                         }
-                        if(!Array.isArray(value)) value = [value];
                         for(l = 0; l < value.length; l++) {
-                            categoriesValue[cat][1].push(value[l]);
+                            categoriesValue[catIndex][1].push(value[l]);
                         }
                     }
                 }

test/image/baselines/missing-category-order.png

@@ -0,0 +1,41 @@
+{
+    "data": [
+        {
+            "type": "histogram",
+            "x": [
+                "A",
+                "A",
+                "B"
+            ]
+        },
+        {
+            "type": "histogram",
+            "x": [
+                "A",
+                "A",
+                "C"
+            ]
+        },
+        {
+            "type": "histogram",
+            "x": [
+                "B",
+                "B"
+            ]
+        }
+    ],
+    "layout": {
+        "barmode": "stack",
+        "xaxis": {
+            "categoryorder": "total descending"
+        },
+        "width": 300,
+        "height": 200,
+        "margin": {
+            "t": 40,
+            "b": 20,
+            "l": 20,
+            "r": 20
+        }
+    }
+}

test/image/mocks/missing-category-order.json

@@ -737,6 +737,7 @@ var list = [
     'matching-missing-axes',
     'mathjax',
     'mirror-all-vs-allticks',
+    'missing-category-order',
     'multicategory',
     'multicategory_histograms',
     'multicategory-inside-ticks',
@@ -1809,6 +1810,7 @@ figs['matching-categories'] = require('@mocks/matching-categories');
 // figs['matching-missing-axes'] = require('@mocks/matching-missing-axes');
 // figs['mathjax'] = require('@mocks/mathjax');
 figs['mirror-all-vs-allticks'] = require('@mocks/mirror-all-vs-allticks');
+figs['missing-category-order'] = require('@mocks/missing-category-order');
 figs['multicategory'] = require('@mocks/multicategory');
 figs['multicategory_histograms'] = require('@mocks/multicategory_histograms');
 figs['multicategory-inside-ticks'] = require('@mocks/multicategory-inside-ticks');