[Vis Colors] Update color mapper to prioritize unique colors per vis (#…

…4890) (#4898) * [Vis Colors] Update color mapper to prioritize unique colors per vis Instead of trying to generate unique colors for an entire dashboard Signed-off-by: Josh Romero <[email protected]> * Update CHANGELOG.md Signed-off-by: Josh Romero <[email protected]> --------- Signed-off-by: Josh Romero <[email protected]> (cherry picked from commit abcef34) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Sep 1, 2023 · 318c537 · 318c537
1 parent b2b338e
commit 318c537
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 14 deletions.
diff --git a/src/plugins/charts/public/services/colors/colors.ts b/src/plugins/charts/public/services/colors/colors.ts
@@ -64,7 +64,7 @@ export class ColorsService {
   ) {
     if (!Array.isArray(arrayOfStringsOrNumbers)) {
       throw new Error(
-        `createColorLookupFunction expects an array but recived: ${typeof arrayOfStringsOrNumbers}`
+        `createColorLookupFunction expects an array but received: ${typeof arrayOfStringsOrNumbers}`
       );
     }
 

diff --git a/src/plugins/charts/public/services/colors/mapped_colors.ts b/src/plugins/charts/public/services/colors/mapped_colors.ts
@@ -83,34 +83,36 @@ export class MappedColors {
     const configColors = _.values(configMapping);
     const oldColors = _.values(this._oldMap);
 
+    const alreadyUsedColors: string[] = [];
     const keysToMap: Array<string | number> = [];
     _.each(keys, (key) => {
       // If this key is mapped in the config, it's unnecessary to have it mapped here
-      if (configMapping[key as any]) delete this._mapping[key];
+      if (configMapping[key as any]) {
+        delete this._mapping[key];
+        alreadyUsedColors.push(configMapping[key]);
+      }
 
       // If this key is mapped to a color used by the config color mapping, we need to remap it
       if (_.includes(configColors, this._mapping[key])) keysToMap.push(key);
 
       // if key exist in oldMap, move it to mapping
-      if (this._oldMap[key]) this._mapping[key] = this._oldMap[key];
+      if (this._oldMap[key]) {
+        this._mapping[key] = this._oldMap[key];
+        alreadyUsedColors.push(this._mapping[key]);
+      }
 
       // If this key isn't mapped, we need to map it
       if (this.get(key) == null) keysToMap.push(key);
     });
 
-    // Generate a color palette big enough that all new keys can have unique color values
-    const allColors = _(this._mapping).values().union(configColors).union(oldColors).value();
-    const numColors = allColors.length + keysToMap.length;
+    // Choose colors from euiPaletteColorBlind and filter out any already assigned to keys
     const colorPalette = euiPaletteColorBlind({
-      rotations: Math.ceil(numColors / 10),
+      rotations: Math.ceil(keys.length / 10),
       direction: 'both',
-    }).slice(0, numColors);
-    let newColors = _.difference(colorPalette, allColors);
+    })
+      .filter((color) => !alreadyUsedColors.includes(color.toLowerCase()))
+      .slice(0, keysToMap.length);
 
-    while (keysToMap.length > newColors.length) {
-      newColors = newColors.concat(_.sampleSize(allColors, keysToMap.length - newColors.length));
-    }
-
-    _.merge(this._mapping, _.zipObject(keysToMap, newColors));
+    _.merge(this._mapping, _.zipObject(keysToMap, colorPalette));
   }
 }