Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

Make CategoricalColorScale instance a function and remove .toFunction() #33

Merged
merged 5 commits into from
Nov 17, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
5 changes: 4 additions & 1 deletion packages/generator-superset/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,8 @@
"yeoman-generator": "^3.1.1",
"yosay": "^2.0.2"
},
"license": "Apache-2.0"
"license": "Apache-2.0",
"publishConfig": {
"access": "public"
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import CategoricalColorScale from './CategoricalColorScale';
import getCategoricalSchemeRegistry from './CategoricalSchemeRegistrySingleton';
import cleanValue from './cleanValue';

export default class CategoricalColorNamespace {
constructor(name) {
Expand Down Expand Up @@ -31,7 +32,7 @@ export default class CategoricalColorNamespace {
* @param {*} forcedColor color
*/
setColor(value, forcedColor) {
this.forcedItems[value] = forcedColor;
this.forcedItems[cleanValue(value)] = forcedColor;

return this;
}
Expand Down
18 changes: 5 additions & 13 deletions packages/superset-ui-color/src/CategoricalColorScale.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
export function cleanValue(value) {
// for superset series that should have the same color
return String(value)
.trim()
.toLowerCase();
}
import { ExtensibleFunction } from '@superset-ui/core';
import cleanValue from './cleanValue';

export default class CategoricalColorScale {
export default class CategoricalColorScale extends ExtensibleFunction {
/**
* Constructor
* @param {*} colors an array of colors
* @param {*} parentForcedColors optional parameter that comes from parent
* (usually CategoricalColorNamespace) and supersede this.forcedColors
*/
constructor(colors, parentForcedColors) {
super((...args) => this.getColor(...args));
this.colors = colors;
this.parentForcedColors = parentForcedColors;
this.forcedColors = {};
this.seen = {};
this.fn = value => this.getColor(value);
}

getColor(value) {
Expand Down Expand Up @@ -51,12 +47,8 @@ export default class CategoricalColorScale {
* @param {*} forcedColor forcedColor
*/
setColor(value, forcedColor) {
this.forcedColors[value] = forcedColor;
this.forcedColors[cleanValue(value)] = forcedColor;

return this;
}

toFunction() {
return this.fn;
}
}
7 changes: 7 additions & 0 deletions packages/superset-ui-color/src/cleanValue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* Ensure value is a string
* @param {any} value
*/
export default function cleanValue(value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we possibly make this have a more specific name? clean is pretty generic, something like stringyifyAndTrim would make it more readable/clear what exactly "clean" means.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return String(value).trim();
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,11 @@ describe('CategoricalColorScale', () => {
expect(scale).toBe(output);
});
});
describe('.toFunction()', () => {
it('returns a function that wraps getColor', () => {
describe('a CategoricalColorScale instance is also a color function itself', () => {
it('scale(value) returns color similar to calling scale.getColor(value)', () => {
const scale = new CategoricalColorScale(['blue', 'red', 'green']);
const colorFn = scale.toFunction();
expect(scale.getColor('pig')).toBe(colorFn('pig'));
expect(scale.getColor('cat')).toBe(colorFn('cat'));
expect(scale.getColor('pig')).toBe(scale('pig'));
expect(scale.getColor('cat')).toBe(scale('cat'));
});
});
});