From abcef341b1c912344f67ec3a6ba3a9baded7d468 Mon Sep 17 00:00:00 2001 From: Josh Romero Date: Fri, 1 Sep 2023 13:14:29 -0700 Subject: [PATCH] [Vis Colors] Update color mapper to prioritize unique colors per vis (#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 * Update CHANGELOG.md Signed-off-by: Josh Romero --------- Signed-off-by: Josh Romero --- CHANGELOG.md | 3 +- .../charts/public/services/colors/colors.ts | 2 +- .../public/services/colors/mapped_colors.ts | 28 ++++++++++--------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86bfda90100c..ec6ad0a63e07 100644 --- 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)) \ No newline at end of file +- 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 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)); } }