Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve color consistency #17792

Closed
wants to merge 41 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
a2cdddd
feat: support color consistency
stephenLYZ Dec 17, 2021
2d4fc05
restore
stephenLYZ Dec 20, 2021
671fc1d
restore
stephenLYZ Dec 20, 2021
ccd7b41
add chartId
stephenLYZ Jan 4, 2022
1980fd4
Update superset-frontend/src/dashboard/actions/dashboardInfo.ts
stephenLYZ Jan 7, 2022
f0e7bae
fix: lint
stephenLYZ Jan 11, 2022
3e442ea
fix: lint
stephenLYZ Jan 11, 2022
94b0f3a
fix: lint
stephenLYZ Jan 12, 2022
34b9345
code cov and rename to sharedColorScale
stephenLYZ Jan 18, 2022
3f09b80
prettier
stephenLYZ Jan 18, 2022
21c4951
fix codecov
stephenLYZ Jan 18, 2022
4dae812
lint
stephenLYZ Jan 19, 2022
ecaf7fd
Merge branch 'master' into feat-color-consistency
stephenLYZ Jan 19, 2022
f6b1558
Merge branch 'feat-color-consistency' of github.com:stephenLYZ/supers…
stephenLYZ Jan 19, 2022
036beb3
Merge branch 'master' into feat-color-consistency
stephenLYZ Jan 30, 2022
eeec951
fix: shared_label_color not update from json editor
stephenLYZ Jan 30, 2022
fe81606
hidden shared_label_color and update when delete chart
stephenLYZ Feb 8, 2022
1cff786
move delete to dashboardState
stephenLYZ Feb 9, 2022
bfaf5b8
Merge branch 'master' into feat-color-consistency
stephenLYZ Feb 9, 2022
bd58e44
update metadat when add chart
stephenLYZ Feb 9, 2022
cafa041
add test and use makeSingleton
stephenLYZ Feb 9, 2022
fa26094
Merge branch 'master' into feat-color-consistency
stephenLYZ Feb 10, 2022
40483d5
fix some problems
stephenLYZ Feb 14, 2022
e145f4a
revert
stephenLYZ Feb 14, 2022
c332f30
make sliceId option
stephenLYZ Feb 14, 2022
1741f1c
Merge branch 'master' into feat-color-consistency
stephenLYZ Feb 14, 2022
788f971
ts error
stephenLYZ Feb 15, 2022
a02c4c7
add rest charts
stephenLYZ Feb 15, 2022
3040788
fix cannot save
stephenLYZ Feb 15, 2022
f391050
support map charts
stephenLYZ Feb 20, 2022
a2962ad
lint
stephenLYZ Feb 20, 2022
c6378c5
put update color scheme to postTransformProps
stephenLYZ Feb 20, 2022
b765c0d
lint
stephenLYZ Feb 20, 2022
3d8d3e3
fix test
stephenLYZ Feb 21, 2022
396e4a1
clean up label color when dashboard unmounts
stephenLYZ Feb 22, 2022
e6bee06
add tinycolor to prevent color conflicts
stephenLYZ Feb 23, 2022
e5839fa
Merge branch 'master' into feat-color-consistency
stephenLYZ Feb 24, 2022
35c7257
fix test
stephenLYZ Feb 24, 2022
4c01844
Merge branch 'master' into feat-color-consistency
stephenLYZ Feb 24, 2022
13ae5f8
fix: color confilcts if shared label less than 3
stephenLYZ Feb 28, 2022
ba02558
Merge branch 'master' into feat-color-consistency
stephenLYZ Mar 2, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,5 @@ release.json
messages.mo

docker/requirements-local.txt

