Regression after fixing #18061 "Make theme color cycles consistent across chart plots"

[Claude Guyomard]

Dojox Charting

The issue ​https://bugs.dojotoolkit.org/ticket/18061 was fixed by ​https://github.com/dojo/dojox/pull/109/. This fix didn't take into account the case of ClusteredColumns?. Files attached show the problem :

ClusteredColumns?-columnInsertion-dojo110.html

Although all the series were iterated from last-to-first, columns are displayed in the right order :

• The 1st series is placed on the left with the the 1st color of the theme,
• The 2nd series is placed after the 1st one with the the 2nd color of the theme,
• and so on...

ClusteredColumns?-columnInsertion-dojo112.html

All the series are now iterated from first-to-last, columns are displayed in the wrong order :

• The 1st series is placed on the RIGHT with the the 1st color of the theme,
• The 2nd series is placed BEFORE the 1st one with the the 2nd color of the theme,
• and so on...

ClusteredColumns?-columnInsertion-dojo112-PATCHED.html

All the series are still iterated from first-to-last, columns are displayed in the right order :

• The 1st series is placed on the left with the the 1st color of the theme,
• The 2nd series is placed after the 1st one with the the 2nd color of the theme,
• and so on...

The reason of this is the incorrect management of the 'z' variable in Columns.js :

The Columns.render(dim, offsets) code is :

  ...
  var z = this.series.length;
  arr.forEach(this.series, function(serie){if(serie.hidden){z--;}});
  for(var i = 0; i < this.series.length; i++){
    ...
    if(run.hidden){
      run.dyn.fill = theme.series.fill;
      continue;
    }
    z--;
    ...
    x: offsets.l + ht(val.x + 0.5) + bar.gap + bar.thickness * z, // line 154, usage of 'z'
    ...
  }

It should be :

  ...
  var z = 0; // the non-hidden series index
  // REMOVED : arr.forEach(this.series, function(serie){if(serie.hidden){z--;}});
  for(var i = 0; i < this.series.length; i++){
    ...
    if(run.hidden){
      run.dyn.fill = theme.series.fill;
      continue;
    }
    // REMOVED z--;
    ...
    x: offsets.l + ht(val.x + 0.5) + bar.gap + bar.thickness * z, // OK, z == 0 at first loop
    ...
    z++; // ADD HERE. CANNOT BE PLACED IN for(;;) because the 'continue' above
  }

[Claude Guyomard]

Columns.js patch

[dylan]

Claude, do you care to open a pull request with this patch per the guidelines at ​https://github.com/dojo/dojo/blob/master/CONTRIBUTING.md ? This will get the fix reviewed and landed faster, and also gives you credit for your contribution.

[dylan]

PR added at ​https://github.com/dojo/dojox/pull/302 .