Improve categorical color management (#5815)

* Create new classes for handling categorical colors

* verify to pass existing unit tests

* separate logic for forcing color and getting color

* replace getColorFromScheme with CategoricalColorManager

* organize static functions

* migrate to new function

* Remove ALL_COLOR_SCHEMES

* move sequential colors to another file

* extract categorical colors to separate file

* move airbnb and lyft colors to separate files

* fix missing toFunction()

* Rewrite to support local and global force items, plus namespacing.

* fix references

* revert nvd3

* update namespace api

* Update the visualizations

* update usage with static functions

* update unit test

* add unit test

* rename default namespace

* add unit test for color namespace

* add unit test for namespace

* start unit test for colorschememanager

* add unit tests for color scheme manager

* check returns for chaining

* complete unit test for the new classes

* fix color tests

* update unit tests

* update unit tests

* move color scheme registration to common

* update unit test

* rename sharedForcedColors to parentForcedColors

* remove import

superset/assets/package.json

@@ -8,6 +8,7 @@
     "test": "spec"
   },
   "scripts": {
+    "tdd": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*' --watch --recursive",
     "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*'",
     "test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js",
     "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --compilers babel-core/register --require spec/helpers/shim.js --require ignore-styles 'spec/**/*_spec.*'",

superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx

@@ -7,10 +7,10 @@ import { Creatable } from 'react-select';
 
 import ColorSchemeControl from
   '../../../../src/explore/components/controls/ColorSchemeControl';
-import { ALL_COLOR_SCHEMES } from '../../../../src/modules/colors';
+import { getAllSchemes } from '../../../../src/modules/ColorSchemeManager';
 
 const defaultProps = {
-  options: Object.keys(ALL_COLOR_SCHEMES).map(s => ([s, s])),
+  options: Object.keys(getAllSchemes()).map(s => ([s, s])),
 };
 
 describe('ColorSchemeControl', () => {

superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js

@@ -0,0 +1,130 @@
+import { it, describe, before } from 'mocha';
+import { expect } from 'chai';
+import CategoricalColorNamespace, {
+  getNamespace,
+  getScale,
+  getColor,
+  DEFAULT_NAMESPACE,
+} from '../../../src/modules/CategoricalColorNamespace';
+import { registerScheme } from '../../../src/modules/ColorSchemeManager';
+
+describe('CategoricalColorNamespace', () => {
+  before(() => {
+    registerScheme('testColors', ['red', 'green', 'blue']);
+    registerScheme('testColors2', ['red', 'green', 'blue']);
+  });
+  it('The class constructor cannot be accessed directly', () => {
+    expect(CategoricalColorNamespace).to.not.be.a('Function');
+  });
+  describe('static getNamespace()', () => {
+    it('returns default namespace if name is not specified', () => {
+      const namespace = getNamespace();
+      expect(namespace !== undefined).to.equal(true);
+      expect(namespace.name).to.equal(DEFAULT_NAMESPACE);
+    });
+    it('returns namespace with specified name', () => {
+      const namespace = getNamespace('myNamespace');
+      expect(namespace !== undefined).to.equal(true);
+      expect(namespace.name).to.equal('myNamespace');
+    });
+    it('returns existing instance if the name already exists', () => {
+      const ns1 = getNamespace('myNamespace');
+      const ns2 = getNamespace('myNamespace');
+      expect(ns1).to.equal(ns2);
+      const ns3 = getNamespace();
+      const ns4 = getNamespace();
+      expect(ns3).to.equal(ns4);
+    });
+  });
+  describe('.getScale()', () => {
+    it('returns a CategoricalColorScale from given scheme name', () => {
+      const namespace = getNamespace('test-get-scale1');
+      const scale = namespace.getScale('testColors');
+      expect(scale).to.not.equal(undefined);
+      expect(scale.getColor('dog')).to.not.equal(undefined);
+    });
+    it('returns same scale if the scale with that name already exists in this namespace', () => {
+      const namespace = getNamespace('test-get-scale2');
+      const scale1 = namespace.getScale('testColors');
+      const scale2 = namespace.getScale('testColors2');
+      const scale3 = namespace.getScale('testColors2');
+      const scale4 = namespace.getScale('testColors');
+      expect(scale1).to.equal(scale4);
+      expect(scale2).to.equal(scale3);
+    });
+  });
+  describe('.setColor()', () => {
+    it('overwrites color for all CategoricalColorScales in this namespace', () => {
+      const namespace = getNamespace('test-set-scale1');
+      namespace.setColor('dog', 'black');
+      const scale = namespace.getScale('testColors');
+      expect(scale.getColor('dog')).to.equal('black');
+      expect(scale.getColor('boy')).to.not.equal('black');
+    });
+    it('can override forcedColors in each scale', () => {
+      const namespace = getNamespace('test-set-scale2');
+      namespace.setColor('dog', 'black');
+      const scale = namespace.getScale('testColors');
+      scale.setColor('dog', 'pink');
+      expect(scale.getColor('dog')).to.equal('black');
+      expect(scale.getColor('boy')).to.not.equal('black');
+    });
+    it('does not affect scales in other namespaces', () => {
+      const ns1 = getNamespace('test-set-scale3.1');
+      ns1.setColor('dog', 'black');
+      const scale1 = ns1.getScale('testColors');
+      const ns2 = getNamespace('test-set-scale3.2');
+      const scale2 = ns2.getScale('testColors');
+      expect(scale1.getColor('dog')).to.equal('black');
+      expect(scale2.getColor('dog')).to.not.equal('black');
+    });
+    it('returns the namespace instance', () => {
+      const ns1 = getNamespace('test-set-scale3.1');
+      const ns2 = ns1.setColor('dog', 'black');
+      expect(ns1).to.equal(ns2);
+    });
+  });
+  describe('static getScale()', () => {
+    it('getScale() returns a CategoricalColorScale with default scheme in default namespace', () => {
+      const scale = getScale();
+      expect(scale).to.not.equal(undefined);
+      const scale2 = getNamespace().getScale();
+      expect(scale).to.equal(scale2);
+    });
+    it('getScale(scheme) returns a CategoricalColorScale with specified scheme in default namespace', () => {
+      const scale = getScale('testColors');
+      expect(scale).to.not.equal(undefined);
+      const scale2 = getNamespace().getScale('testColors');
+      expect(scale).to.equal(scale2);
+    });
+    it('getScale(scheme, namespace) returns a CategoricalColorScale with specified scheme in specified namespace', () => {
+      const scale = getScale('testColors', 'test-getScale');
+      expect(scale).to.not.equal(undefined);
+      const scale2 = getNamespace('test-getScale').getScale('testColors');
+      expect(scale).to.equal(scale2);
+    });
+  });
+  describe('static getColor()', () => {
+    it('getColor(value) returns a color from default scheme in default namespace', () => {
+      const value = 'dog';
+      const color = getColor(value);
+      const color2 = getNamespace().getScale().getColor(value);
+      expect(color).to.equal(color2);
+    });
+    it('getColor(value, scheme) returns a color from specified scheme in default namespace', () => {
+      const value = 'dog';
+      const scheme = 'testColors';
+      const color = getColor(value, scheme);
+      const color2 = getNamespace().getScale(scheme).getColor(value);
+      expect(color).to.equal(color2);
+    });
+    it('getColor(value, scheme, namespace) returns a color from specified scheme in specified namespace', () => {
+      const value = 'dog';
+      const scheme = 'testColors';
+      const namespace = 'test-getColor';
+      const color = getColor(value, scheme, namespace);
+      const color2 = getNamespace(namespace).getScale(scheme).getColor(value);
+      expect(color).to.equal(color2);
+    });
+  });
+});

superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js

@@ -0,0 +1,96 @@
+import { it, describe } from 'mocha';
+import { expect } from 'chai';
+import CategoricalColorScale from '../../../src/modules/CategoricalColorScale';
+
+describe('CategoricalColorScale', () => {
+  it('exists', () => {
+    expect(CategoricalColorScale !== undefined).to.equal(true);
+  });
+
+  describe('new CategoricalColorScale(colors, parentForcedColors)', () => {
+    it('can create new scale when parentForcedColors is not given', () => {
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      expect(scale).to.be.instanceOf(CategoricalColorScale);
+    });
+    it('can create new scale when parentForcedColors is given', () => {
+      const parentForcedColors = {};
+      const scale = new CategoricalColorScale(['blue', 'red', 'green'], parentForcedColors);
+      expect(scale).to.be.instanceOf(CategoricalColorScale);
+      expect(scale.parentForcedColors).to.equal(parentForcedColors);
+    });
+  });
+  describe('.getColor(value)', () => {
+    it('returns same color for same value', () => {
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      const c1 = scale.getColor('pig');
+      const c2 = scale.getColor('horse');
+      const c3 = scale.getColor('pig');
+      scale.getColor('cow');
+      const c5 = scale.getColor('horse');
+
+      expect(c1).to.equal(c3);
+      expect(c2).to.equal(c5);
+    });
+    it('returns different color for consecutive items', () => {
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      const c1 = scale.getColor('pig');
+      const c2 = scale.getColor('horse');
+      const c3 = scale.getColor('cat');
+
+      expect(c1).to.not.equal(c2);
+      expect(c2).to.not.equal(c3);
+      expect(c3).to.not.equal(c1);
+    });
+    it('recycles colors when number of items exceed available colors', () => {
+      const colorSet = {};
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      const colors = [
+        scale.getColor('pig'),
+        scale.getColor('horse'),
+        scale.getColor('cat'),
+        scale.getColor('cow'),
+        scale.getColor('donkey'),
+        scale.getColor('goat'),
+      ];
+      colors.forEach((color) => {
+        if (colorSet[color]) {
+          colorSet[color]++;
+        } else {
+          colorSet[color] = 1;
+        }
+      });
+      expect(Object.keys(colorSet).length).to.equal(3);
+      ['blue', 'red', 'green'].forEach((color) => {
+        expect(colorSet[color]).to.equal(2);
+      });
+    });
+  });
+  describe('.setColor(value, forcedColor)', () => {
+    it('overrides default color', () => {
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      scale.setColor('pig', 'pink');
+      expect(scale.getColor('pig')).to.equal('pink');
+    });
+    it('does not override parentForcedColors', () => {
+      const scale1 = new CategoricalColorScale(['blue', 'red', 'green']);
+      scale1.setColor('pig', 'black');
+      const scale2 = new CategoricalColorScale(['blue', 'red', 'green'], scale1.forcedColors);
+      scale2.setColor('pig', 'pink');
+      expect(scale1.getColor('pig')).to.equal('black');
+      expect(scale2.getColor('pig')).to.equal('black');
+    });
+    it('returns the scale', () => {
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      const output = scale.setColor('pig', 'pink');
+      expect(scale).to.equal(output);
+    });
+  });
+  describe('.toFunction()', () => {
+    it('returns a function that wraps getColor', () => {
+      const scale = new CategoricalColorScale(['blue', 'red', 'green']);
+      const colorFn = scale.toFunction();
+      expect(scale.getColor('pig')).to.equal(colorFn('pig'));
+      expect(scale.getColor('cat')).to.equal(colorFn('cat'));
+    });
+  });
+});

superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js

@@ -0,0 +1,141 @@
+import { it, describe, beforeEach } from 'mocha';
+import { expect } from 'chai';
+import ColorSchemeManager, {
+  getInstance,
+  getScheme,
+  getAllSchemes,
+  getDefaultSchemeName,
+  setDefaultSchemeName,
+  registerScheme,
+  registerMultipleSchemes,
+} from '../../../src/modules/ColorSchemeManager';
+
+describe('ColorSchemeManager', () => {
+  beforeEach(() => {
+    const m = getInstance();
+    m.clearScheme();
+    m.registerScheme('test', ['red', 'green', 'blue']);
+    m.registerScheme('test2', ['orange', 'yellow', 'pink']);
+    m.setDefaultSchemeName('test');
+  });
+  it('The class constructor cannot be accessed directly', () => {
+    expect(ColorSchemeManager).to.not.be.a('Function');
+  });
+  describe('static getInstance()', () => {
+    it('returns a singleton instance', () => {
+      const m1 = getInstance();
+      const m2 = getInstance();
+      expect(m1).to.not.equal(undefined);
+      expect(m1).to.equal(m2);
+    });
+  });
+  describe('.getScheme()', () => {
+    it('.getScheme() returns default color scheme', () => {
+      const scheme = getInstance().getScheme();
+      expect(scheme).to.deep.equal(['red', 'green', 'blue']);
+    });
+    it('.getScheme(name) returns color scheme with specified name', () => {
+      const scheme = getInstance().getScheme('test2');
+      expect(scheme).to.deep.equal(['orange', 'yellow', 'pink']);
+    });
+  });
+  describe('.getAllSchemes()', () => {
+    it('returns all registered schemes', () => {
+      const schemes = getInstance().getAllSchemes();
+      expect(schemes).to.deep.equal({
+        test: ['red', 'green', 'blue'],
+        test2: ['orange', 'yellow', 'pink'],
+      });
+    });
+  });
+  describe('.getDefaultSchemeName()', () => {
+    it('returns default scheme name', () => {
+      const name = getInstance().getDefaultSchemeName();
+      expect(name).to.equal('test');
+    });
+  });
+  describe('.setDefaultSchemeName()', () => {
+    it('set default scheme name', () => {
+      getInstance().setDefaultSchemeName('test2');
+      const name = getInstance().getDefaultSchemeName();
+      expect(name).to.equal('test2');
+      getInstance().setDefaultSchemeName('test');
+    });
+    it('returns the ColorSchemeManager instance', () => {
+      const instance = getInstance().setDefaultSchemeName('test');
+      expect(instance).to.equal(getInstance());
+    });
+  });
+  describe('.registerScheme(name, colors)', () => {
+    it('sets schemename and color', () => {
+      getInstance().registerScheme('test3', ['cyan', 'magenta']);
+      const scheme = getInstance().getScheme('test3');
+      expect(scheme).to.deep.equal(['cyan', 'magenta']);
+    });
+    it('returns the ColorSchemeManager instance', () => {
+      const instance = getInstance().registerScheme('test3', ['cyan', 'magenta']);
+      expect(instance).to.equal(getInstance());
+    });
+  });
+  describe('.registerMultipleSchemes(object)', () => {
+    it('sets multiple schemes at once', () => {
+      getInstance().registerMultipleSchemes({
+        test4: ['cyan', 'magenta'],
+        test5: ['brown', 'purple'],
+      });
+      const scheme1 = getInstance().getScheme('test4');
+      expect(scheme1).to.deep.equal(['cyan', 'magenta']);
+      const scheme2 = getInstance().getScheme('test5');
+      expect(scheme2).to.deep.equal(['brown', 'purple']);
+    });
+    it('returns the ColorSchemeManager instance', () => {
+      const instance = getInstance().registerMultipleSchemes({
+        test4: ['cyan', 'magenta'],
+        test5: ['brown', 'purple'],
+      });
+      expect(instance).to.equal(getInstance());
+    });
+  });
+  describe('static getScheme()', () => {
+    it('is equivalent to getInstance().getScheme()', () => {
+      expect(getInstance().getScheme()).to.equal(getScheme());
+    });
+  });
+  describe('static getAllSchemes()', () => {
+    it('is equivalent to getInstance().getAllSchemes()', () => {
+      expect(getInstance().getAllSchemes()).to.equal(getAllSchemes());
+    });
+  });
+  describe('static getDefaultSchemeName()', () => {
+    it('is equivalent to getInstance().getDefaultSchemeName()', () => {
+      expect(getInstance().getDefaultSchemeName()).to.equal(getDefaultSchemeName());
+    });
+  });
+  describe('static setDefaultSchemeName()', () => {
+    it('is equivalent to getInstance().setDefaultSchemeName()', () => {
+      setDefaultSchemeName('test2');
+      const name = getInstance().getDefaultSchemeName();
+      expect(name).to.equal('test2');
+      setDefaultSchemeName('test');
+    });
+  });
+  describe('static registerScheme()', () => {
+    it('is equivalent to getInstance().registerScheme()', () => {
+      registerScheme('test3', ['cyan', 'magenta']);
+      const scheme = getInstance().getScheme('test3');
+      expect(scheme).to.deep.equal(['cyan', 'magenta']);
+    });
+  });
+  describe('static registerMultipleSchemes()', () => {
+    it('is equivalent to getInstance().registerMultipleSchemes()', () => {
+      registerMultipleSchemes({
+        test4: ['cyan', 'magenta'],
+        test5: ['brown', 'purple'],
+      });
+      const scheme1 = getInstance().getScheme('test4');
+      expect(scheme1).to.deep.equal(['cyan', 'magenta']);
+      const scheme2 = getInstance().getScheme('test5');
+      expect(scheme2).to.deep.equal(['brown', 'purple']);
+    });
+  });
+});