cache/
26 changes: 20 additions & 6 deletions superset-frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@
"rison": "^0.1.1",
"scroll-into-view-if-needed": "^2.2.28",
"shortid": "^2.2.6",
"tinycolor2": "^1.4.2",
"urijs": "^1.19.8",
"use-immer": "^0.6.0",
"use-query-params": "^1.1.9",
Expand Down Expand Up @@ -262,6 +263,7 @@
"@types/rison": "0.0.6",
"@types/shortid": "^0.0.29",
"@types/sinon": "^9.0.5",
"@types/tinycolor2": "^1.4.3",
"@types/yargs": "12 - 15",
"@typescript-eslint/eslint-plugin": "^5.3.0",
"@typescript-eslint/parser": "^5.3.0",
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/packages/superset-ui-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"@types/math-expression-evaluator": "^1.2.1",
"@types/rison": "0.0.6",
"@types/seedrandom": "^2.4.28",
"@types/tinycolor2": "^1.4.3",
"@types/fetch-mock": "^7.3.3",
"@types/enzyme": "^3.10.5",
"@types/prop-types": "^15.7.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import { scaleOrdinal, ScaleOrdinal } from 'd3-scale';
import { ExtensibleFunction } from '../models';
import { ColorsLookup } from './types';
import stringifyAndTrim from './stringifyAndTrim';
import getSharedLabelColor from './SharedLabelColorSingleton';

// Use type augmentation to correct the fact that
// an instance of CategoricalScale is also a function

interface CategoricalColorScale {
(x: { toString(): string }): string;
(x: { toString(): string }, y?: number): string;
}

