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

…pensearch-project#4890) * [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]>
joshuarrrr · Sep 1, 2023 · abcef34 · abcef34
1 parent 83cd7f6
commit abcef34
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Home] Add modal to introduce the `next` theme ([#4715](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4715))
 - Remove visualization editor sidebar background ([#4719](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4719))
 - [Vis Colors] Remove customized colors from sample visualizations and dashboards ([#4741](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4741))
+- [Vis Colors] Update color mapper to prioritize unique colors per visualization rather than across entire dashboard ([#4890](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4890))
 - [Decouple] Allow plugin manifest config to define semver compatible OpenSearch plugin and verify if it is installed on the cluster([#4612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4612))
 - Adds Data explorer framework and implements Discover using it ([#4806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4806))
 - [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854/))
@@ -764,4 +765,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### 🔩 Tests
 
-- Update caniuse to fix failed integration tests ([#2322](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2322))
+- Update caniuse to fix failed integration tests ([#2322](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2322))
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));
   }
 }