Merge pull request #1419 from CartoDB/fix-min-sub-palette

Fix best subpalette detection
CartoDB · Dec 20, 2019 · 2cdc485 · 2cdc485
2 parents 22c95ba + 3192d7a
commit 2cdc485
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 11 deletions.
diff --git a/src/renderer/viz/expressions/color/palettes/Palette.js b/src/renderer/viz/expressions/color/palettes/Palette.js
@@ -3,8 +3,6 @@ import { hexToRgb } from '../../utils';
 import { RGBA } from '../rgb';
 import { constant } from '../../../expressions';
 
-const MIN_CARTOCOLOR_SUBPALETTE_SIZE = 2;
-
 /**
  * Color palettes.
  *
@@ -70,23 +68,33 @@ export default class Palette extends BaseExpression {
     }
 
     _getBestSubPalette (subPaletteIndex) {
-        subPaletteIndex = subPaletteIndex <= MIN_CARTOCOLOR_SUBPALETTE_SIZE
-            ? MIN_CARTOCOLOR_SUBPALETTE_SIZE
-            : subPaletteIndex;
-        const longestSubPalette = this.getLongestSubPalette();
-        const subPalette = (subPaletteIndex < longestSubPalette.length
-            ? [...this.subPalettes[subPaletteIndex]]
-            : [...longestSubPalette]);
+        const longestSubPaletteIndex = this.getLongestSubPaletteIndex();
+        const smallestSubPaletteIndex = this.getSmallestSubPaletteIndex();
+        if (!Number.isInteger(subPaletteIndex) || subPaletteIndex > longestSubPaletteIndex) {
+            subPaletteIndex = longestSubPaletteIndex;
+        } else if (subPaletteIndex < smallestSubPaletteIndex) {
+            subPaletteIndex = smallestSubPaletteIndex;
+        }
+        const subPalette = [...this.subPalettes[subPaletteIndex]];
         return subPalette.map(color =>
             new RGBA(constant(color.r), constant(color.g), constant(color.b), constant(color.a))
         );
     }
 
-    getLongestSubPalette () {
+    getSmallestSubPaletteIndex () {
+        const s = this.subPalettes;
+        for (let i = 0; i <= 20; i++) {
+            if (s[i]) {
+                return i;
+            }
+        }
+    }
+
+    getLongestSubPaletteIndex () {
         const s = this.subPalettes;
         for (let i = 20; i >= 0; i--) {
             if (s[i]) {
-                return s[i];
+                return i;
             }
         }
     }

diff --git a/test/integration/render/scenarios/regression/cartocolor-subpalette/reference.png b/test/integration/render/scenarios/regression/cartocolor-subpalette/reference.png
diff --git a/test/integration/render/scenarios/regression/cartocolor-subpalette/scenario.js b/test/integration/render/scenarios/regression/cartocolor-subpalette/scenario.js
@@ -0,0 +1,19 @@
+const map = new CartoMap({
+    container: 'map',
+    background: 'black'
+});
+
+const source = new carto.source.GeoJSON(sources['points3']);
+const viz = new carto.Viz(`
+    color: ramp(top(@prop, @num), CB_BLUES)
+    width: 50
+    @prop: $cat
+    @num: 2
+`);
+const layer = new carto.Layer('layer', source, viz);
+
+viz.variables.num.blendTo(2, 0);
+layer.addTo(map);
+layer.on('loaded', () => {
+    window.loaded = true;
+});