[fix] custom palette issues (#2850)

* 1 update fix of custom palette issues 2 restrict custom steps to 20, hide + button if needed 3 update scale function to handle some special cases: not enough k groups
* add name|type|category to updateCustomPalette if customBreaks
* add test cases for redesigned color range feature
* update test cases to increase coverage

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
Co-authored-by: Ilya Boyandin <iboyandin@foursquare.com>

src/components/src/side-panel/layer-panel/custom-palette.tsx

@@ -72,6 +72,7 @@ export type CustomPaletteInputProps = {
   inputColorHex: (index: number, v: HexColor) => void;
   editColorMapValue: EditColorMapFunc;
   actionIcons?: ActionIcons;
+  disableAppend?: boolean;
   disableDelete?: boolean;
   onDelete: (index: number) => void;
   onAdd: (index: number) => void;
@@ -353,6 +354,7 @@ export const CustomPaletteInput: React.FC<CustomPaletteInputProps> = ({
   inputColorHex,
   editColorMapValue,
   actionIcons = defaultActionIcons,
+  disableAppend,
   disableDelete,
   onDelete,
   onAdd,
@@ -394,7 +396,9 @@ export const CustomPaletteInput: React.FC<CustomPaletteInputProps> = ({
         ) : null}
       </div>
       <div className="custom-palette-input__right">
-        <AddColorStop onColorAdd={onColorAdd} IconComponent={actionIcons.add} />
+        {!disableAppend ? (
+          <AddColorStop onColorAdd={onColorAdd} IconComponent={actionIcons.add} />
+        ) : null}
         {!disableDelete ? (
           <DeleteColorStop onColorDelete={onColorDelete} IconComponent={actionIcons.delete} />
         ) : null}
@@ -555,6 +559,7 @@ function CustomPaletteFactory(): React.FC<CustomPaletteProps> {
               isSorting={isSorting}
               color={color}
               inputColorHex={inputColorHex}
+              disableAppend={colors.length >= 20}
               disableDelete={colors.length <= 2}
               actionIcons={actionIcons}
               onAdd={onAdd}

src/layers/src/base-layer.ts

@@ -65,7 +65,11 @@ import {
   RGBColor,
   ValueOf
 } from '@kepler.gl/types';
-import {getScaleFunction, initializeLayerColorMap} from '@kepler.gl/utils';
+import {
+  getScaleFunction,
+  initializeLayerColorMap,
+  updateCustomColorRangeByColorUI
+} from '@kepler.gl/utils';
 import memoize from 'lodash.memoize';
 import {isDomainQuantile, getDomainStepsbyZoom, getThresholdsFromQuantiles} from '@kepler.gl/utils';
 
@@ -914,6 +918,11 @@ class Layer {
         Console.warn(`updateColorUI: Can't find visual channel which range is ${prop}`);
         return;
       }
+      // add name|type|category to updateCustomPalette if customBreaks, so that
+      // colors will not be override as well when inverse palette with custom break
+      customPalette.name = DEFAULT_CUSTOM_PALETTE.name;
+      customPalette.type = DEFAULT_CUSTOM_PALETTE.type;
+      customPalette.category = DEFAULT_CUSTOM_PALETTE.category;
       // initiate colorMap from current scale
       customPalette.colorMap = initializeLayerColorMap(this, visualChannels[channelKey]);
     } else if (newConfig.colorRangeConfig.custom) {
@@ -940,17 +949,27 @@ class Layer {
    * @param {*} prop
    */
   updateColorUIByColorRange(newConfig, prop) {
-    if (typeof newConfig.showDropdown !== 'number') return;
-
     const {colorUI, visConfig} = this.config;
+
+    // when custom palette adds/removes step, the number in "Steps" input control
+    // should be updated as well
+    const isCustom = newConfig.customPalette?.category === 'Custom';
+    const customStepsChanged = isCustom
+      ? newConfig.customPalette.colors.length !== visConfig[prop].colors.length
+      : false;
+
+    if (typeof newConfig.showDropdown !== 'number' && !customStepsChanged) return;
+
     this.updateLayerConfig({
       colorUI: {
         ...colorUI,
         [prop]: {
           ...colorUI[prop],
           colorRangeConfig: {
             ...colorUI[prop].colorRangeConfig,
-            steps: visConfig[prop].colors.length,
+            steps: customStepsChanged
+              ? colorUI[prop].customPalette.colors.length
+              : visConfig[prop].colors.length,
             reversed: Boolean(visConfig[prop].reversed)
           }
         }
@@ -972,10 +991,16 @@ class Layer {
 
     const {colorUI, visConfig} = this.config;
 
-    const update = updateColorRangeByMatchingPalette(
-      visConfig[prop],
-      colorUI[prop].colorRangeConfig
-    );
+    // for custom palette, one can only 'reverse' the colors in custom palette.
+    // changing 'steps', 'colorBindSafe', 'type' should fall back to predefined palette.
+    const isCustomColorReversed =
+      visConfig.colorRange.category === 'Custom' &&
+      newConfig.colorRangeConfig &&
+      Object.prototype.hasOwnProperty.call(newConfig.colorRangeConfig, 'reversed');
+
+    const update = isCustomColorReversed
+      ? updateCustomColorRangeByColorUI(visConfig[prop], colorUI[prop].colorRangeConfig)
+      : updateColorRangeByMatchingPalette(visConfig[prop], colorUI[prop].colorRangeConfig);
 
     if (update) {
       this.updateLayerVisConfig({[prop]: update});

src/utils/src/color-utils.ts

@@ -365,29 +365,48 @@ export function updateColorRangeByMatchingPalette(
 }
 
 /**
- * Update color range after selecting a palette from color range selectoer
- * Copy over colorMap and colorLegends
+ * Update custom palette when reverse the colors in custom palette, since changing 'steps',
+ * 'colorBindSafe', 'type' should fall back to predefined palette.
  */
-export function updateColorRangeBySelectedPalette(
+export function updateCustomColorRangeByColorUI(
   oldColorRange: ColorRange,
-  colorPalette: ColorPalette,
-  colorConfig: {
-    reversed: boolean;
-    steps: number;
-  }
+  colorConfig: ColorRangeConfig
 ): ColorRange {
-  // const {reversed} = colorConfig;
+  const {reversed} = colorConfig;
+  const colors = oldColorRange.colors;
+  // for custom palette, one can only 'reverse' the colors in custom palette.
+  colors.reverse();
 
   const colorRange = {
-    ...colorPaletteToColorRange(colorPalette, colorConfig),
-    // ...(reversed ? {reversed} : {}),
+    name: oldColorRange.name,
+    type: oldColorRange.type,
+    category: oldColorRange.category,
+    colors,
+    ...(reversed ? {reversed} : {}),
     ...(oldColorRange.colorMap ? {colorMap: oldColorRange.colorMap} : {}),
     ...(oldColorRange.colorLegends ? {colorLegends: oldColorRange.colorLegends} : {})
   };
 
   return replaceColorsInColorRange(colorRange, colorRange.colors);
 }
 
+/**
+ * Update color range after selecting a palette from color range selectoer
+ * Copy over colorMap and colorLegends
+ */
+ export function updateColorRangeBySelectedPalette(oldColorRange, colorPalette, colorConfig) {
+  const {colors: newColors, ...newColorRange} = colorPaletteToColorRange(colorPalette, colorConfig);
+
+  const colorRange = {
+    colors: oldColorRange.colors,
+    ...newColorRange,
+    ...(oldColorRange.colorMap ? {colorMap: oldColorRange.colorMap} : {}),
+    ...(oldColorRange.colorLegends ? {colorLegends: oldColorRange.colorLegends} : {})
+  };
+
+  return replaceColorsInColorRange(colorRange, newColors);
+}
+
 const UberNameRegex = new RegExp(/^([A-Za-z ])+/g);
 const ColorBrewerRegex = new RegExp(/^ColorBrewer ([A-Za-z1-9])+/g);
 

src/utils/src/data-scale-utils.ts

@@ -218,7 +218,12 @@ export function getQuantLegends(scale: D3ScaleFunction, labelFormat: LabelFormat
   }
   const labels =
     scale.scaleType === 'threshold' || scale.scaleType === 'custom'
-      ? getThresholdLabels(scale, customScaleLabelFormat)
+      ? getThresholdLabels(
+          scale,
+          scale.scaleType === 'custom'
+            ? customScaleLabelFormat
+            : n => (n ? formatNumber(n) : 'no value')
+        )
       : getScaleLabels(scale, labelFormat);
 
   const data = scale.range();
@@ -256,7 +261,7 @@ export function getQuantLabelFormat(domain, fieldType) {
     ? getTimeLabelFormat(domain)
     : !fieldType
     ? defaultFormat
-    : n => formatNumber(n, fieldType);
+    : n => (n ? formatNumber(n, fieldType) : 'no value');
 }
 
 /**

src/utils/src/index.ts

@@ -22,7 +22,8 @@ export {
   paletteIsSteps,
   paletteIsType,
   paletteIsColorBlindSafe,
-  updateColorRangeByMatchingPalette
+  updateColorRangeByMatchingPalette,
+  updateCustomColorRangeByColorUI
 } from './color-utils';
 export {errorNotification} from './notifications-utils';
 

test/browser/components/side-panel/channel-by-value-selctor-test.js

@@ -44,9 +44,9 @@ function nop() {
 
 const ExpectedCustomPalette = {
   customPalette: {
-    name: 'Uber Viz Sequential',
-    type: 'sequential',
-    category: 'Uber',
+    name: 'color.customPalette',
+    type: 'custom',
+    category: 'Custom',
     colors: expectedColorRangeInLayer.colors,
     colorMap: [
       [3032, expectedColorRangeInLayer.colors[0]],