Skip to content

Commit

Permalink
[Vis Colors] Update legacy seed colors to use ouiPaletteColorBlind() (
Browse files Browse the repository at this point in the history
#4348)

Signed-off-by: Manasvini B Suryanarayana <manasvis@amazon.com>
  • Loading branch information
manasvinibs committed Jun 28, 2023
1 parent 29d7a4b commit 88e2b2d
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 135 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Vis Colors] [Maps] Replace hardcoded color to OUI color in `maps_legacy` plugin ([#4294](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4294))
- [Vis Colors] [TSVB] Update default color in `vis_type_timeseries` to use `ouiPaletteColorBlind()[0]`([#4363](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4363))
- [Vis Colors] [Timeline] Replace `vis_type_timeline` colors with `ouiPaletteColorBlind()` ([#4366](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4366))
- [Vis Colors] Update legacy seed colors to use `ouiPaletteColorBlind()` ([#4348](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4348))

### 🔩 Tests

Expand Down
38 changes: 3 additions & 35 deletions src/plugins/charts/public/services/colors/color_palette.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,48 +29,16 @@
*/

import _ from 'lodash';
import { hsl } from 'color';

import { seedColors } from './seed_colors';

const offset = 300; // Hue offset to start at

const fraction = function (goal: number) {
const walkTree = (numerator: number, denominator: number, bytes: number[]): number => {
if (bytes.length) {
return walkTree(numerator * 2 + (bytes.pop() ? 1 : -1), denominator * 2, bytes);
} else {
return numerator / denominator;
}
};

const b = (goal + 2)
.toString(2)
.split('')
.map(function (num) {
return parseInt(num, 10);
});
b.shift();

return walkTree(1, 2, b);
};
import { euiPaletteColorBlind } from '@elastic/eui';

/**
* Generates an array of hex colors the length of the input number.
* If the number is greater than the length of seed colors available,
* new colors are generated up to the value of the input number.
* Generates an array of hex colors the length of the input number
*/
export function createColorPalette(num: number): string[] {
if (!_.isNumber(num)) {
throw new TypeError('ColorPaletteUtilService expects a number');
}

const colors = seedColors;
const seedLength = seedColors.length;

_.times(num - seedLength, function (i) {
colors.push(hsl((fraction(i + seedLength + 1) * 360 + offset) % 360, 50, 50).hex());
});

return colors;
return euiPaletteColorBlind({ rotations: Math.ceil(num / 10), direction: 'both' }).slice(0, num);
}
4 changes: 2 additions & 2 deletions src/plugins/charts/public/services/colors/colors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

import { coreMock } from '../../../../../core/public/mocks';
import { COLOR_MAPPING_SETTING } from '../../../common';
import { seedColors } from './seed_colors';
import { euiPaletteColorBlind } from '@elastic/eui';
import { ColorsService } from './colors';

// Local state for config
Expand Down Expand Up @@ -138,7 +138,7 @@ describe('Vislib Color Service', () => {
});

it('should return the first hex color in the seed colors array', () => {
expect(color(arr[0])).toBe(seedColors[0]);
expect(color(arr[0])).toBe(euiPaletteColorBlind()[0]);
});

it('should return the value from the mapped colors', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/charts/public/services/colors/colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import _ from 'lodash';

import { CoreSetup } from 'opensearch-dashboards/public';

import { euiPaletteColorBlind } from '@elastic/eui';
import { MappedColors } from './mapped_colors';
import { seedColors } from './seed_colors';

/**
* Accepts an array of strings or numbers that are used to create a
Expand All @@ -44,7 +44,7 @@ import { seedColors } from './seed_colors';
export class ColorsService {
private _mappedColors?: MappedColors;

public readonly seedColors = seedColors;
public readonly seedColors = euiPaletteColorBlind();

public get mappedColors() {
if (!this._mappedColors) {
Expand Down
17 changes: 13 additions & 4 deletions src/plugins/charts/public/services/colors/colors_palette.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
* under the License.
*/

import { seedColors } from './seed_colors';
import { createColorPalette } from './color_palette';
import { euiPaletteColorBlind } from '@elastic/eui';

describe('Color Palette', () => {
const num1 = 45;
Expand All @@ -46,6 +46,12 @@ describe('Color Palette', () => {
colorPalette = createColorPalette(num1);
});

function isHexValue(value: string): boolean {
// Check if the hex value is valid.
const regex = /^#[0-9a-fA-F]{3}|[0-9a-fA-F]{6}$/;
return regex.test(value) ? true : false;
}

it('should throw an error if input is not a number', () => {
expect(() => {
// @ts-expect-error
Expand Down Expand Up @@ -91,18 +97,21 @@ describe('Color Palette', () => {
});

it('should return the seed color array when input length is 72', () => {
expect(createColorPalette(num2)[71]).toBe(seedColors[71]);
expect(createColorPalette(num2).length).toBe(num2);
expect(isHexValue(createColorPalette(num2)[71])).toBe(true);
});

it('should return an array of the same length as the input when input is greater than 72', () => {
expect(createColorPalette(num3).length).toBe(num3);
});

it('should create new darker colors when input is greater than 72', () => {
expect(createColorPalette(num3)[72]).not.toEqual(seedColors[0]);
expect(createColorPalette(num3)[72]).not.toEqual(euiPaletteColorBlind()[0]);
});

it('should create new colors and convert them correctly', () => {
expect(createColorPalette(num3)[72]).toEqual('#404ABF');
expect(createColorPalette(num3).length).toBe(num3);
expect(createColorPalette(num3)[72]).not.toEqual(euiPaletteColorBlind()[9]);
expect(isHexValue(createColorPalette(num3)[89])).toBe(true);
});
});
16 changes: 8 additions & 8 deletions src/plugins/charts/public/services/colors/mapped_colors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import Color from 'color';

import { coreMock } from '../../../../../core/public/mocks';
import { COLOR_MAPPING_SETTING } from '../../../common';
import { seedColors } from './seed_colors';
import { euiPaletteColorBlind } from '@elastic/eui';
import { MappedColors } from './mapped_colors';

// Local state for config
Expand Down Expand Up @@ -65,31 +65,31 @@ describe('Mapped Colors', () => {
});

it('should not include colors used by the config', () => {
const newConfig = { bar: seedColors[0] };
const newConfig = { bar: euiPaletteColorBlind()[9] };
config.set(COLOR_MAPPING_SETTING, newConfig);

const arr = ['foo', 'baz', 'qux'];
mappedColors.mapKeys(arr);

const colorValues = _(mappedColors.mapping).values();
expect(colorValues).not.toContain(seedColors[0]);
expect(colorValues).not.toContain(euiPaletteColorBlind()[9]);
expect(colorValues.uniq().size()).toBe(arr.length);
});

it('should create a unique array of colors even when config is set', () => {
const newConfig = { bar: seedColors[0] };
const newConfig = { bar: euiPaletteColorBlind()[9] };
config.set(COLOR_MAPPING_SETTING, newConfig);

const arr = ['foo', 'bar', 'baz', 'qux'];
mappedColors.mapKeys(arr);

const expectedSize = _(arr).difference(_.keys(newConfig)).size();
expect(_(mappedColors.mapping).values().uniq().size()).toBe(expectedSize);
expect(mappedColors.get(arr[0])).not.toBe(seedColors[0]);
expect(mappedColors.get(arr[0])).not.toBe(euiPaletteColorBlind()[9]);
});

it('should treat different formats of colors as equal', () => {
const color = new Color(seedColors[0]);
const color = new Color(euiPaletteColorBlind()[9]);
const rgb = `rgb(${color.red()}, ${color.green()}, ${color.blue()})`;
const newConfig = { bar: rgb };
config.set(COLOR_MAPPING_SETTING, newConfig);
Expand All @@ -99,8 +99,8 @@ describe('Mapped Colors', () => {

const expectedSize = _(arr).difference(_.keys(newConfig)).size();
expect(_(mappedColors.mapping).values().uniq().size()).toBe(expectedSize);
expect(mappedColors.get(arr[0])).not.toBe(seedColors[0]);
expect(mappedColors.get('bar')).toBe(seedColors[0]);
expect(mappedColors.get(arr[0])).not.toBe(euiPaletteColorBlind()[9]);
expect(mappedColors.get('bar')).toBe(euiPaletteColorBlind()[9].toLowerCase());
});

it('should have a flush method that moves the current map to the old map', function () {
Expand Down
37 changes: 0 additions & 37 deletions src/plugins/charts/public/services/colors/seed_colors.test.ts

This file was deleted.

44 changes: 0 additions & 44 deletions src/plugins/charts/public/services/colors/seed_colors.ts

This file was deleted.

6 changes: 3 additions & 3 deletions test/functional/apps/dashboard/dashboard_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,14 @@ export default function ({ getService, getPageObjects }) {

await retry.try(async () => {
const pieSliceStyle = await pieChart.getPieSliceStyle('80,000');
// The default green color that was stored with the visualization before any dashboard overrides.
expect(pieSliceStyle.indexOf('rgb(87, 193, 123)')).to.be.greaterThan(0);
// The default color that was stored with the visualization before any dashboard overrides.
expect(pieSliceStyle.indexOf('rgb(84, 179, 153)')).to.be.greaterThan(0);
});
});

it('resets the legend color as well', async function () {
await retry.try(async () => {
const colorExists = await PageObjects.visChart.doesSelectedLegendColorExist('#57c17b');
const colorExists = await PageObjects.visChart.doesSelectedLegendColorExist('#54B399');
expect(colorExists).to.be(true);
});
});
Expand Down

0 comments on commit 88e2b2d

Please sign in to comment.