class CategoricalColorScale extends ExtensibleFunction {
Expand All @@ -46,7 +46,7 @@ class CategoricalColorScale extends ExtensibleFunction {
* (usually CategoricalColorNamespace) and supersede this.forcedColors
*/
constructor(colors: string[], parentForcedColors?: ColorsLookup) {
super((value: string) => this.getColor(value));
super((value: string, sliceId?: number) => this.getColor(value, sliceId));

this.colors = colors;
this.scale = scaleOrdinal<{ toString(): string }, string>();
Expand All @@ -55,8 +55,11 @@ class CategoricalColorScale extends ExtensibleFunction {
this.forcedColors = {};
}

getColor(value?: string) {
getColor(value?: string, sliceId?: number) {
const cleanedValue = stringifyAndTrim(value);

getSharedLabelColor().addSlice(cleanedValue, sliceId);

const parentColor =
this.parentForcedColors && this.parentForcedColors[cleanedValue];
if (parentColor) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import tinycolor from 'tinycolor2';
import { CategoricalColorNamespace } from '.';
import makeSingleton from '../utils/makeSingleton';

export class SharedLabelColor {
values: string[];

valueSliceMap: Record<string, number[]>;

constructor() {
// save all shared label value
this.values = [];
// { value1: [sliceId1, sliceId2], value2: [sliceId], ...}
this.valueSliceMap = {};
}

getColorMap(colorNamespace?: string, colorScheme?: string) {
if (colorScheme) {
const categoricalNamespace =
CategoricalColorNamespace.getNamespace(colorNamespace);
const scale = categoricalNamespace.getScale(colorScheme);
const colors = scale.range();
const multiple = Math.ceil(this.values.length / colors.length);
const analogousColors = colors.map(color => {
const result = tinycolor(color).analogous(Math.max(multiple, 5));
return result.slice(result.length - multiple, result.length);
});
const generatedColors: tinycolor.Instance[] = [];
// [[A, AA, AAA], [B, BB, BBB]] => [A, B, AA, BB, AAA, BBB]
while (analogousColors[analogousColors.length - 1]?.length) {
analogousColors.forEach(colors =>
generatedColors.push(colors.shift() as tinycolor.Instance),
);
}
return this.values.reduce(
(res, name, index) => ({
...res,
[name.toString()]: generatedColors[index]?.toHexString(),
}),
{},
);
}
return undefined;
}

addSlice(value: string, sliceId?: number) {
if (!sliceId) return;
const sliceIds = this.valueSliceMap[value] ?? [];
if (sliceIds.indexOf(sliceId) === -1) {
sliceIds.push(sliceId);
}
this.valueSliceMap[value] = sliceIds;
if (sliceIds.length > 1 && this.values.indexOf(value) === -1) {
this.values.push(value);
}
}

removeSlice(sliceId: number) {
Object.keys(this.valueSliceMap).forEach(value => {
const sliceIds = this.valueSliceMap[value];
if (sliceIds.indexOf(sliceId) > -1) {
sliceIds.splice(sliceIds.indexOf(sliceId), 1);
if (sliceIds.length <= 1 && this.values.indexOf(value) > -1) {
this.values.splice(this.values.indexOf(value), 1);
}
}
});
}

clear() {
this.values = [];
this.valueSliceMap = {};
}
}

const getInstance = makeSingleton(SharedLabelColor);

export default getInstance;
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,9 @@ export * from './SequentialScheme';
export { default as ColorSchemeRegistry } from './ColorSchemeRegistry';
export * from './colorSchemes';
export * from './utils';
export {
default as getSharedLabelColor,
SharedLabelColor,
} from './SharedLabelColorSingleton';

export const BRAND_COLOR = '#00A699';
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import {
CategoricalScheme,
getCategoricalSchemeRegistry,
getSharedLabelColor,
SharedLabelColor,
} from '@superset-ui/core';

describe('SharedLabelColor', () => {
beforeAll(() => {
getCategoricalSchemeRegistry()
.registerValue(
'testColors',
new CategoricalScheme({
id: 'testColors',
colors: ['red', 'green', 'blue'],
}),
)
.registerValue(
'testColors2',
new CategoricalScheme({
id: 'testColors2',
colors: ['red', 'green', 'blue'],
}),
);
});

beforeEach(() => {
getSharedLabelColor().clear();
});

it('has default value out-of-the-box', () => {
expect(getSharedLabelColor()).toBeInstanceOf(SharedLabelColor);
});

describe('.addSlice(value, sliceId)', () => {
it('should add to valueSliceMap when first adding label', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
expect(sharedLabelColor.valueSliceMap).toHaveProperty('a', [1]);
expect(sharedLabelColor.values).toHaveLength(0);
});

it('should not add to value when adding same sliceId', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
sharedLabelColor.addSlice('a', 1);
expect(sharedLabelColor.valueSliceMap).toHaveProperty('a', [1]);
expect(sharedLabelColor.values).toHaveLength(0);
});

it('should add to value when adding same label', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
sharedLabelColor.addSlice('a', 2);
expect(sharedLabelColor.valueSliceMap).toHaveProperty('a', [1, 2]);
expect(sharedLabelColor.values).toHaveLength(1);
});

it('do nothing when sliceId is undefined', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a');
expect(sharedLabelColor.valueSliceMap).toEqual({});
expect(sharedLabelColor.values).toHaveLength(0);
});
});

describe('.remove(sliceId)', () => {
it('should remove sliceId', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
sharedLabelColor.removeSlice(1);
expect(sharedLabelColor.valueSliceMap).toHaveProperty('a', []);
});

it('should not remove sliceId when do not have sliceId', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
sharedLabelColor.removeSlice(2);
expect(sharedLabelColor.valueSliceMap).toHaveProperty('a', [1]);
});

it('should remove label if less than two sliceIds', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
sharedLabelColor.addSlice('a', 2);
sharedLabelColor.removeSlice(2);
expect(sharedLabelColor.valueSliceMap).toHaveProperty('a', [1]);
expect(sharedLabelColor.values).toHaveLength(0);
});
});

describe('.getColorMap(namespace, scheme)', () => {
it('return undefined when scheme is undefined', () => {
const sharedLabelColor = getSharedLabelColor();
const colorMap = sharedLabelColor.getColorMap();
expect(colorMap).toBeUndefined();
});

it('return colorMap when scheme is defined', () => {
const sharedLabelColor = getSharedLabelColor();
sharedLabelColor.addSlice('a', 1);
sharedLabelColor.addSlice('a', 2);
const colorMap = sharedLabelColor.getColorMap('', 'testColors');
expect(colorMap).not.toBeUndefined();
});
});
});
Loading