From 318c537efac5004bcd63ca79bb44434df7b95c9d Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Fri, 1 Sep 2023 14:54:21 -0700 Subject: [PATCH] [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 * Update CHANGELOG.md Signed-off-by: Josh Romero --------- Signed-off-by: Josh Romero (cherry picked from commit abcef341b1c912344f67ec3a6ba3a9baded7d468) Signed-off-by: github-actions[bot] # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] --- .../charts/public/services/colors/colors.ts | 2 +- .../public/services/colors/mapped_colors.ts | 28 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/plugins/charts/public/services/colors/colors.ts b/src/plugins/charts/public/services/colors/colors.ts index 1706f67acf97..124931c30ea7 100644 --- 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 index bc8ad8640985..a6051bf2b30f 100644 --- 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 = []; _.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)); } }