ensure that Plots getSubplotIds return ordered arrays of ids

Author: etpinard

src/plots/plots.js

@@ -174,8 +174,19 @@ plots.findSubplotIds = function findSubplotIds(data, type) {
     return subplotIds;
 };
 
+/**
+ * Get the ids of the current subplots.
+ *
+ * @param {object} layout plotly full layout object.
+ * @param {string} type subplot type to look for.
+ *
+ * @return {array} list of ordered subplot ids (strings).
+ *
+ */
 plots.getSubplotIds = function getSubplotIds(layout, type) {
-    if(plots.subplotsRegistry[type] === undefined) return [];
+    var _module = plots.subplotsRegistry[type];
+
+    if(_module === undefined) return [];
 
     // layout must be 'fullLayout' here
     if(type === 'cartesian' && !layout._hasCartesian) return [];
@@ -184,19 +195,23 @@ plots.getSubplotIds = function getSubplotIds(layout, type) {
         return Object.keys(layout._plots);
     }
 
-    var idRegex = plots.subplotsRegistry[type].idRegex,
+    var idRegex = _module.idRegex,
         layoutKeys = Object.keys(layout),
-        subplotIds = [],
-        layoutKey;
+        subplotIds = [];
 
     for(var i = 0; i < layoutKeys.length; i++) {
-        layoutKey = layoutKeys[i];
+        var layoutKey = layoutKeys[i];
+
         if(idRegex.test(layoutKey)) subplotIds.push(layoutKey);
     }
 
-    return subplotIds;
-};
-
+    // order the ids
+    var idLen = _module.idRoot.length;
+    subplotIds.sort(function(a, b) {
+        var aNum = +(a.substr(idLen) || 1),
+            bNum = +(b.substr(idLen) || 1);
+        return aNum - bNum;
+    });
 
     return subplotIds;
 };

test/jasmine/tests/plots_test.js

@@ -102,12 +102,11 @@ describe('Test Plots', function() {
 
     describe('Plots.getSubplotIds', function() {
         var getSubplotIds = Plots.getSubplotIds;
-        var layout;
 
-        it('returns scene ids', function() {
-            layout = {
-                scene: {},
+        it('returns scene ids in order', function() {
+            var layout = {
                 scene2: {},
+                scene: {},
                 scene3: {}
             };
 
@@ -116,13 +115,32 @@ describe('Test Plots', function() {
 
             expect(getSubplotIds(layout, 'cartesian'))
                 .toEqual([]);
+            expect(getSubplotIds(layout, 'geo'))
+                .toEqual([]);
+            expect(getSubplotIds(layout, 'no-valid-subplot-type'))
+                .toEqual([]);
+        });
+
+        it('returns geo ids in order', function() {
+            var layout = {
+                geo2: {},
+                geo: {},
+                geo3: {}
+            };
+
+            expect(getSubplotIds(layout, 'geo'))
+                .toEqual(['geo', 'geo2', 'geo3']);
 
+            expect(getSubplotIds(layout, 'cartesian'))
+                .toEqual([]);
+            expect(getSubplotIds(layout, 'gl3d'))
+                .toEqual([]);
             expect(getSubplotIds(layout, 'no-valid-subplot-type'))
                 .toEqual([]);
         });
 
         it('returns cartesian ids', function() {
-            layout = {
+            var layout = {
                 _plots: { xy: {}, x2y2: {} }
             };
 
@@ -167,7 +185,6 @@ describe('Test Plots', function() {
             ids = findSubplotIds(data, 'gl3d');
             expect(ids).toEqual(['scene', 'scene2']);
         });
-
     });
 
     describe('Plots.register, getModule, and traceIs', function() {