From 3c1210c3b8b5c361096c331d8f95ef72a8890fee Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 31 Aug 2018 18:08:48 -0700 Subject: [PATCH 01/33] Create new classes for handling categorical colors --- .../src/modules/CategoricalColorManager.js | 55 +++++++++++++++ .../src/modules/CategoricalColorScale.js | 41 ++++++++++++ superset/assets/src/modules/colors.js | 67 ++++++++++--------- 3 files changed, 133 insertions(+), 30 deletions(-) create mode 100644 superset/assets/src/modules/CategoricalColorManager.js create mode 100644 superset/assets/src/modules/CategoricalColorScale.js diff --git a/superset/assets/src/modules/CategoricalColorManager.js b/superset/assets/src/modules/CategoricalColorManager.js new file mode 100644 index 0000000000000..4545a659c078c --- /dev/null +++ b/superset/assets/src/modules/CategoricalColorManager.js @@ -0,0 +1,55 @@ +import CategoricalColorScale from './CategoricalColorScale'; + +const schemes = {}; + +class CategoricalColorManager { + constructor() { + this.scales = {}; + } + + getScale(schemeName) { + const scale = this.scales[schemeName]; + if (scale) { + return scale; + } + const colors = schemes[schemeName] || schemes.default; + const newScale = new CategoricalColorScale(colors); + this.scales[schemeName] = newScale; + return newScale; + } + + getColorFromScheme(schemeName, value, forcedColor) { + return this.getScale(schemeName).getColor(value, forcedColor); + } +} + +let singleton; + +function getInstance() { + if (!singleton) { + singleton = new CategoricalColorManager(); + } + return singleton; +} + +CategoricalColorManager.getInstance = getInstance; +CategoricalColorManager.getScale = function (schemeName) { + return getInstance().getScale(schemeName); +}; +CategoricalColorManager.getColorFromScheme = function (schemeName, value, forcedColor) { + return getInstance().getColorFromScheme(schemeName, value, forcedColor); +}; +CategoricalColorManager.registerScheme = function (schemeName, colors) { + schemes[schemeName] = colors; +}; +CategoricalColorManager.registerSchemes = function (multipleSchemes) { + Object.assign(schemes, multipleSchemes); +}; +CategoricalColorManager.getScheme = function (schemeName) { + return schemes[schemeName]; +}; +CategoricalColorManager.getSchemes = function () { + return schemes; +}; + +export default CategoricalColorManager; diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js new file mode 100644 index 0000000000000..b7b08acda0b84 --- /dev/null +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -0,0 +1,41 @@ +import { TIME_SHIFT_PATTERN } from '../utils/common'; + +function cleanValue(value) { + return String(value).trim() + .toLowerCase() + // for superset series that should have the same color + .split(', ') + .filter(k => !TIME_SHIFT_PATTERN.test(k)) + .join(', '); +} + +export default class CategoricalColorScale { + constructor(colors) { + this.colors = colors; + this.forced = {}; + this.seen = {}; + } + + getColor(value, forcedColor) { + const cleanedValue = cleanValue(value); + + const specialColor = this.forced[cleanedValue]; + if (specialColor) { + return specialColor; + } + + if (forcedColor) { + this.forced[cleanedValue] = forcedColor; + return forcedColor; + } + + const seenColor = this.seen[cleanedValue]; + if (seenColor !== undefined) { + return this.colors[seenColor % this.colors.length]; + } + + const index = Object.keys(this.seen).length; + this.seen[cleanedValue] = index; + return this.colors[index % this.colors.length]; + } +} diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 1cb9eed4e85ec..71dccb4bcd5cf 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -1,5 +1,5 @@ import d3 from 'd3'; -import { TIME_SHIFT_PATTERN } from '../utils/common'; +import CategoricalColorManager from './CategoricalColorManager'; export const brandColor = '#00A699'; export const colorPrimary = { r: 0, g: 122, b: 135, a: 1 }; @@ -80,7 +80,8 @@ const googleCategory20c = [ '#5574a6', '#3b3eac', ]; -export const ALL_COLOR_SCHEMES = { + +CategoricalColorManager.registerSchemes({ bnbColors, d3Category10, d3Category20, @@ -89,7 +90,9 @@ export const ALL_COLOR_SCHEMES = { googleCategory10c, googleCategory20c, lyftColors, -}; +}); + +export const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); export const spectrums = { blue_white_yellow: [ @@ -546,35 +549,39 @@ export function hexToRGB(hex, alpha = 255) { * @param {string} forcedColor - A color that the caller wants to forcibly associate to a label. */ -export const getColorFromScheme = (function () { - const seen = {}; - const forcedColors = {}; - return function (s, scheme, forcedColor) { - if (!s) { - return; - } - const selectedScheme = scheme ? ALL_COLOR_SCHEMES[scheme] : ALL_COLOR_SCHEMES.bnbColors; - let stringifyS = String(s).toLowerCase(); - // next line is for superset series that should have the same color - stringifyS = stringifyS.split(', ').filter(k => !TIME_SHIFT_PATTERN.test(k)).join(', '); +export function getColorFromScheme(value, schemeName, forcedColor) { + return CategoricalColorManager.getColorFromScheme(schemeName, value, forcedColor); +} + +// export const getColorFromScheme = (function () { +// const seen = {}; +// const forcedColors = {}; +// return function (s, scheme, forcedColor) { +// if (!s) { +// return; +// } +// const selectedScheme = scheme ? ALL_COLOR_SCHEMES[scheme] : ALL_COLOR_SCHEMES.bnbColors; +// let stringifyS = String(s).toLowerCase(); +// // next line is for superset series that should have the same color +// stringifyS = stringifyS.split(', ').filter(k => !TIME_SHIFT_PATTERN.test(k)).join(', '); - if (forcedColor && !forcedColors[stringifyS]) { - forcedColors[stringifyS] = forcedColor; - } - if (forcedColors[stringifyS]) { - return forcedColors[stringifyS]; - } +// if (forcedColor && !forcedColors[stringifyS]) { +// forcedColors[stringifyS] = forcedColor; +// } +// if (forcedColors[stringifyS]) { +// return forcedColors[stringifyS]; +// } - if (seen[selectedScheme] === undefined) { - seen[selectedScheme] = {}; - } - if (seen[selectedScheme][stringifyS] === undefined) { - seen[selectedScheme][stringifyS] = Object.keys(seen[selectedScheme]).length; - } - /* eslint consistent-return: 0 */ - return selectedScheme[seen[selectedScheme][stringifyS] % selectedScheme.length]; - }; -}()); +// if (seen[selectedScheme] === undefined) { +// seen[selectedScheme] = {}; +// } +// if (seen[selectedScheme][stringifyS] === undefined) { +// seen[selectedScheme][stringifyS] = Object.keys(seen[selectedScheme]).length; +// } +// /* eslint consistent-return: 0 */ +// return selectedScheme[seen[selectedScheme][stringifyS] % selectedScheme.length]; +// }; +// }()); export const colorScalerFactory = function (colors, data, accessor, extents, outputRGBA = false) { // Returns a linear scaler our of an array of color From 385a21afefd8aee15b25b8bf28f412697800d034 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 31 Aug 2018 18:29:21 -0700 Subject: [PATCH 02/33] verify to pass existing unit tests --- superset/assets/src/modules/CategoricalColorScale.js | 6 ++++-- superset/assets/src/modules/colors.js | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index b7b08acda0b84..2ce4e5dbe270b 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -9,10 +9,12 @@ function cleanValue(value) { .join(', '); } +const sharedForced = {}; + export default class CategoricalColorScale { - constructor(colors) { + constructor(colors, forced = sharedForced) { this.colors = colors; - this.forced = {}; + this.forced = forced; this.seen = {}; } diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 71dccb4bcd5cf..e2452a1a1ae89 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -91,6 +91,7 @@ CategoricalColorManager.registerSchemes({ googleCategory20c, lyftColors, }); +CategoricalColorManager.registerScheme('default', bnbColors); export const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); From 1502fcb24f6ecbda575f2e5015d7f3c68ae120f3 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 17:01:50 -0700 Subject: [PATCH 03/33] separate logic for forcing color and getting color --- .../src/modules/CategoricalColorManager.js | 7 +++- .../src/modules/CategoricalColorScale.js | 25 ++++++++----- superset/assets/src/modules/colors.js | 37 +++---------------- .../src/visualizations/wordcloud/WordCloud.js | 6 ++- 4 files changed, 30 insertions(+), 45 deletions(-) diff --git a/superset/assets/src/modules/CategoricalColorManager.js b/superset/assets/src/modules/CategoricalColorManager.js index 4545a659c078c..f68a08ec4f649 100644 --- a/superset/assets/src/modules/CategoricalColorManager.js +++ b/superset/assets/src/modules/CategoricalColorManager.js @@ -36,8 +36,11 @@ CategoricalColorManager.getInstance = getInstance; CategoricalColorManager.getScale = function (schemeName) { return getInstance().getScale(schemeName); }; -CategoricalColorManager.getColorFromScheme = function (schemeName, value, forcedColor) { - return getInstance().getColorFromScheme(schemeName, value, forcedColor); +CategoricalColorManager.getColor = function (schemeName, value) { + return getInstance().getScale(schemeName).getColor(value); +}; +CategoricalColorManager.setColor = function (schemeName, value, forcedColor) { + return getInstance().getScale(schemeName).setColor(value, forcedColor); }; CategoricalColorManager.registerScheme = function (schemeName, colors) { schemes[schemeName] = colors; diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index 2ce4e5dbe270b..f5af5dfab7695 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -9,25 +9,21 @@ function cleanValue(value) { .join(', '); } -const sharedForced = {}; +const sharedForcedItems = {}; export default class CategoricalColorScale { - constructor(colors, forced = sharedForced) { + constructor(colors, forcedItems = sharedForcedItems) { this.colors = colors; - this.forced = forced; + this.forcedItems = forcedItems; this.seen = {}; + this.fn = value => this.getColor(value); } - getColor(value, forcedColor) { + getColor(value) { const cleanedValue = cleanValue(value); - const specialColor = this.forced[cleanedValue]; - if (specialColor) { - return specialColor; - } - + const forcedColor = this.forcedItems[cleanedValue]; if (forcedColor) { - this.forced[cleanedValue] = forcedColor; return forcedColor; } @@ -40,4 +36,13 @@ export default class CategoricalColorScale { this.seen[cleanedValue] = index; return this.colors[index % this.colors.length]; } + + setColor(value, forcedColor) { + this.forcedItems[value] = forcedColor; + return this; + } + + toFunction() { + return this.fn; + } } diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index e2452a1a1ae89..2980909284c9d 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -551,39 +551,14 @@ export function hexToRGB(hex, alpha = 255) { forcibly associate to a label. */ export function getColorFromScheme(value, schemeName, forcedColor) { - return CategoricalColorManager.getColorFromScheme(schemeName, value, forcedColor); + const scale = CategoricalColorManager.getScale(schemeName); + if (forcedColor) { + scale.setColor(value, forcedColor); + return forcedColor; + } + return scale.getColor(value); } -// export const getColorFromScheme = (function () { -// const seen = {}; -// const forcedColors = {}; -// return function (s, scheme, forcedColor) { -// if (!s) { -// return; -// } -// const selectedScheme = scheme ? ALL_COLOR_SCHEMES[scheme] : ALL_COLOR_SCHEMES.bnbColors; -// let stringifyS = String(s).toLowerCase(); -// // next line is for superset series that should have the same color -// stringifyS = stringifyS.split(', ').filter(k => !TIME_SHIFT_PATTERN.test(k)).join(', '); - -// if (forcedColor && !forcedColors[stringifyS]) { -// forcedColors[stringifyS] = forcedColor; -// } -// if (forcedColors[stringifyS]) { -// return forcedColors[stringifyS]; -// } - -// if (seen[selectedScheme] === undefined) { -// seen[selectedScheme] = {}; -// } -// if (seen[selectedScheme][stringifyS] === undefined) { -// seen[selectedScheme][stringifyS] = Object.keys(seen[selectedScheme]).length; -// } -// /* eslint consistent-return: 0 */ -// return selectedScheme[seen[selectedScheme][stringifyS] % selectedScheme.length]; -// }; -// }()); - export const colorScalerFactory = function (colors, data, accessor, extents, outputRGBA = false) { // Returns a linear scaler our of an array of color if (!Array.isArray(colors)) { diff --git a/superset/assets/src/visualizations/wordcloud/WordCloud.js b/superset/assets/src/visualizations/wordcloud/WordCloud.js index d4d2d7e3b4cca..019bad6b68ec7 100644 --- a/superset/assets/src/visualizations/wordcloud/WordCloud.js +++ b/superset/assets/src/visualizations/wordcloud/WordCloud.js @@ -1,7 +1,7 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import cloudLayout from 'd3-cloud'; -import { getColorFromScheme } from '../../modules/colors'; +import CategoricalColorManager from '../../modules/CategoricalColorManager'; const ROTATION = { square: () => Math.floor((Math.random() * 2)) * 90, @@ -50,6 +50,8 @@ function wordCloud(element, props) { .fontWeight('bold') .fontSize(d => scale(d.size)); + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + function draw(words) { chart.selectAll('*').remove(); @@ -67,7 +69,7 @@ function wordCloud(element, props) { .style('font-size', d => `${d.size}px`) .style('font-weight', 'bold') .style('font-family', 'Helvetica') - .style('fill', d => getColorFromScheme(d.text, colorScheme)) + .style('fill', d => colorFn(d.text)) .attr('text-anchor', 'middle') .attr('transform', d => `translate(${d.x}, ${d.y}) rotate(${d.rotate})`) .text(d => d.text); From 1fc211dc12d2d9161346287ec02b9736410a0ffc Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 17:22:58 -0700 Subject: [PATCH 04/33] replace getColorFromScheme with CategoricalColorManager --- .../assets/src/dashboard/reducers/getInitialState.js | 4 +++- .../assets/src/modules/CategoricalColorManager.js | 4 ---- superset/assets/src/visualizations/chord.jsx | 7 ++++--- superset/assets/src/visualizations/partition.js | 5 +++-- superset/assets/src/visualizations/rose.js | 12 ++++++------ superset/assets/src/visualizations/sankey.js | 6 ++++-- superset/assets/src/visualizations/sunburst.js | 10 ++++++---- superset/assets/src/visualizations/treemap.js | 5 +++-- 8 files changed, 29 insertions(+), 24 deletions(-) diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js index e913adbf89d8a..a87180aeb2d6d 100644 --- a/superset/assets/src/dashboard/reducers/getInitialState.js +++ b/superset/assets/src/dashboard/reducers/getInitialState.js @@ -19,6 +19,7 @@ import { CHART_TYPE, ROW_TYPE, } from '../util/componentTypes'; +import CategoricalColorManager from '../../modules/CategoricalColorManager'; export default function(bootstrapData) { const { user_id, datasources, common, editMode } = bootstrapData; @@ -41,7 +42,8 @@ export default function(bootstrapData) { if (dashboard.metadata && dashboard.metadata.label_colors) { const colorMap = dashboard.metadata.label_colors; Object.keys(colorMap).forEach(label => { - getColorFromScheme(label, null, colorMap[label]); + CategoricalColorManager.getScale(null) + .setColor(label, colorMap[label]); }); } diff --git a/superset/assets/src/modules/CategoricalColorManager.js b/superset/assets/src/modules/CategoricalColorManager.js index f68a08ec4f649..e2f3dc53eef68 100644 --- a/superset/assets/src/modules/CategoricalColorManager.js +++ b/superset/assets/src/modules/CategoricalColorManager.js @@ -17,10 +17,6 @@ class CategoricalColorManager { this.scales[schemeName] = newScale; return newScale; } - - getColorFromScheme(schemeName, value, forcedColor) { - return this.getScale(schemeName).getColor(value, forcedColor); - } } let singleton; diff --git a/superset/assets/src/visualizations/chord.jsx b/superset/assets/src/visualizations/chord.jsx index 2a7cdf1a87949..8f307c97d678e 100644 --- a/superset/assets/src/visualizations/chord.jsx +++ b/superset/assets/src/visualizations/chord.jsx @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign */ import d3 from 'd3'; import PropTypes from 'prop-types'; -import { getColorFromScheme } from '../modules/colors'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; import './chord.css'; const propTypes = { @@ -31,6 +31,7 @@ function chordVis(element, props) { const div = d3.select(element); const { nodes, matrix } = data; const f = d3.format(numberFormat); + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); const outerRadius = Math.min(width, height) / 2 - 10; const innerRadius = outerRadius - 24; @@ -78,7 +79,7 @@ function chordVis(element, props) { const groupPath = group.append('path') .attr('id', (d, i) => 'group' + i) .attr('d', arc) - .style('fill', (d, i) => getColorFromScheme(nodes[i], colorScheme)); + .style('fill', (d, i) => colorFn(nodes[i])); // Add a text label. const groupText = group.append('text') @@ -102,7 +103,7 @@ function chordVis(element, props) { .on('mouseover', (d) => { chord.classed('fade', p => p !== d); }) - .style('fill', d => getColorFromScheme(nodes[d.source.index], colorScheme)) + .style('fill', d => colorFn(nodes[d.source.index])) .attr('d', path); // Add an elaborate mouseover title for each chord. diff --git a/superset/assets/src/visualizations/partition.js b/superset/assets/src/visualizations/partition.js index fa7bbad869aeb..1954e9bda0ca3 100644 --- a/superset/assets/src/visualizations/partition.js +++ b/superset/assets/src/visualizations/partition.js @@ -2,8 +2,8 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import { hierarchy } from 'd3-hierarchy'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; import { d3TimeFormatPreset } from '../modules/utils'; -import { getColorFromScheme } from '../modules/colors'; import './partition.css'; // Compute dx, dy, x, y for each node and @@ -97,6 +97,7 @@ function Icicle(element, props) { const hasTime = ['adv_anal', 'time_series'].indexOf(chartType) >= 0; const format = d3.format(numberFormat); const timeFormat = d3TimeFormatPreset(dateTimeFormat); + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); div.selectAll('*').remove(); const tooltip = div @@ -363,7 +364,7 @@ function Icicle(element, props) { // Apply color scheme g.selectAll('rect') .style('fill', (d) => { - d.color = getColorFromScheme(d.name, colorScheme); + d.color = colorFn(d.name); return d.color; }); } diff --git a/superset/assets/src/visualizations/rose.js b/superset/assets/src/visualizations/rose.js index 875e748b5ef34..ca6ec43953812 100644 --- a/superset/assets/src/visualizations/rose.js +++ b/superset/assets/src/visualizations/rose.js @@ -3,7 +3,7 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import nv from 'nvd3'; import { d3TimeFormatPreset } from '../modules/utils'; -import { getColorFromScheme } from '../modules/colors'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; import './rose.css'; const propTypes = { @@ -62,6 +62,7 @@ function Rose(element, props) { const numGroups = datum[times[0]].length; const format = d3.format(numberFormat); const timeFormat = d3TimeFormatPreset(dateTimeFormat); + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); d3.select('.nvtooltip').remove(); div.selectAll('*').remove(); @@ -70,7 +71,6 @@ function Rose(element, props) { const legend = nv.models.legend(); const tooltip = nv.models.tooltip(); const state = { disabled: datum[times[0]].map(() => false) }; - const color = name => getColorFromScheme(name, colorScheme); const svg = div .append('svg') @@ -101,9 +101,9 @@ function Rose(element, props) { .map(v => ({ key: v.name, value: v.value, - color: color(v.name), + color: colorFn(v.name), highlight: v.id === d.arcId, - })) : [{ key: d.name, value: d.val, color: color(d.name) }]; + })) : [{ key: d.name, value: d.val, color: colorFn(d.name) }]; return { key: 'Date', value: d.time, @@ -113,7 +113,7 @@ function Rose(element, props) { legend .width(width) - .color(d => getColorFromScheme(d.key, colorScheme)); + .color(d => colorFn(d.key)); legendWrap .datum(legendData(datum)) .call(legend); @@ -331,7 +331,7 @@ function Rose(element, props) { const arcs = ae .append('path') .attr('class', 'arc') - .attr('fill', d => color(d.name)) + .attr('fill', d => colorFn(d.name)) .attr('d', arc); function mousemove() { diff --git a/superset/assets/src/visualizations/sankey.js b/superset/assets/src/visualizations/sankey.js index 29ed2e2ffa18e..fde7b2b9978bd 100644 --- a/superset/assets/src/visualizations/sankey.js +++ b/superset/assets/src/visualizations/sankey.js @@ -2,7 +2,7 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import { sankey as d3Sankey } from 'd3-sankey'; -import { getColorFromScheme } from '../modules/colors'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; import './sankey.css'; const propTypes = { @@ -49,6 +49,8 @@ function Sankey(element, props) { .attr('class', 'sankey-tooltip') .style('opacity', 0); + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const sankey = d3Sankey() .nodeWidth(15) .nodePadding(10) @@ -153,7 +155,7 @@ function Sankey(element, props) { .attr('width', sankey.nodeWidth()) .style('fill', function (d) { const name = d.name || 'N/A'; - d.color = getColorFromScheme(name.replace(/ .*/, ''), colorScheme); + d.color = colorFn(name.replace(/ .*/, '')); return d.color; }) .style('stroke', d => d3.rgb(d.color).darker(2)) diff --git a/superset/assets/src/visualizations/sunburst.js b/superset/assets/src/visualizations/sunburst.js index 28fe605ebab11..43a4e8d4b5d00 100644 --- a/superset/assets/src/visualizations/sunburst.js +++ b/superset/assets/src/visualizations/sunburst.js @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign */ import d3 from 'd3'; import PropTypes from 'prop-types'; -import { getColorFromScheme } from '../modules/colors'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; import { wrapSvgText } from '../modules/utils'; import './sunburst.css'; @@ -68,6 +68,8 @@ function Sunburst(element, props) { let arcs; let gMiddleText; // dom handles + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + // Helper + path gen functions const partition = d3.layout.partition() .size([2 * Math.PI, radius * radius]) @@ -132,7 +134,7 @@ function Sunburst(element, props) { .attr('points', breadcrumbPoints) .style('fill', function (d) { return colorByCategory ? - getColorFromScheme(d.name, colorScheme) : + colorFn(d.name) : colorScale(d.m2 / d.m1); }); @@ -143,7 +145,7 @@ function Sunburst(element, props) { .style('fill', function (d) { // Make text white or black based on the lightness of the background const col = d3.hsl(colorByCategory ? - getColorFromScheme(d.name, colorScheme) : + colorFn(d.name) : colorScale(d.m2 / d.m1)); return col.l < 0.5 ? 'white' : 'black'; }) @@ -377,7 +379,7 @@ function Sunburst(element, props) { .attr('d', arc) .attr('fill-rule', 'evenodd') .style('fill', d => colorByCategory - ? getColorFromScheme(d.name, colorScheme) + ? colorFn(d.name) : colorScale(d.m2 / d.m1)) .style('opacity', 1) .on('mouseenter', mouseenter); diff --git a/superset/assets/src/visualizations/treemap.js b/superset/assets/src/visualizations/treemap.js index 7834cd7ca9db9..d69050f0bfac6 100644 --- a/superset/assets/src/visualizations/treemap.js +++ b/superset/assets/src/visualizations/treemap.js @@ -1,7 +1,7 @@ /* eslint-disable no-shadow, no-param-reassign */ import d3 from 'd3'; import PropTypes from 'prop-types'; -import { getColorFromScheme } from '../modules/colors'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; import './treemap.css'; // Declare PropTypes for recursive data structures @@ -63,6 +63,7 @@ function treemap(element, props) { } = props; const div = d3.select(element); const formatNumber = d3.format(numberFormat); + const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); function draw(data, eltWidth, eltHeight) { const navBarHeight = 36; @@ -282,7 +283,7 @@ function treemap(element, props) { .text(d => formatNumber(d.value)); t.call(text); g.selectAll('rect') - .style('fill', d => getColorFromScheme(d.name, colorScheme)); + .style('fill', d => colorFn(d.name)); return g; }; From e3ddbd5e0d26c6c376740a9564ae0fd25aaef7dc Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 17:32:47 -0700 Subject: [PATCH 05/33] organize static functions --- .../src/dashboard/reducers/getInitialState.js | 4 +- .../src/modules/CategoricalColorManager.js | 52 +++++++++++-------- .../deckgl/CategoricalDeckGLContainer.jsx | 9 ++-- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js index a87180aeb2d6d..6481093548b91 100644 --- a/superset/assets/src/dashboard/reducers/getInitialState.js +++ b/superset/assets/src/dashboard/reducers/getInitialState.js @@ -5,7 +5,6 @@ import { chart } from '../../chart/chartReducer'; import { initSliceEntities } from './sliceEntities'; import { getParam } from '../../modules/utils'; import { applyDefaultFormData } from '../../explore/store'; -import { getColorFromScheme } from '../../modules/colors'; import findFirstParentContainerId from '../util/findFirstParentContainer'; import getEmptyLayout from '../util/getEmptyLayout'; import newComponentFactory from '../util/newComponentFactory'; @@ -42,8 +41,7 @@ export default function(bootstrapData) { if (dashboard.metadata && dashboard.metadata.label_colors) { const colorMap = dashboard.metadata.label_colors; Object.keys(colorMap).forEach(label => { - CategoricalColorManager.getScale(null) - .setColor(label, colorMap[label]); + CategoricalColorManager.getScale(null).setColor(label, colorMap[label]); }); } diff --git a/superset/assets/src/modules/CategoricalColorManager.js b/superset/assets/src/modules/CategoricalColorManager.js index e2f3dc53eef68..886a8b3b222f2 100644 --- a/superset/assets/src/modules/CategoricalColorManager.js +++ b/superset/assets/src/modules/CategoricalColorManager.js @@ -2,17 +2,33 @@ import CategoricalColorScale from './CategoricalColorScale'; const schemes = {}; +function getScheme(schemeName = 'default') { + return schemes[schemeName]; +} + +function getSchemes() { + return schemes; +} + +function registerScheme(schemeName, colors) { + schemes[schemeName] = colors; +} + +function registerSchemes(multipleSchemes) { + Object.assign(schemes, multipleSchemes); +} + class CategoricalColorManager { constructor() { this.scales = {}; } - getScale(schemeName) { + getScale(schemeName = 'default') { const scale = this.scales[schemeName]; if (scale) { return scale; } - const colors = schemes[schemeName] || schemes.default; + const colors = getScheme(schemeName); const newScale = new CategoricalColorScale(colors); this.scales[schemeName] = newScale; return newScale; @@ -28,27 +44,17 @@ function getInstance() { return singleton; } -CategoricalColorManager.getInstance = getInstance; -CategoricalColorManager.getScale = function (schemeName) { +function getScale(schemeName) { return getInstance().getScale(schemeName); -}; -CategoricalColorManager.getColor = function (schemeName, value) { - return getInstance().getScale(schemeName).getColor(value); -}; -CategoricalColorManager.setColor = function (schemeName, value, forcedColor) { - return getInstance().getScale(schemeName).setColor(value, forcedColor); -}; -CategoricalColorManager.registerScheme = function (schemeName, colors) { - schemes[schemeName] = colors; -}; -CategoricalColorManager.registerSchemes = function (multipleSchemes) { - Object.assign(schemes, multipleSchemes); -}; -CategoricalColorManager.getScheme = function (schemeName) { - return schemes[schemeName]; -}; -CategoricalColorManager.getSchemes = function () { - return schemes; -}; +} + +Object.assign(CategoricalColorManager, { + getInstance, + getScale, + registerScheme, + registerSchemes, + getScheme, + getSchemes, +}); export default CategoricalColorManager; diff --git a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx index ff5ab09381262..cada6c5ecf2c2 100644 --- a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx +++ b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx @@ -6,19 +6,21 @@ import PropTypes from 'prop-types'; import AnimatableDeckGLContainer from './AnimatableDeckGLContainer'; import Legend from '../Legend'; -import { getColorFromScheme, hexToRGB } from '../../modules/colors'; +import { hexToRGB } from '../../modules/colors'; import { getPlaySliderParams } from '../../modules/time'; import sandboxedEval from '../../modules/sandbox'; +import CategoricalColorManager from '../../modules/CategoricalColorManager'; function getCategories(fd, data) { const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 }; const fixedColor = [c.r, c.g, c.b, 255 * c.a]; + const colorFn = CategoricalColorManager.getScale(fd.color_scheme); const categories = {}; data.forEach((d) => { if (d.cat_color != null && !categories.hasOwnProperty(d.cat_color)) { let color; if (fd.dimension) { - color = hexToRGB(getColorFromScheme(d.cat_color, fd.color_scheme), c.a * 255); + color = hexToRGB(colorFn(d.cat_color), c.a * 255); } else { color = fixedColor; } @@ -98,10 +100,11 @@ export default class CategoricalDeckGLContainer extends React.PureComponent { } addColor(data, fd) { const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 }; + const colorFn = CategoricalColorManager.getScale(fd.color_scheme); return data.map((d) => { let color; if (fd.dimension) { - color = hexToRGB(getColorFromScheme(d.cat_color, fd.color_scheme), c.a * 255); + color = hexToRGB(colorFn(d.cat_color), c.a * 255); return { ...d, color }; } return d; From dd60dc7713c598414a4b96376d43f31e47cf5163 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 17:38:13 -0700 Subject: [PATCH 06/33] migrate to new function --- superset/assets/src/visualizations/nvd3_vis.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/superset/assets/src/visualizations/nvd3_vis.js b/superset/assets/src/visualizations/nvd3_vis.js index 1baf5f576a0c1..dd9629bbb0dbe 100644 --- a/superset/assets/src/visualizations/nvd3_vis.js +++ b/superset/assets/src/visualizations/nvd3_vis.js @@ -9,7 +9,6 @@ import moment from 'moment'; import d3tip from 'd3-tip'; import dompurify from 'dompurify'; -import { getColorFromScheme } from '../modules/colors'; import AnnotationTypes, { applyNativeColumns, } from '../modules/AnnotationTypes'; @@ -21,6 +20,7 @@ import { t } from '../locales'; // CSS import './nvd3_vis.css'; import { VIZ_TYPES } from './'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; const minBarWidth = 15; // Limit on how large axes margins can grow as the chart window is resized @@ -468,7 +468,8 @@ export default function nvd3Vis(slice, payload) { return `rgba(${c.r}, ${c.g}, ${c.b}, ${alpha})`; }); } else if (vizType !== 'bullet') { - chart.color(d => d.color || getColorFromScheme(d[colorKey], fd.color_scheme)); + const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); + chart.color(d => d.color || colorFn(d[colorKey])); } if ((vizType === 'line' || vizType === 'area') && fd.rich_tooltip) { chart.useInteractiveGuideline(true); @@ -747,7 +748,8 @@ export default function nvd3Vis(slice, payload) { // Add event annotation layer const annotations = d3.select(slice.selector).select('.nv-wrap').append('g') .attr('class', `nv-event-annotation-layer-${index}`); - const aColor = e.color || getColorFromScheme(e.name, fd.color_scheme); + const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); + const aColor = e.color || colorFn(e.name); const tip = tipFactory(e); const records = (slice.annotationData[e.name].records || []).map((r) => { @@ -805,7 +807,8 @@ export default function nvd3Vis(slice, payload) { const annotations = d3.select(slice.selector).select('.nv-wrap').append('g') .attr('class', `nv-interval-annotation-layer-${index}`); - const aColor = e.color || getColorFromScheme(e.name, fd.color_scheme); + const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); + const aColor = e.color || colorFn(e.name); const tip = tipFactory(e); const records = (slice.annotationData[e.name].records || []).map((r) => { From 211a777e45a387f9eb00b01a0c27c01d76a17581 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 17:51:21 -0700 Subject: [PATCH 07/33] Remove ALL_COLOR_SCHEMES --- .../javascripts/explore/components/ColorScheme_spec.jsx | 4 +++- superset/assets/spec/javascripts/modules/colors_spec.jsx | 4 +++- .../src/explore/components/controls/AnnotationLayer.jsx | 6 +++--- superset/assets/src/explore/controls.jsx | 5 ++++- superset/assets/src/modules/colors.js | 2 -- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx index a7d4d66ab6762..a2dfaf632c8a2 100644 --- a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx @@ -7,7 +7,9 @@ import { Creatable } from 'react-select'; import ColorSchemeControl from '../../../../src/explore/components/controls/ColorSchemeControl'; -import { ALL_COLOR_SCHEMES } from '../../../../src/modules/colors'; +import CategoricalColorManager from '../../../../src/modules/CategoricalColorManager'; + +const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); const defaultProps = { options: Object.keys(ALL_COLOR_SCHEMES).map(s => ([s, s])), diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index e83b4732b115c..e797a083082bc 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -1,7 +1,9 @@ import { it, describe } from 'mocha'; import { expect } from 'chai'; +import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; +import CategoricalColorManager from '../../../src/modules/CategoricalColorManager'; -import { ALL_COLOR_SCHEMES, getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; +const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); describe('colors', () => { it('default to bnbColors', () => { diff --git a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx index 812882c2d8b33..2a4cb9be2f844 100644 --- a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx +++ b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx @@ -20,13 +20,13 @@ import AnnotationTypes, { requiresQuery, } from '../../../modules/AnnotationTypes'; -import { ALL_COLOR_SCHEMES } from '../../../modules/colors'; import PopoverSection from '../../../components/PopoverSection'; import ControlHeader from '../ControlHeader'; import { nonEmpty } from '../../validators'; import vizTypes from '../../visTypes'; import { t } from '../../../locales'; +import CategoricalColorManager from '../../../modules/CategoricalColorManager'; const AUTOMATIC_COLOR = ''; @@ -276,7 +276,7 @@ export default class AnnotationLayer extends React.PureComponent { description = t('Select the Annotation Layer you would like to use.'); } else { label = t('Chart'); - description = `Use a pre defined Superset Chart as a source for annotations and overlays. + description = `Use a pre defined Superset Chart as a source for annotations and overlays. 'your chart must be one of these visualization types: '[${getSupportedSourceTypes(annotationType) .map(x => vizTypes[x].label).join(', ')}]'`; @@ -478,7 +478,7 @@ export default class AnnotationLayer extends React.PureComponent { renderDisplayConfiguration() { const { color, opacity, style, width, showMarkers, hideLine, annotationType } = this.state; - const colorScheme = [...ALL_COLOR_SCHEMES[this.props.colorScheme]]; + const colorScheme = [...CategoricalColorManager.getScheme(this.props.colorScheme)]; if (color && color !== AUTOMATIC_COLOR && !colorScheme.find(x => x.toLowerCase() === color.toLowerCase())) { colorScheme.push(color); diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index 5e422eaa20108..d40a762b80a9b 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -45,11 +45,14 @@ import { mainMetric, } from '../modules/utils'; import * as v from './validators'; -import { colorPrimary, ALL_COLOR_SCHEMES, spectrums } from '../modules/colors'; +import { colorPrimary, spectrums } from '../modules/colors'; import { defaultViewport } from '../modules/geo'; import ColumnOption from '../components/ColumnOption'; import OptionDescription from '../components/OptionDescription'; import { t } from '../locales'; +import CategoricalColorManager from '../modules/CategoricalColorManager'; + +const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format'; diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 2980909284c9d..7e1684a5fcef0 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -93,8 +93,6 @@ CategoricalColorManager.registerSchemes({ }); CategoricalColorManager.registerScheme('default', bnbColors); -export const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); - export const spectrums = { blue_white_yellow: [ '#00d1c1', From c1aeeb547db7b79311453ea8d56418e6f5820aeb Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 17:58:51 -0700 Subject: [PATCH 08/33] move sequential colors to another file --- superset/assets/src/explore/controls.jsx | 5 +- .../src/modules/colorSchemes/sequential.js | 433 +++++++++++++++++ superset/assets/src/modules/colors.js | 437 +----------------- 3 files changed, 438 insertions(+), 437 deletions(-) create mode 100644 superset/assets/src/modules/colorSchemes/sequential.js diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index d40a762b80a9b..20cb03c2fea33 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -45,12 +45,13 @@ import { mainMetric, } from '../modules/utils'; import * as v from './validators'; -import { colorPrimary, spectrums } from '../modules/colors'; +import { colorPrimary } from '../modules/colors'; import { defaultViewport } from '../modules/geo'; import ColumnOption from '../components/ColumnOption'; import OptionDescription from '../components/OptionDescription'; import { t } from '../locales'; import CategoricalColorManager from '../modules/CategoricalColorManager'; +import sequentialSchemes from '../modules/colorSchemes/sequential'; const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); @@ -374,7 +375,7 @@ export const controls = { clearable: false, description: '', renderTrigger: true, - schemes: spectrums, + schemes: sequentialSchemes, isLinear: true, }, diff --git a/superset/assets/src/modules/colorSchemes/sequential.js b/superset/assets/src/modules/colorSchemes/sequential.js new file mode 100644 index 0000000000000..6970ed411cb97 --- /dev/null +++ b/superset/assets/src/modules/colorSchemes/sequential.js @@ -0,0 +1,433 @@ +export default { + blue_white_yellow: [ + '#00d1c1', + 'white', + '#ffb400', + ], + fire: [ + 'white', + 'yellow', + 'red', + 'black', + ], + white_black: [ + 'white', + 'black', + ], + black_white: [ + 'black', + 'white', + ], + dark_blue: [ + '#EBF5F8', + '#6BB1CC', + '#357E9B', + '#1B4150', + '#092935', + ], + pink_grey: [ + '#E70B81', + '#FAFAFA', + '#666666', + ], + greens: [ + '#ffffcc', + '#78c679', + '#006837', + ], + purples: [ + '#f2f0f7', + '#9e9ac8', + '#54278f', + ], + oranges: [ + '#fef0d9', + '#fc8d59', + '#b30000', + ], + red_yellow_blue: [ + '#d7191c', + '#fdae61', + '#ffffbf', + '#abd9e9', + '#2c7bb6', + ], + brown_white_green: [ + '#a6611a', + '#dfc27d', + '#f5f5f5', + '#80cdc1', + '#018571', + ], + purple_white_green: [ + '#7b3294', + '#c2a5cf', + '#f7f7f7', + '#a6dba0', + '#008837', + ], + schemeBrBG: [ + '#543005', + '#8c510a', + '#bf812d', + '#dfc27d', + '#f6e8c3', + '#c7eae5', + '#80cdc1', + '#35978f', + '#01665e', + '#003c30', + ], + schemePRGn: [ + '#40004b', + '#762a83', + '#9970ab', + '#c2a5cf', + '#e7d4e8', + '#d9f0d3', + '#a6dba0', + '#5aae61', + '#1b7837', + '#00441b', + ], + schemePiYG: [ + '#8e0152', + '#c51b7d', + '#de77ae', + '#f1b6da', + '#fde0ef', + '#e6f5d0', + '#b8e186', + '#7fbc41', + '#4d9221', + '#276419', + ], + schemePuOr: [ + '#2d004b', + '#542788', + '#8073ac', + '#b2abd2', + '#d8daeb', + '#fee0b6', + '#fdb863', + '#e08214', + '#b35806', + '#7f3b08', + ], + schemeRdBu: [ + '#67001f', + '#b2182b', + '#d6604d', + '#f4a582', + '#fddbc7', + '#d1e5f0', + '#92c5de', + '#4393c3', + '#2166ac', + '#053061', + ], + schemeRdGy: [ + '#67001f', + '#b2182b', + '#d6604d', + '#f4a582', + '#fddbc7', + '#e0e0e0', + '#bababa', + '#878787', + '#4d4d4d', + '#1a1a1a', + ], + schemeRdYlBu: [ + '#a50026', + '#d73027', + '#f46d43', + '#fdae61', + '#fee090', + '#e0f3f8', + '#abd9e9', + '#74add1', + '#4575b4', + '#313695', + ], + schemeRdYlGn: [ + '#a50026', + '#d73027', + '#f46d43', + '#fdae61', + '#fee08b', + '#d9ef8b', + '#a6d96a', + '#66bd63', + '#1a9850', + '#006837', + ], + schemeSpectral: [ + '#9e0142', + '#d53e4f', + '#f46d43', + '#fdae61', + '#fee08b', + '#e6f598', + '#abdda4', + '#66c2a5', + '#3288bd', + '#5e4fa2', + ], + schemeBlues: [ + '#b5d4e9', + '#93c3df', + '#6daed5', + '#4b97c9', + '#2f7ebc', + '#1864aa', + '#0a4a90', + '#08306b', + ], + schemeGreens: [ + '#b7e2b1', + '#97d494', + '#73c378', + '#4daf62', + '#2f984f', + '#157f3b', + '#036429', + '#00441b', + ], + schemeGrays: [ + '#cecece', + '#b4b4b4', + '#979797', + '#7a7a7a', + '#5f5f5f', + '#404040', + '#1e1e1e', + '#000000', + ], + schemeOranges: [ + '#fdc28c', + '#fda762', + '#fb8d3d', + '#f2701d', + '#e25609', + '#c44103', + '#9f3303', + '#7f2704', + ], + schemePurples: [ + '#cecee5', + '#b6b5d8', + '#9e9bc9', + '#8782bc', + '#7363ac', + '#61409b', + '#501f8c', + '#3f007d', + ], + schemeReds: [ + '#fcaa8e', + '#fc8a6b', + '#f9694c', + '#ef4533', + '#d92723', + '#bb151a', + '#970b13', + '#67000d', + ], + schemeViridis: [ + '#482475', + '#414487', + '#355f8d', + '#2a788e', + '#21918c', + '#22a884', + '#44bf70', + '#7ad151', + '#bddf26', + '#fde725', + ], + schemeInferno: [ + '#160b39', + '#420a68', + '#6a176e', + '#932667', + '#bc3754', + '#dd513a', + '#f37819', + '#fca50a', + '#f6d746', + '#fcffa4', + ], + schemeMagma: [ + '#140e36', + '#3b0f70', + '#641a80', + '#8c2981', + '#b73779', + '#de4968', + '#f7705c', + '#fe9f6d', + '#fecf92', + '#fcfdbf', + ], + schemeWarm: [ + '#963db3', + '#bf3caf', + '#e4419d', + '#fe4b83', + '#ff5e63', + '#ff7847', + '#fb9633', + '#e2b72f', + '#c6d63c', + '#aff05b', + ], + schemeCool: [ + '#6054c8', + '#4c6edb', + '#368ce1', + '#23abd8', + '#1ac7c2', + '#1ddfa3', + '#30ef82', + '#52f667', + '#7ff658', + '#aff05b', + ], + schemeCubehelixDefault: [ + '#1a1530', + '#163d4e', + '#1f6642', + '#54792f', + '#a07949', + '#d07e93', + '#cf9cda', + '#c1caf3', + '#d2eeef', + '#ffffff', + ], + schemeBuGn: [ + '#b7e4da', + '#8fd3c1', + '#68c2a3', + '#49b17f', + '#2f9959', + '#157f3c', + '#036429', + '#00441b', + ], + schemeBuPu: [ + '#b2cae1', + '#9cb3d5', + '#8f95c6', + '#8c74b5', + '#8952a5', + '#852d8f', + '#730f71', + '#4d004b', + ], + schemeGnBu: [ + '#bde5bf', + '#9ed9bb', + '#7bcbc4', + '#58b7cd', + '#399cc6', + '#1d7eb7', + '#0b60a1', + '#084081', + ], + schemeOrRd: [ + '#fdca94', + '#fdb07a', + '#fa8e5d', + '#f16c49', + '#e04530', + '#c81d13', + '#a70403', + '#7f0000', + ], + schemePuBuGn: [ + '#bec9e2', + '#98b9d9', + '#69a8cf', + '#4096c0', + '#19879f', + '#037877', + '#016353', + '#014636', + ], + schemePuBu: [ + '#bfc9e2', + '#9bb9d9', + '#72a8cf', + '#4394c3', + '#1a7db6', + '#0667a1', + '#045281', + '#023858', + ], + schemePuRd: [ + '#d0aad2', + '#d08ac2', + '#dd63ae', + '#e33890', + '#d71c6c', + '#b70b4f', + '#8f023a', + '#67001f', + ], + schemeRdPu: [ + '#fbb5bc', + '#f993b0', + '#f369a3', + '#e03e98', + '#c01788', + '#99037c', + '#700174', + '#49006a', + ], + schemeYlGnBu: [ + '#d5eeb3', + '#a9ddb7', + '#73c9bd', + '#45b4c2', + '#2897bf', + '#2073b2', + '#234ea0', + '#1c3185', + '#081d58', + ], + schemeYlGn: [ + '#e4f4ac', + '#c7e89b', + '#a2d88a', + '#78c578', + '#4eaf63', + '#2f944e', + '#15793f', + '#036034', + '#004529', + ], + schemeYlOrBr: [ + '#feeaa1', + '#fed676', + '#feba4a', + '#fb992c', + '#ee7918', + '#d85b0a', + '#b74304', + '#8f3204', + '#662506', + ], + schemeYlOrRd: [ + '#fee087', + '#fec965', + '#feab4b', + '#fd893c', + '#fa5c2e', + '#ec3023', + '#d31121', + '#af0225', + '#800026', + ], +}; diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 7e1684a5fcef0..19b97739c3991 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -1,5 +1,6 @@ import d3 from 'd3'; import CategoricalColorManager from './CategoricalColorManager'; +import sequential from './colorSchemes/sequential'; export const brandColor = '#00A699'; export const colorPrimary = { r: 0, g: 122, b: 135, a: 1 }; @@ -93,440 +94,6 @@ CategoricalColorManager.registerSchemes({ }); CategoricalColorManager.registerScheme('default', bnbColors); -export const spectrums = { - blue_white_yellow: [ - '#00d1c1', - 'white', - '#ffb400', - ], - fire: [ - 'white', - 'yellow', - 'red', - 'black', - ], - white_black: [ - 'white', - 'black', - ], - black_white: [ - 'black', - 'white', - ], - dark_blue: [ - '#EBF5F8', - '#6BB1CC', - '#357E9B', - '#1B4150', - '#092935', - ], - pink_grey: [ - '#E70B81', - '#FAFAFA', - '#666666', - ], - greens: [ - '#ffffcc', - '#78c679', - '#006837', - ], - purples: [ - '#f2f0f7', - '#9e9ac8', - '#54278f', - ], - oranges: [ - '#fef0d9', - '#fc8d59', - '#b30000', - ], - red_yellow_blue: [ - '#d7191c', - '#fdae61', - '#ffffbf', - '#abd9e9', - '#2c7bb6', - ], - brown_white_green: [ - '#a6611a', - '#dfc27d', - '#f5f5f5', - '#80cdc1', - '#018571', - ], - purple_white_green: [ - '#7b3294', - '#c2a5cf', - '#f7f7f7', - '#a6dba0', - '#008837', - ], - schemeBrBG: [ - '#543005', - '#8c510a', - '#bf812d', - '#dfc27d', - '#f6e8c3', - '#c7eae5', - '#80cdc1', - '#35978f', - '#01665e', - '#003c30', - ], - schemePRGn: [ - '#40004b', - '#762a83', - '#9970ab', - '#c2a5cf', - '#e7d4e8', - '#d9f0d3', - '#a6dba0', - '#5aae61', - '#1b7837', - '#00441b', - ], - schemePiYG: [ - '#8e0152', - '#c51b7d', - '#de77ae', - '#f1b6da', - '#fde0ef', - '#e6f5d0', - '#b8e186', - '#7fbc41', - '#4d9221', - '#276419', - ], - schemePuOr: [ - '#2d004b', - '#542788', - '#8073ac', - '#b2abd2', - '#d8daeb', - '#fee0b6', - '#fdb863', - '#e08214', - '#b35806', - '#7f3b08', - ], - schemeRdBu: [ - '#67001f', - '#b2182b', - '#d6604d', - '#f4a582', - '#fddbc7', - '#d1e5f0', - '#92c5de', - '#4393c3', - '#2166ac', - '#053061', - ], - schemeRdGy: [ - '#67001f', - '#b2182b', - '#d6604d', - '#f4a582', - '#fddbc7', - '#e0e0e0', - '#bababa', - '#878787', - '#4d4d4d', - '#1a1a1a', - ], - schemeRdYlBu: [ - '#a50026', - '#d73027', - '#f46d43', - '#fdae61', - '#fee090', - '#e0f3f8', - '#abd9e9', - '#74add1', - '#4575b4', - '#313695', - ], - schemeRdYlGn: [ - '#a50026', - '#d73027', - '#f46d43', - '#fdae61', - '#fee08b', - '#d9ef8b', - '#a6d96a', - '#66bd63', - '#1a9850', - '#006837', - ], - schemeSpectral: [ - '#9e0142', - '#d53e4f', - '#f46d43', - '#fdae61', - '#fee08b', - '#e6f598', - '#abdda4', - '#66c2a5', - '#3288bd', - '#5e4fa2', - ], - schemeBlues: [ - '#b5d4e9', - '#93c3df', - '#6daed5', - '#4b97c9', - '#2f7ebc', - '#1864aa', - '#0a4a90', - '#08306b', - ], - schemeGreens: [ - '#b7e2b1', - '#97d494', - '#73c378', - '#4daf62', - '#2f984f', - '#157f3b', - '#036429', - '#00441b', - ], - schemeGrays: [ - '#cecece', - '#b4b4b4', - '#979797', - '#7a7a7a', - '#5f5f5f', - '#404040', - '#1e1e1e', - '#000000', - ], - schemeOranges: [ - '#fdc28c', - '#fda762', - '#fb8d3d', - '#f2701d', - '#e25609', - '#c44103', - '#9f3303', - '#7f2704', - ], - schemePurples: [ - '#cecee5', - '#b6b5d8', - '#9e9bc9', - '#8782bc', - '#7363ac', - '#61409b', - '#501f8c', - '#3f007d', - ], - schemeReds: [ - '#fcaa8e', - '#fc8a6b', - '#f9694c', - '#ef4533', - '#d92723', - '#bb151a', - '#970b13', - '#67000d', - ], - schemeViridis: [ - '#482475', - '#414487', - '#355f8d', - '#2a788e', - '#21918c', - '#22a884', - '#44bf70', - '#7ad151', - '#bddf26', - '#fde725', - ], - schemeInferno: [ - '#160b39', - '#420a68', - '#6a176e', - '#932667', - '#bc3754', - '#dd513a', - '#f37819', - '#fca50a', - '#f6d746', - '#fcffa4', - ], - schemeMagma: [ - '#140e36', - '#3b0f70', - '#641a80', - '#8c2981', - '#b73779', - '#de4968', - '#f7705c', - '#fe9f6d', - '#fecf92', - '#fcfdbf', - ], - schemeWarm: [ - '#963db3', - '#bf3caf', - '#e4419d', - '#fe4b83', - '#ff5e63', - '#ff7847', - '#fb9633', - '#e2b72f', - '#c6d63c', - '#aff05b', - ], - schemeCool: [ - '#6054c8', - '#4c6edb', - '#368ce1', - '#23abd8', - '#1ac7c2', - '#1ddfa3', - '#30ef82', - '#52f667', - '#7ff658', - '#aff05b', - ], - schemeCubehelixDefault: [ - '#1a1530', - '#163d4e', - '#1f6642', - '#54792f', - '#a07949', - '#d07e93', - '#cf9cda', - '#c1caf3', - '#d2eeef', - '#ffffff', - ], - schemeBuGn: [ - '#b7e4da', - '#8fd3c1', - '#68c2a3', - '#49b17f', - '#2f9959', - '#157f3c', - '#036429', - '#00441b', - ], - schemeBuPu: [ - '#b2cae1', - '#9cb3d5', - '#8f95c6', - '#8c74b5', - '#8952a5', - '#852d8f', - '#730f71', - '#4d004b', - ], - schemeGnBu: [ - '#bde5bf', - '#9ed9bb', - '#7bcbc4', - '#58b7cd', - '#399cc6', - '#1d7eb7', - '#0b60a1', - '#084081', - ], - schemeOrRd: [ - '#fdca94', - '#fdb07a', - '#fa8e5d', - '#f16c49', - '#e04530', - '#c81d13', - '#a70403', - '#7f0000', - ], - schemePuBuGn: [ - '#bec9e2', - '#98b9d9', - '#69a8cf', - '#4096c0', - '#19879f', - '#037877', - '#016353', - '#014636', - ], - schemePuBu: [ - '#bfc9e2', - '#9bb9d9', - '#72a8cf', - '#4394c3', - '#1a7db6', - '#0667a1', - '#045281', - '#023858', - ], - schemePuRd: [ - '#d0aad2', - '#d08ac2', - '#dd63ae', - '#e33890', - '#d71c6c', - '#b70b4f', - '#8f023a', - '#67001f', - ], - schemeRdPu: [ - '#fbb5bc', - '#f993b0', - '#f369a3', - '#e03e98', - '#c01788', - '#99037c', - '#700174', - '#49006a', - ], - schemeYlGnBu: [ - '#d5eeb3', - '#a9ddb7', - '#73c9bd', - '#45b4c2', - '#2897bf', - '#2073b2', - '#234ea0', - '#1c3185', - '#081d58', - ], - schemeYlGn: [ - '#e4f4ac', - '#c7e89b', - '#a2d88a', - '#78c578', - '#4eaf63', - '#2f944e', - '#15793f', - '#036034', - '#004529', - ], - schemeYlOrBr: [ - '#feeaa1', - '#fed676', - '#feba4a', - '#fb992c', - '#ee7918', - '#d85b0a', - '#b74304', - '#8f3204', - '#662506', - ], - schemeYlOrRd: [ - '#fee087', - '#fec965', - '#feab4b', - '#fd893c', - '#fa5c2e', - '#ec3023', - '#d31121', - '#af0225', - '#800026', - ], -}; - export function hexToRGB(hex, alpha = 255) { if (!hex) { return [0, 0, 0, alpha]; @@ -561,7 +128,7 @@ export const colorScalerFactory = function (colors, data, accessor, extents, out // Returns a linear scaler our of an array of color if (!Array.isArray(colors)) { /* eslint no-param-reassign: 0 */ - colors = spectrums[colors]; + colors = sequential[colors]; } let ext = [0, 1]; if (extents) { From 56f9ef33b2c0a124d5a63c122a8615776148878a Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 18:08:08 -0700 Subject: [PATCH 09/33] extract categorical colors to separate file --- .../src/modules/colorSchemes/categorical.js | 42 ++++++++++++++ superset/assets/src/modules/colors.js | 57 ++----------------- 2 files changed, 48 insertions(+), 51 deletions(-) create mode 100644 superset/assets/src/modules/colorSchemes/categorical.js diff --git a/superset/assets/src/modules/colorSchemes/categorical.js b/superset/assets/src/modules/colorSchemes/categorical.js new file mode 100644 index 0000000000000..946d14a187b72 --- /dev/null +++ b/superset/assets/src/modules/colorSchemes/categorical.js @@ -0,0 +1,42 @@ +import d3 from 'd3'; + +export default { + d3Category10: d3.scale.category10().range(), + d3Category20: d3.scale.category20().range(), + d3Category20b: d3.scale.category20b().range(), + d3Category20c: d3.scale.category20c().range(), + googleCategory10c: [ + '#3366cc', + '#dc3912', + '#ff9900', + '#109618', + '#990099', + '#0099c6', + '#dd4477', + '#66aa00', + '#b82e2e', + '#316395', + ], + googleCategory20c: [ + '#3366cc', + '#dc3912', + '#ff9900', + '#109618', + '#990099', + '#0099c6', + '#dd4477', + '#66aa00', + '#b82e2e', + '#316395', + '#994499', + '#22aa99', + '#aaaa11', + '#6633cc', + '#e67300', + '#8b0707', + '#651067', + '#329262', + '#5574a6', + '#3b3eac', + ], +}; diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 19b97739c3991..058730d56aaf0 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -1,6 +1,7 @@ import d3 from 'd3'; import CategoricalColorManager from './CategoricalColorManager'; -import sequential from './colorSchemes/sequential'; +import categoricalSchemes from './colorSchemes/categorical'; +import sequentialSchemes from './colorSchemes/sequential'; export const brandColor = '#00A699'; export const colorPrimary = { r: 0, g: 122, b: 135, a: 1 }; @@ -43,55 +44,9 @@ export const lyftColors = [ '#AC2077', ]; -const d3Category10 = d3.scale.category10().range(); -const d3Category20 = d3.scale.category20().range(); -const d3Category20b = d3.scale.category20b().range(); -const d3Category20c = d3.scale.category20c().range(); -const googleCategory10c = [ - '#3366cc', - '#dc3912', - '#ff9900', - '#109618', - '#990099', - '#0099c6', - '#dd4477', - '#66aa00', - '#b82e2e', - '#316395', -]; -const googleCategory20c = [ - '#3366cc', - '#dc3912', - '#ff9900', - '#109618', - '#990099', - '#0099c6', - '#dd4477', - '#66aa00', - '#b82e2e', - '#316395', - '#994499', - '#22aa99', - '#aaaa11', - '#6633cc', - '#e67300', - '#8b0707', - '#651067', - '#329262', - '#5574a6', - '#3b3eac', -]; - -CategoricalColorManager.registerSchemes({ - bnbColors, - d3Category10, - d3Category20, - d3Category20b, - d3Category20c, - googleCategory10c, - googleCategory20c, - lyftColors, -}); +CategoricalColorManager.registerSchemes({ bnbColors }); +CategoricalColorManager.registerSchemes(categoricalSchemes); +CategoricalColorManager.registerSchemes({ lyftColors }); CategoricalColorManager.registerScheme('default', bnbColors); export function hexToRGB(hex, alpha = 255) { @@ -128,7 +83,7 @@ export const colorScalerFactory = function (colors, data, accessor, extents, out // Returns a linear scaler our of an array of color if (!Array.isArray(colors)) { /* eslint no-param-reassign: 0 */ - colors = sequential[colors]; + colors = sequentialSchemes[colors]; } let ext = [0, 1]; if (extents) { From f3671c87743e51eb2b151291e379e6851d788a98 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 18:20:30 -0700 Subject: [PATCH 10/33] move airbnb and lyft colors to separate files --- .../src/modules/CategoricalColorManager.js | 8 ++++ .../assets/src/modules/colorSchemes/airbnb.js | 25 ++++++++++ .../assets/src/modules/colorSchemes/lyft.js | 14 ++++++ superset/assets/src/modules/colors.js | 48 ++----------------- 4 files changed, 51 insertions(+), 44 deletions(-) create mode 100644 superset/assets/src/modules/colorSchemes/airbnb.js create mode 100644 superset/assets/src/modules/colorSchemes/lyft.js diff --git a/superset/assets/src/modules/CategoricalColorManager.js b/superset/assets/src/modules/CategoricalColorManager.js index 886a8b3b222f2..c96951b08983d 100644 --- a/superset/assets/src/modules/CategoricalColorManager.js +++ b/superset/assets/src/modules/CategoricalColorManager.js @@ -1,4 +1,7 @@ import CategoricalColorScale from './CategoricalColorScale'; +import categoricalSchemes from './colorSchemes/categorical'; +import airbnb from './colorSchemes/airbnb'; +import lyft from './colorSchemes/lyft'; const schemes = {}; @@ -57,4 +60,9 @@ Object.assign(CategoricalColorManager, { getSchemes, }); +CategoricalColorManager.registerSchemes({ bnbColors: airbnb.bnbColors }); +CategoricalColorManager.registerSchemes(categoricalSchemes); +CategoricalColorManager.registerSchemes({ lyftColors: lyft.lyftColors }); +CategoricalColorManager.registerScheme('default', airbnb.bnbColors); + export default CategoricalColorManager; diff --git a/superset/assets/src/modules/colorSchemes/airbnb.js b/superset/assets/src/modules/colorSchemes/airbnb.js new file mode 100644 index 0000000000000..d26a923e4d616 --- /dev/null +++ b/superset/assets/src/modules/colorSchemes/airbnb.js @@ -0,0 +1,25 @@ +export default { + bnbColors: [ + '#ff5a5f', // rausch + '#7b0051', // hackb + '#007A87', // kazan + '#00d1c1', // babu + '#8ce071', // lima + '#ffb400', // beach + '#b4a76c', // barol + '#ff8083', + '#cc0086', + '#00a1b3', + '#00ffeb', + '#bbedab', + '#ffd266', + '#cbc29a', + '#ff3339', + '#ff1ab1', + '#005c66', + '#00b3a5', + '#55d12e', + '#b37e00', + '#988b4e', + ], +}; diff --git a/superset/assets/src/modules/colorSchemes/lyft.js b/superset/assets/src/modules/colorSchemes/lyft.js new file mode 100644 index 0000000000000..cd9412163e97d --- /dev/null +++ b/superset/assets/src/modules/colorSchemes/lyft.js @@ -0,0 +1,14 @@ +export default { + lyftColors: [ + '#EA0B8C', + '#6C838E', + '#29ABE2', + '#33D9C1', + '#9DACB9', + '#7560AA', + '#2D5584', + '#831C4A', + '#333D47', + '#AC2077', + ], +}; diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 058730d56aaf0..a60313474fd83 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -1,53 +1,13 @@ import d3 from 'd3'; import CategoricalColorManager from './CategoricalColorManager'; -import categoricalSchemes from './colorSchemes/categorical'; import sequentialSchemes from './colorSchemes/sequential'; +import airbnb from './colorSchemes/airbnb'; +import lyft from './colorSchemes/lyft'; export const brandColor = '#00A699'; export const colorPrimary = { r: 0, g: 122, b: 135, a: 1 }; - -// Color related utility functions go in this object -export const bnbColors = [ - '#ff5a5f', // rausch - '#7b0051', // hackb - '#007A87', // kazan - '#00d1c1', // babu - '#8ce071', // lima - '#ffb400', // beach - '#b4a76c', // barol - '#ff8083', - '#cc0086', - '#00a1b3', - '#00ffeb', - '#bbedab', - '#ffd266', - '#cbc29a', - '#ff3339', - '#ff1ab1', - '#005c66', - '#00b3a5', - '#55d12e', - '#b37e00', - '#988b4e', -]; - -export const lyftColors = [ - '#EA0B8C', - '#6C838E', - '#29ABE2', - '#33D9C1', - '#9DACB9', - '#7560AA', - '#2D5584', - '#831C4A', - '#333D47', - '#AC2077', -]; - -CategoricalColorManager.registerSchemes({ bnbColors }); -CategoricalColorManager.registerSchemes(categoricalSchemes); -CategoricalColorManager.registerSchemes({ lyftColors }); -CategoricalColorManager.registerScheme('default', bnbColors); +export const bnbColors = airbnb.bnbColors; +export const lyftColors = lyft.lyftColors; export function hexToRGB(hex, alpha = 255) { if (!hex) { From 9894368613e82afc690693dd37e4eae0f8dff5fa Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 4 Sep 2018 18:27:38 -0700 Subject: [PATCH 11/33] fix missing toFunction() --- .../src/visualizations/deckgl/CategoricalDeckGLContainer.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx index cada6c5ecf2c2..a242548a451de 100644 --- a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx +++ b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx @@ -14,7 +14,7 @@ import CategoricalColorManager from '../../modules/CategoricalColorManager'; function getCategories(fd, data) { const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 }; const fixedColor = [c.r, c.g, c.b, 255 * c.a]; - const colorFn = CategoricalColorManager.getScale(fd.color_scheme); + const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); const categories = {}; data.forEach((d) => { if (d.cat_color != null && !categories.hasOwnProperty(d.cat_color)) { @@ -100,7 +100,7 @@ export default class CategoricalDeckGLContainer extends React.PureComponent { } addColor(data, fd) { const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 }; - const colorFn = CategoricalColorManager.getScale(fd.color_scheme); + const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); return data.map((d) => { let color; if (fd.dimension) { From 943ecacaf9c61d808cb7e31c8acf02ecd2411195 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 16:19:28 -0700 Subject: [PATCH 12/33] Rewrite to support local and global force items, plus namespacing. --- .../src/modules/CategoricalColorManager.js | 68 ------------------- .../src/modules/CategoricalColorNamespace.js | 52 ++++++++++++++ .../src/modules/CategoricalColorScale.js | 26 ++++--- .../assets/src/modules/ColorSchemeManager.js | 64 +++++++++++++++++ 4 files changed, 134 insertions(+), 76 deletions(-) delete mode 100644 superset/assets/src/modules/CategoricalColorManager.js create mode 100644 superset/assets/src/modules/CategoricalColorNamespace.js create mode 100644 superset/assets/src/modules/ColorSchemeManager.js diff --git a/superset/assets/src/modules/CategoricalColorManager.js b/superset/assets/src/modules/CategoricalColorManager.js deleted file mode 100644 index c96951b08983d..0000000000000 --- a/superset/assets/src/modules/CategoricalColorManager.js +++ /dev/null @@ -1,68 +0,0 @@ -import CategoricalColorScale from './CategoricalColorScale'; -import categoricalSchemes from './colorSchemes/categorical'; -import airbnb from './colorSchemes/airbnb'; -import lyft from './colorSchemes/lyft'; - -const schemes = {}; - -function getScheme(schemeName = 'default') { - return schemes[schemeName]; -} - -function getSchemes() { - return schemes; -} - -function registerScheme(schemeName, colors) { - schemes[schemeName] = colors; -} - -function registerSchemes(multipleSchemes) { - Object.assign(schemes, multipleSchemes); -} - -class CategoricalColorManager { - constructor() { - this.scales = {}; - } - - getScale(schemeName = 'default') { - const scale = this.scales[schemeName]; - if (scale) { - return scale; - } - const colors = getScheme(schemeName); - const newScale = new CategoricalColorScale(colors); - this.scales[schemeName] = newScale; - return newScale; - } -} - -let singleton; - -function getInstance() { - if (!singleton) { - singleton = new CategoricalColorManager(); - } - return singleton; -} - -function getScale(schemeName) { - return getInstance().getScale(schemeName); -} - -Object.assign(CategoricalColorManager, { - getInstance, - getScale, - registerScheme, - registerSchemes, - getScheme, - getSchemes, -}); - -CategoricalColorManager.registerSchemes({ bnbColors: airbnb.bnbColors }); -CategoricalColorManager.registerSchemes(categoricalSchemes); -CategoricalColorManager.registerSchemes({ lyftColors: lyft.lyftColors }); -CategoricalColorManager.registerScheme('default', airbnb.bnbColors); - -export default CategoricalColorManager; diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js new file mode 100644 index 0000000000000..fa5ddc5394238 --- /dev/null +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -0,0 +1,52 @@ +import CategoricalColorScale from './CategoricalColorScale'; +import ColorSchemeManager from './ColorSchemeManager'; + +class CategoricalColorNamespace { + constructor() { + this.scales = {}; + this.forcedItems = {}; + } + + getScale(schemeName) { + const name = schemeName || ColorSchemeManager.getDefaultSchemeName(); + const scale = this.scales[name]; + if (scale) { + return scale; + } + const newScale = new CategoricalColorScale( + ColorSchemeManager.getScheme(name), + this.forcedItems, + ); + this.scales[name] = newScale; + return newScale; + } + + /** + * Enforce specific color for given value + * This will apply across all color scales + * in this namespace. + * @param {*} value value + * @param {*} forcedColor color + */ + setColor(value, forcedColor) { + this.forcedItems[value] = forcedColor; + return this; + } +} + +const namespaces = {}; +const DEFAULT_NAMESPACE = 'DEFAULT'; + +function getNamespace(namespace = DEFAULT_NAMESPACE) { + const instance = namespaces[namespace]; + if (instance) { + return instance; + } + const newInstance = new CategoricalColorNamespace(); + namespaces[namespace] = newInstance; + return newInstance; +} + +CategoricalColorNamespace.getNamespace = getNamespace; + +export default CategoricalColorNamespace; diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index f5af5dfab7695..ea11e3f9f310d 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -1,20 +1,19 @@ import { TIME_SHIFT_PATTERN } from '../utils/common'; function cleanValue(value) { - return String(value).trim() - .toLowerCase() // for superset series that should have the same color + return String(value).trim() + .toLowerCase() .split(', ') .filter(k => !TIME_SHIFT_PATTERN.test(k)) .join(', '); } -const sharedForcedItems = {}; - export default class CategoricalColorScale { - constructor(colors, forcedItems = sharedForcedItems) { + constructor(colors, sharedForcedItems) { this.colors = colors; - this.forcedItems = forcedItems; + this.sharedForcedItems = sharedForcedItems; + this.forcedItems = {}; this.seen = {}; this.fn = value => this.getColor(value); } @@ -22,21 +21,32 @@ export default class CategoricalColorScale { getColor(value) { const cleanedValue = cleanValue(value); + const sharedColor = this.sharedForcedItems && this.shareForcedItems[cleanedValue]; + if (sharedColor) { + return sharedColor; + } + const forcedColor = this.forcedItems[cleanedValue]; if (forcedColor) { return forcedColor; } const seenColor = this.seen[cleanedValue]; + const length = this.colors.length; if (seenColor !== undefined) { - return this.colors[seenColor % this.colors.length]; + return this.colors[seenColor % length]; } const index = Object.keys(this.seen).length; this.seen[cleanedValue] = index; - return this.colors[index % this.colors.length]; + return this.colors[index % length]; } + /** + * Enforce specific color for given value + * @param {*} value value + * @param {*} forcedColor forcedColor + */ setColor(value, forcedColor) { this.forcedItems[value] = forcedColor; return this; diff --git a/superset/assets/src/modules/ColorSchemeManager.js b/superset/assets/src/modules/ColorSchemeManager.js new file mode 100644 index 0000000000000..2e5d029f06b8d --- /dev/null +++ b/superset/assets/src/modules/ColorSchemeManager.js @@ -0,0 +1,64 @@ +import airbnb from './colorSchemes/airbnb'; +import categoricalSchemes from './colorSchemes/categorical'; +import lyft from './colorSchemes/lyft'; + +class ColorSchemeManager { + constructor() { + this.schemes = {}; + this.defaultSchemeName = undefined; + } + + get(schemeName) { + return this.schemes[schemeName || this.defaultSchemeName]; + } + + getAll() { + return this.schemes; + } + + getDefaultSchemeName() { + return this.defaultSchemeName; + } + + setDefaultSchemeName(schemeName) { + this.defaultSchemeName = schemeName; + } + + register(schemeName, colors) { + this.schemes[schemeName] = colors; + // If there is no default, set as default + if (!this.defaultSchemeName) { + this.defaultSchemeName = schemeName; + } + } + + registerAll(multipleSchemes) { + Object.assign(this.schemes, multipleSchemes); + // If there is no default, set the first scheme as default + const keys = Object.keys(multipleSchemes); + if (!this.defaultSchemeName && keys.length > 0) { + this.defaultSchemeName = keys[0]; + } + } +} + +let singleton; + +function getInstance() { + if (!singleton) { + singleton = new ColorSchemeManager(); + } + return singleton; +} + +ColorSchemeManager.getInstance = getInstance; + +// These registration code eventually should go into per-app configuration +// when we are completely in the plug-in system. +const manager = ColorSchemeManager.getInstance(); +manager.register('bnbColors', airbnb.bnbColors); +manager.registerAll(categoricalSchemes); +manager.register('lyftColors', lyft.lyftColors); +manager.setDefaultSchemeName('bnbColors'); + +export default ColorSchemeManager; From 33000800b65ef06dc0e3b8df07516acc1a89b182 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 16:32:00 -0700 Subject: [PATCH 13/33] fix references --- .../spec/javascripts/explore/components/ColorScheme_spec.jsx | 4 ++-- superset/assets/spec/javascripts/modules/colors_spec.jsx | 4 ++-- .../src/explore/components/controls/AnnotationLayer.jsx | 4 ++-- superset/assets/src/explore/controls.jsx | 4 ++-- superset/assets/src/modules/colors.js | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx index a2dfaf632c8a2..2c62f19ba7793 100644 --- a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx @@ -7,9 +7,9 @@ import { Creatable } from 'react-select'; import ColorSchemeControl from '../../../../src/explore/components/controls/ColorSchemeControl'; -import CategoricalColorManager from '../../../../src/modules/CategoricalColorManager'; +import ColorSchemeManager from '../../../../src/modules/ColorSchemeManager'; -const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAll(); const defaultProps = { options: Object.keys(ALL_COLOR_SCHEMES).map(s => ([s, s])), diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index e797a083082bc..0aad8ec57fc84 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -1,9 +1,9 @@ import { it, describe } from 'mocha'; import { expect } from 'chai'; import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; -import CategoricalColorManager from '../../../src/modules/CategoricalColorManager'; +import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; -const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAll(); describe('colors', () => { it('default to bnbColors', () => { diff --git a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx index 2a4cb9be2f844..1924936fff074 100644 --- a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx +++ b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx @@ -26,7 +26,7 @@ import { nonEmpty } from '../../validators'; import vizTypes from '../../visTypes'; import { t } from '../../../locales'; -import CategoricalColorManager from '../../../modules/CategoricalColorManager'; +import ColorSchemeManager from '../../../modules/ColorSchemeManager'; const AUTOMATIC_COLOR = ''; @@ -478,7 +478,7 @@ export default class AnnotationLayer extends React.PureComponent { renderDisplayConfiguration() { const { color, opacity, style, width, showMarkers, hideLine, annotationType } = this.state; - const colorScheme = [...CategoricalColorManager.getScheme(this.props.colorScheme)]; + const colorScheme = [...ColorSchemeManager.getInstance().get(this.props.colorScheme)]; if (color && color !== AUTOMATIC_COLOR && !colorScheme.find(x => x.toLowerCase() === color.toLowerCase())) { colorScheme.push(color); diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index 20cb03c2fea33..a27d3af499fbb 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -50,10 +50,10 @@ import { defaultViewport } from '../modules/geo'; import ColumnOption from '../components/ColumnOption'; import OptionDescription from '../components/OptionDescription'; import { t } from '../locales'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; +import ColorSchemeManager from '../modules/ColorSchemeManager'; import sequentialSchemes from '../modules/colorSchemes/sequential'; -const ALL_COLOR_SCHEMES = CategoricalColorManager.getSchemes(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAll(); const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format'; diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index a60313474fd83..0a6126dde018c 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -1,5 +1,5 @@ import d3 from 'd3'; -import CategoricalColorManager from './CategoricalColorManager'; +import CategoricalColorNamespace from './CategoricalColorNamespace'; import sequentialSchemes from './colorSchemes/sequential'; import airbnb from './colorSchemes/airbnb'; import lyft from './colorSchemes/lyft'; @@ -31,7 +31,7 @@ export function hexToRGB(hex, alpha = 255) { forcibly associate to a label. */ export function getColorFromScheme(value, schemeName, forcedColor) { - const scale = CategoricalColorManager.getScale(schemeName); + const scale = CategoricalColorNamespace.getNamespace().getScale(schemeName); if (forcedColor) { scale.setColor(value, forcedColor); return forcedColor; From 2efac2ee5ba83bcb28d8a34d64edfc22741eafc6 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 16:32:47 -0700 Subject: [PATCH 14/33] revert nvd3 --- superset/assets/src/visualizations/nvd3_vis.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/superset/assets/src/visualizations/nvd3_vis.js b/superset/assets/src/visualizations/nvd3_vis.js index dd9629bbb0dbe..1baf5f576a0c1 100644 --- a/superset/assets/src/visualizations/nvd3_vis.js +++ b/superset/assets/src/visualizations/nvd3_vis.js @@ -9,6 +9,7 @@ import moment from 'moment'; import d3tip from 'd3-tip'; import dompurify from 'dompurify'; +import { getColorFromScheme } from '../modules/colors'; import AnnotationTypes, { applyNativeColumns, } from '../modules/AnnotationTypes'; @@ -20,7 +21,6 @@ import { t } from '../locales'; // CSS import './nvd3_vis.css'; import { VIZ_TYPES } from './'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; const minBarWidth = 15; // Limit on how large axes margins can grow as the chart window is resized @@ -468,8 +468,7 @@ export default function nvd3Vis(slice, payload) { return `rgba(${c.r}, ${c.g}, ${c.b}, ${alpha})`; }); } else if (vizType !== 'bullet') { - const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); - chart.color(d => d.color || colorFn(d[colorKey])); + chart.color(d => d.color || getColorFromScheme(d[colorKey], fd.color_scheme)); } if ((vizType === 'line' || vizType === 'area') && fd.rich_tooltip) { chart.useInteractiveGuideline(true); @@ -748,8 +747,7 @@ export default function nvd3Vis(slice, payload) { // Add event annotation layer const annotations = d3.select(slice.selector).select('.nv-wrap').append('g') .attr('class', `nv-event-annotation-layer-${index}`); - const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); - const aColor = e.color || colorFn(e.name); + const aColor = e.color || getColorFromScheme(e.name, fd.color_scheme); const tip = tipFactory(e); const records = (slice.annotationData[e.name].records || []).map((r) => { @@ -807,8 +805,7 @@ export default function nvd3Vis(slice, payload) { const annotations = d3.select(slice.selector).select('.nv-wrap').append('g') .attr('class', `nv-interval-annotation-layer-${index}`); - const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); - const aColor = e.color || colorFn(e.name); + const aColor = e.color || getColorFromScheme(e.name, fd.color_scheme); const tip = tipFactory(e); const records = (slice.annotationData[e.name].records || []).map((r) => { From 1515f05ee4191b34f6fc13fa155fec7995d8c8f5 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 17:04:47 -0700 Subject: [PATCH 15/33] update namespace api --- .../explore/components/ColorScheme_spec.jsx | 5 ++-- .../spec/javascripts/modules/colors_spec.jsx | 2 +- superset/assets/src/explore/controls.jsx | 2 +- .../src/modules/CategoricalColorNamespace.js | 26 +++++++++++++------ .../src/modules/CategoricalColorScale.js | 2 +- .../assets/src/modules/ColorSchemeManager.js | 26 ++++++++++++------- superset/assets/src/modules/colors.js | 4 +-- 7 files changed, 42 insertions(+), 25 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx index 2c62f19ba7793..5177f27dddaed 100644 --- a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx @@ -9,10 +9,9 @@ import ColorSchemeControl from '../../../../src/explore/components/controls/ColorSchemeControl'; import ColorSchemeManager from '../../../../src/modules/ColorSchemeManager'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAll(); - const defaultProps = { - options: Object.keys(ALL_COLOR_SCHEMES).map(s => ([s, s])), + options: Object.keys(ColorSchemeManager.getInstance() + .getAllSchemes()).map(s => ([s, s])), }; describe('ColorSchemeControl', () => { diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index 0aad8ec57fc84..67c58144cc809 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -3,7 +3,7 @@ import { expect } from 'chai'; import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAll(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAllSchemes(); describe('colors', () => { it('default to bnbColors', () => { diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index a27d3af499fbb..8e8bd8720654f 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -53,7 +53,7 @@ import { t } from '../locales'; import ColorSchemeManager from '../modules/ColorSchemeManager'; import sequentialSchemes from '../modules/colorSchemes/sequential'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAll(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAllSchemes(); const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format'; diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js index fa5ddc5394238..f40f5ca7f08c1 100644 --- a/superset/assets/src/modules/CategoricalColorNamespace.js +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -2,19 +2,20 @@ import CategoricalColorScale from './CategoricalColorScale'; import ColorSchemeManager from './ColorSchemeManager'; class CategoricalColorNamespace { - constructor() { + constructor(name) { + this.name = name; this.scales = {}; this.forcedItems = {}; } getScale(schemeName) { - const name = schemeName || ColorSchemeManager.getDefaultSchemeName(); + const name = schemeName || ColorSchemeManager.getInstance().getDefaultSchemeName(); const scale = this.scales[name]; if (scale) { return scale; } const newScale = new CategoricalColorScale( - ColorSchemeManager.getScheme(name), + ColorSchemeManager.getInstance().getScheme(name), this.forcedItems, ); this.scales[name] = newScale; @@ -37,16 +38,25 @@ class CategoricalColorNamespace { const namespaces = {}; const DEFAULT_NAMESPACE = 'DEFAULT'; -function getNamespace(namespace = DEFAULT_NAMESPACE) { - const instance = namespaces[namespace]; +export function getNamespace(name = DEFAULT_NAMESPACE) { + const instance = namespaces[name]; if (instance) { return instance; } - const newInstance = new CategoricalColorNamespace(); - namespaces[namespace] = newInstance; + const newInstance = new CategoricalColorNamespace(name); + namespaces[name] = newInstance; return newInstance; } -CategoricalColorNamespace.getNamespace = getNamespace; +export function getColor(value, scheme, namespace) { + return getNamespace(namespace) + .getScale(scheme) + .getColor(value); +} + +export function getScale(scheme, namespace) { + return getNamespace(namespace) + .getScale(scheme); +} export default CategoricalColorNamespace; diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index ea11e3f9f310d..f431c7b830345 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -21,7 +21,7 @@ export default class CategoricalColorScale { getColor(value) { const cleanedValue = cleanValue(value); - const sharedColor = this.sharedForcedItems && this.shareForcedItems[cleanedValue]; + const sharedColor = this.sharedForcedItems && this.sharedForcedItems[cleanedValue]; if (sharedColor) { return sharedColor; } diff --git a/superset/assets/src/modules/ColorSchemeManager.js b/superset/assets/src/modules/ColorSchemeManager.js index 2e5d029f06b8d..c9ec16b930f07 100644 --- a/superset/assets/src/modules/ColorSchemeManager.js +++ b/superset/assets/src/modules/ColorSchemeManager.js @@ -8,11 +8,16 @@ class ColorSchemeManager { this.defaultSchemeName = undefined; } - get(schemeName) { + clearSchemes() { + this.schemes = {}; + return this; + } + + getScheme(schemeName) { return this.schemes[schemeName || this.defaultSchemeName]; } - getAll() { + getAllSchemes() { return this.schemes; } @@ -22,23 +27,26 @@ class ColorSchemeManager { setDefaultSchemeName(schemeName) { this.defaultSchemeName = schemeName; + return this; } - register(schemeName, colors) { + registerScheme(schemeName, colors) { this.schemes[schemeName] = colors; // If there is no default, set as default if (!this.defaultSchemeName) { this.defaultSchemeName = schemeName; } + return this; } - registerAll(multipleSchemes) { + registerSchemes(multipleSchemes) { Object.assign(this.schemes, multipleSchemes); // If there is no default, set the first scheme as default const keys = Object.keys(multipleSchemes); if (!this.defaultSchemeName && keys.length > 0) { this.defaultSchemeName = keys[0]; } + return this; } } @@ -55,10 +63,10 @@ ColorSchemeManager.getInstance = getInstance; // These registration code eventually should go into per-app configuration // when we are completely in the plug-in system. -const manager = ColorSchemeManager.getInstance(); -manager.register('bnbColors', airbnb.bnbColors); -manager.registerAll(categoricalSchemes); -manager.register('lyftColors', lyft.lyftColors); -manager.setDefaultSchemeName('bnbColors'); +ColorSchemeManager.getInstance() + .registerScheme('bnbColors', airbnb.bnbColors) + .registerSchemes(categoricalSchemes) + .registerScheme('lyftColors', lyft.lyftColors) + .setDefaultSchemeName('bnbColors'); export default ColorSchemeManager; diff --git a/superset/assets/src/modules/colors.js b/superset/assets/src/modules/colors.js index 0a6126dde018c..43025dc85bcd5 100644 --- a/superset/assets/src/modules/colors.js +++ b/superset/assets/src/modules/colors.js @@ -1,5 +1,5 @@ import d3 from 'd3'; -import CategoricalColorNamespace from './CategoricalColorNamespace'; +import { getScale } from './CategoricalColorNamespace'; import sequentialSchemes from './colorSchemes/sequential'; import airbnb from './colorSchemes/airbnb'; import lyft from './colorSchemes/lyft'; @@ -31,7 +31,7 @@ export function hexToRGB(hex, alpha = 255) { forcibly associate to a label. */ export function getColorFromScheme(value, schemeName, forcedColor) { - const scale = CategoricalColorNamespace.getNamespace().getScale(schemeName); + const scale = getScale(schemeName); if (forcedColor) { scale.setColor(value, forcedColor); return forcedColor; From cd48bd6aeda1a9fed2d8344cdcd21d7e8a8f36da Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 17:08:31 -0700 Subject: [PATCH 16/33] Update the visualizations --- superset/assets/src/dashboard/reducers/getInitialState.js | 4 ++-- superset/assets/src/visualizations/chord.jsx | 4 ++-- .../visualizations/deckgl/CategoricalDeckGLContainer.jsx | 6 +++--- superset/assets/src/visualizations/partition.js | 4 ++-- superset/assets/src/visualizations/rose.js | 4 ++-- superset/assets/src/visualizations/sankey.js | 4 ++-- superset/assets/src/visualizations/sunburst.js | 4 ++-- superset/assets/src/visualizations/treemap.js | 4 ++-- superset/assets/src/visualizations/wordcloud/WordCloud.js | 4 ++-- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/superset/assets/src/dashboard/reducers/getInitialState.js b/superset/assets/src/dashboard/reducers/getInitialState.js index 6481093548b91..2a4a5e26fa310 100644 --- a/superset/assets/src/dashboard/reducers/getInitialState.js +++ b/superset/assets/src/dashboard/reducers/getInitialState.js @@ -18,7 +18,7 @@ import { CHART_TYPE, ROW_TYPE, } from '../util/componentTypes'; -import CategoricalColorManager from '../../modules/CategoricalColorManager'; +import { getScale } from '../../modules/CategoricalColorNamespace'; export default function(bootstrapData) { const { user_id, datasources, common, editMode } = bootstrapData; @@ -41,7 +41,7 @@ export default function(bootstrapData) { if (dashboard.metadata && dashboard.metadata.label_colors) { const colorMap = dashboard.metadata.label_colors; Object.keys(colorMap).forEach(label => { - CategoricalColorManager.getScale(null).setColor(label, colorMap[label]); + getScale().setColor(label, colorMap[label]); }); } diff --git a/superset/assets/src/visualizations/chord.jsx b/superset/assets/src/visualizations/chord.jsx index 8f307c97d678e..672a31e549514 100644 --- a/superset/assets/src/visualizations/chord.jsx +++ b/superset/assets/src/visualizations/chord.jsx @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign */ import d3 from 'd3'; import PropTypes from 'prop-types'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; +import { getScale } from '../modules/CategoricalColorNamespace'; import './chord.css'; const propTypes = { @@ -31,7 +31,7 @@ function chordVis(element, props) { const div = d3.select(element); const { nodes, matrix } = data; const f = d3.format(numberFormat); - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); const outerRadius = Math.min(width, height) / 2 - 10; const innerRadius = outerRadius - 24; diff --git a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx index a242548a451de..0976ec00dfce1 100644 --- a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx +++ b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx @@ -6,15 +6,15 @@ import PropTypes from 'prop-types'; import AnimatableDeckGLContainer from './AnimatableDeckGLContainer'; import Legend from '../Legend'; +import { getScale } from '../../modules/CategoricalColorNamespace'; import { hexToRGB } from '../../modules/colors'; import { getPlaySliderParams } from '../../modules/time'; import sandboxedEval from '../../modules/sandbox'; -import CategoricalColorManager from '../../modules/CategoricalColorManager'; function getCategories(fd, data) { const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 }; const fixedColor = [c.r, c.g, c.b, 255 * c.a]; - const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); + const colorFn = getScale(fd.color_scheme).toFunction(); const categories = {}; data.forEach((d) => { if (d.cat_color != null && !categories.hasOwnProperty(d.cat_color)) { @@ -100,7 +100,7 @@ export default class CategoricalDeckGLContainer extends React.PureComponent { } addColor(data, fd) { const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 }; - const colorFn = CategoricalColorManager.getScale(fd.color_scheme).toFunction(); + const colorFn = getScale(fd.color_scheme).toFunction(); return data.map((d) => { let color; if (fd.dimension) { diff --git a/superset/assets/src/visualizations/partition.js b/superset/assets/src/visualizations/partition.js index 1954e9bda0ca3..e70a1eece7823 100644 --- a/superset/assets/src/visualizations/partition.js +++ b/superset/assets/src/visualizations/partition.js @@ -2,7 +2,7 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import { hierarchy } from 'd3-hierarchy'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; +import { getScale } from '../modules/CategoricalColorNamespace'; import { d3TimeFormatPreset } from '../modules/utils'; import './partition.css'; @@ -97,7 +97,7 @@ function Icicle(element, props) { const hasTime = ['adv_anal', 'time_series'].indexOf(chartType) >= 0; const format = d3.format(numberFormat); const timeFormat = d3TimeFormatPreset(dateTimeFormat); - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); div.selectAll('*').remove(); const tooltip = div diff --git a/superset/assets/src/visualizations/rose.js b/superset/assets/src/visualizations/rose.js index ca6ec43953812..62df302020d0d 100644 --- a/superset/assets/src/visualizations/rose.js +++ b/superset/assets/src/visualizations/rose.js @@ -2,8 +2,8 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import nv from 'nvd3'; +import { getScale } from '../modules/CategoricalColorNamespace'; import { d3TimeFormatPreset } from '../modules/utils'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; import './rose.css'; const propTypes = { @@ -62,7 +62,7 @@ function Rose(element, props) { const numGroups = datum[times[0]].length; const format = d3.format(numberFormat); const timeFormat = d3TimeFormatPreset(dateTimeFormat); - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); d3.select('.nvtooltip').remove(); div.selectAll('*').remove(); diff --git a/superset/assets/src/visualizations/sankey.js b/superset/assets/src/visualizations/sankey.js index fde7b2b9978bd..2509a50db6fc3 100644 --- a/superset/assets/src/visualizations/sankey.js +++ b/superset/assets/src/visualizations/sankey.js @@ -2,7 +2,7 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import { sankey as d3Sankey } from 'd3-sankey'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; +import { getScale } from '../modules/CategoricalColorNamespace'; import './sankey.css'; const propTypes = { @@ -49,7 +49,7 @@ function Sankey(element, props) { .attr('class', 'sankey-tooltip') .style('opacity', 0); - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); const sankey = d3Sankey() .nodeWidth(15) diff --git a/superset/assets/src/visualizations/sunburst.js b/superset/assets/src/visualizations/sunburst.js index 43a4e8d4b5d00..7a8717311c073 100644 --- a/superset/assets/src/visualizations/sunburst.js +++ b/superset/assets/src/visualizations/sunburst.js @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign */ import d3 from 'd3'; import PropTypes from 'prop-types'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; +import { getScale } from '../modules/CategoricalColorNamespace'; import { wrapSvgText } from '../modules/utils'; import './sunburst.css'; @@ -68,7 +68,7 @@ function Sunburst(element, props) { let arcs; let gMiddleText; // dom handles - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); // Helper + path gen functions const partition = d3.layout.partition() diff --git a/superset/assets/src/visualizations/treemap.js b/superset/assets/src/visualizations/treemap.js index d69050f0bfac6..8ffd14055b5bf 100644 --- a/superset/assets/src/visualizations/treemap.js +++ b/superset/assets/src/visualizations/treemap.js @@ -1,7 +1,7 @@ /* eslint-disable no-shadow, no-param-reassign */ import d3 from 'd3'; import PropTypes from 'prop-types'; -import CategoricalColorManager from '../modules/CategoricalColorManager'; +import { getScale } from '../modules/CategoricalColorNamespace'; import './treemap.css'; // Declare PropTypes for recursive data structures @@ -63,7 +63,7 @@ function treemap(element, props) { } = props; const div = d3.select(element); const formatNumber = d3.format(numberFormat); - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); function draw(data, eltWidth, eltHeight) { const navBarHeight = 36; diff --git a/superset/assets/src/visualizations/wordcloud/WordCloud.js b/superset/assets/src/visualizations/wordcloud/WordCloud.js index 019bad6b68ec7..7458f7d336dc3 100644 --- a/superset/assets/src/visualizations/wordcloud/WordCloud.js +++ b/superset/assets/src/visualizations/wordcloud/WordCloud.js @@ -1,7 +1,7 @@ import d3 from 'd3'; import PropTypes from 'prop-types'; import cloudLayout from 'd3-cloud'; -import CategoricalColorManager from '../../modules/CategoricalColorManager'; +import { getScale } from '../../modules/CategoricalColorNamespace'; const ROTATION = { square: () => Math.floor((Math.random() * 2)) * 90, @@ -50,7 +50,7 @@ function wordCloud(element, props) { .fontWeight('bold') .fontSize(d => scale(d.size)); - const colorFn = CategoricalColorManager.getScale(colorScheme).toFunction(); + const colorFn = getScale(colorScheme).toFunction(); function draw(words) { chart.selectAll('*').remove(); From 2915afc78f3e5d153d7569ece4ce14060f878422 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 17:31:15 -0700 Subject: [PATCH 17/33] update usage with static functions --- .../explore/components/ColorScheme_spec.jsx | 3 +-- .../spec/javascripts/modules/colors_spec.jsx | 2 +- .../components/controls/AnnotationLayer.jsx | 2 +- superset/assets/src/explore/controls.jsx | 2 +- .../src/modules/CategoricalColorNamespace.js | 4 ++-- .../assets/src/modules/ColorSchemeManager.js | 19 +++++++++++++++---- 6 files changed, 21 insertions(+), 11 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx index 5177f27dddaed..73431a09291de 100644 --- a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx @@ -10,8 +10,7 @@ import ColorSchemeControl from import ColorSchemeManager from '../../../../src/modules/ColorSchemeManager'; const defaultProps = { - options: Object.keys(ColorSchemeManager.getInstance() - .getAllSchemes()).map(s => ([s, s])), + options: Object.keys(ColorSchemeManager.getAllSchemes()).map(s => ([s, s])), }; describe('ColorSchemeControl', () => { diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index 67c58144cc809..db2b7aa4f6749 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -3,7 +3,7 @@ import { expect } from 'chai'; import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAllSchemes(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getAllSchemes(); describe('colors', () => { it('default to bnbColors', () => { diff --git a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx index 1924936fff074..86f220cb78a2d 100644 --- a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx +++ b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx @@ -478,7 +478,7 @@ export default class AnnotationLayer extends React.PureComponent { renderDisplayConfiguration() { const { color, opacity, style, width, showMarkers, hideLine, annotationType } = this.state; - const colorScheme = [...ColorSchemeManager.getInstance().get(this.props.colorScheme)]; + const colorScheme = [...ColorSchemeManager.getScheme(this.props.colorScheme)]; if (color && color !== AUTOMATIC_COLOR && !colorScheme.find(x => x.toLowerCase() === color.toLowerCase())) { colorScheme.push(color); diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index 8e8bd8720654f..b2a411d84d328 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -53,7 +53,7 @@ import { t } from '../locales'; import ColorSchemeManager from '../modules/ColorSchemeManager'; import sequentialSchemes from '../modules/colorSchemes/sequential'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getInstance().getAllSchemes(); +const ALL_COLOR_SCHEMES = ColorSchemeManager.getAllSchemes(); const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format'; diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js index f40f5ca7f08c1..91dc494c21eed 100644 --- a/superset/assets/src/modules/CategoricalColorNamespace.js +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -9,13 +9,13 @@ class CategoricalColorNamespace { } getScale(schemeName) { - const name = schemeName || ColorSchemeManager.getInstance().getDefaultSchemeName(); + const name = schemeName || ColorSchemeManager.getDefaultSchemeName(); const scale = this.scales[name]; if (scale) { return scale; } const newScale = new CategoricalColorScale( - ColorSchemeManager.getInstance().getScheme(name), + ColorSchemeManager.getScheme(name), this.forcedItems, ); this.scales[name] = newScale; diff --git a/superset/assets/src/modules/ColorSchemeManager.js b/superset/assets/src/modules/ColorSchemeManager.js index c9ec16b930f07..5cb356933ac68 100644 --- a/superset/assets/src/modules/ColorSchemeManager.js +++ b/superset/assets/src/modules/ColorSchemeManager.js @@ -59,14 +59,25 @@ function getInstance() { return singleton; } +const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.prototype) + .filter(fn => fn !== 'constructor') + .reduce((all, fn) => { + const functions = all; + functions[fn] = function (...args) { + return getInstance()[fn](...args); + }; + return functions; + }, {}); + ColorSchemeManager.getInstance = getInstance; +Object.assign(ColorSchemeManager, staticFunctions); + +export default ColorSchemeManager; // These registration code eventually should go into per-app configuration -// when we are completely in the plug-in system. -ColorSchemeManager.getInstance() +// when we migrate to the plug-in system. +ColorSchemeManager .registerScheme('bnbColors', airbnb.bnbColors) .registerSchemes(categoricalSchemes) .registerScheme('lyftColors', lyft.lyftColors) .setDefaultSchemeName('bnbColors'); - -export default ColorSchemeManager; From d60d4eee5f880e8cd3d7a8b68b8794f71657ddb7 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 17:36:23 -0700 Subject: [PATCH 18/33] update unit test --- superset/assets/spec/javascripts/modules/colors_spec.jsx | 4 ++-- superset/assets/src/modules/CategoricalColorScale.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index db2b7aa4f6749..390dadf3d2f03 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -24,11 +24,11 @@ describe('colors', () => { it('getColorFromScheme forcing colors persists through calls', () => { expect(getColorFromScheme('boys', 'bnbColors', 'blue')).to.equal('blue'); expect(getColorFromScheme('boys', 'bnbColors')).to.equal('blue'); - expect(getColorFromScheme('boys', 'googleCategory20c')).to.equal('blue'); + expect(getColorFromScheme('boys', 'googleCategory20c')).to.not.equal('blue'); expect(getColorFromScheme('girls', 'bnbColors', 'pink')).to.equal('pink'); expect(getColorFromScheme('girls', 'bnbColors')).to.equal('pink'); - expect(getColorFromScheme('girls', 'googleCategory20c')).to.equal('pink'); + expect(getColorFromScheme('girls', 'googleCategory20c')).to.not.equal('pink'); }); it('getColorFromScheme is not case sensitive', () => { const c1 = getColorFromScheme('girls', 'bnbColors'); diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index f431c7b830345..01dc61abf7093 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -1,8 +1,8 @@ import { TIME_SHIFT_PATTERN } from '../utils/common'; -function cleanValue(value) { - // for superset series that should have the same color - return String(value).trim() +export function cleanValue(value) { + // for superset series that should have the same color + return String(value).trim() .toLowerCase() .split(', ') .filter(k => !TIME_SHIFT_PATTERN.test(k)) From f45606549de2b08e9ec53ed5b994010c878e8a06 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 18:10:19 -0700 Subject: [PATCH 19/33] add unit test --- .../modules/CategoricalColorScale_spec.js | 69 +++++++++++++++++++ .../src/modules/CategoricalColorScale.js | 12 ++-- 2 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js new file mode 100644 index 0000000000000..96762ead313f3 --- /dev/null +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -0,0 +1,69 @@ +/* eslint-disable no-unused-expressions */ +import { it, describe } from 'mocha'; +import { expect } from 'chai'; +import CategoricalColorScale from '../../../src/modules/CategoricalColorScale'; + +describe('CategoricalColorScale', () => { + it('exists', () => { + expect(CategoricalColorScale).to.exist; + }); + + describe('new CategoricalColorScale(colors, sharedForcedColors)', () => { + it('can create new scale when sharedForcedItem is not given', () => { + const scale = new CategoricalColorScale(['blue', 'red', 'green']); + expect(scale).to.be.instanceOf(CategoricalColorScale); + }); + it('can create new scale when sharedForcedColors is given', () => { + const sharedForcedColors = {}; + const scale = new CategoricalColorScale(['blue', 'red', 'green'], sharedForcedColors); + expect(scale).to.be.instanceOf(CategoricalColorScale); + expect(scale.sharedForcedColors).to.equal(sharedForcedColors); + }); + }); + 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); + }); + }); + 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 sharedForcedColors', () => { + 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'); + }); + }); + 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')); + }); + }); +}); diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index 01dc61abf7093..a4466b597e60b 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -10,10 +10,10 @@ export function cleanValue(value) { } export default class CategoricalColorScale { - constructor(colors, sharedForcedItems) { + constructor(colors, sharedForcedColors) { this.colors = colors; - this.sharedForcedItems = sharedForcedItems; - this.forcedItems = {}; + this.sharedForcedColors = sharedForcedColors; + this.forcedColors = {}; this.seen = {}; this.fn = value => this.getColor(value); } @@ -21,12 +21,12 @@ export default class CategoricalColorScale { getColor(value) { const cleanedValue = cleanValue(value); - const sharedColor = this.sharedForcedItems && this.sharedForcedItems[cleanedValue]; + const sharedColor = this.sharedForcedColors && this.sharedForcedColors[cleanedValue]; if (sharedColor) { return sharedColor; } - const forcedColor = this.forcedItems[cleanedValue]; + const forcedColor = this.forcedColors[cleanedValue]; if (forcedColor) { return forcedColor; } @@ -48,7 +48,7 @@ export default class CategoricalColorScale { * @param {*} forcedColor forcedColor */ setColor(value, forcedColor) { - this.forcedItems[value] = forcedColor; + this.forcedColors[value] = forcedColor; return this; } From 75318f423a7a9e3bb8b3eaf8110b667e8328c5e0 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Fri, 7 Sep 2018 18:24:16 -0700 Subject: [PATCH 20/33] rename default namespace --- superset/assets/src/modules/CategoricalColorNamespace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js index 91dc494c21eed..bd108a9f94517 100644 --- a/superset/assets/src/modules/CategoricalColorNamespace.js +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -36,7 +36,7 @@ class CategoricalColorNamespace { } const namespaces = {}; -const DEFAULT_NAMESPACE = 'DEFAULT'; +const DEFAULT_NAMESPACE = 'GLOBAL'; export function getNamespace(name = DEFAULT_NAMESPACE) { const instance = namespaces[name]; From 9d457a0de2b9982f5b6d11af21965551b2f4300d Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 15:52:06 -0700 Subject: [PATCH 21/33] add unit test for color namespace --- superset/assets/package.json | 1 + .../modules/CategoricalColorNameSpace_spec.js | 9 +++++++++ .../javascripts/modules/CategoricalColorScale_spec.js | 3 +-- 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js diff --git a/superset/assets/package.json b/superset/assets/package.json index 667b4cb321c3f..11b2b77f4b81c 100644 --- a/superset/assets/package.json +++ b/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.*'", diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js new file mode 100644 index 0000000000000..5388dc88b90f0 --- /dev/null +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -0,0 +1,9 @@ +import { it, describe } from 'mocha'; +import { expect } from 'chai'; +import CategoricalColorNamespace from '../../../src/modules/CategoricalColorNamespace'; + +describe.only('CategoricalColorNamespace', () => { + it('exists', () => { + expect(CategoricalColorNamespace !== undefined).to.equal(true); + }); +}); diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js index 96762ead313f3..58d5e2c3f314a 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -1,11 +1,10 @@ -/* eslint-disable no-unused-expressions */ import { it, describe } from 'mocha'; import { expect } from 'chai'; import CategoricalColorScale from '../../../src/modules/CategoricalColorScale'; describe('CategoricalColorScale', () => { it('exists', () => { - expect(CategoricalColorScale).to.exist; + expect(CategoricalColorScale !== undefined).to.equal(true); }); describe('new CategoricalColorScale(colors, sharedForcedColors)', () => { From 00d8090f5d825b576a8598f6c365dcb61afbc94c Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 16:46:09 -0700 Subject: [PATCH 22/33] add unit test for namespace --- .../modules/CategoricalColorNameSpace_spec.js | 123 +++++++++++++++++- .../src/modules/CategoricalColorNamespace.js | 6 +- 2 files changed, 124 insertions(+), 5 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index 5388dc88b90f0..50d4ca99f98db 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -1,9 +1,124 @@ -import { it, describe } from 'mocha'; +import { it, describe, before } from 'mocha'; import { expect } from 'chai'; -import CategoricalColorNamespace from '../../../src/modules/CategoricalColorNamespace'; +import CategoricalColorNamespace, { + getNamespace, + getScale, + getColor, +} from '../../../src/modules/CategoricalColorNamespace'; +import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; describe.only('CategoricalColorNamespace', () => { - it('exists', () => { - expect(CategoricalColorNamespace !== undefined).to.equal(true); + before(() => { + ColorSchemeManager.registerScheme('testColors', ['red', 'green', 'blue']); + ColorSchemeManager.registerScheme('testColors2', ['red', 'green', 'blue']); + }); + it('The class constructor cannot be accessed directly', () => { + expect(CategoricalColorNamespace).to.not.be.a('Function'); + }); + describe('static function getNamespace()', () => { + it('returns default namespace if name is not specified', () => { + const namespace = getNamespace(); + expect(namespace !== undefined).to.equal(true); + expect(namespace.name).to.equal('GLOBAL'); + }); + 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'); + }); + }); + describe('static function 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 function 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); + }); }); }); diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js index bd108a9f94517..9e46bb345231f 100644 --- a/superset/assets/src/modules/CategoricalColorNamespace.js +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -59,4 +59,8 @@ export function getScale(scheme, namespace) { .getScale(scheme); } -export default CategoricalColorNamespace; +export default { + getNamespace, + getScale, + getColor, +}; From 8a91c4d888243533971779c0798132db4ce5db78 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 16:58:28 -0700 Subject: [PATCH 23/33] start unit test for colorschememanager --- .../modules/CategoricalColorNameSpace_spec.js | 8 ++++---- .../javascripts/modules/ColorSchemeManager_spec.js | 12 ++++++++++++ superset/assets/src/modules/ColorSchemeManager.js | 11 ++++------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index 50d4ca99f98db..041e4f1c93a6b 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -5,12 +5,12 @@ import CategoricalColorNamespace, { getScale, getColor, } from '../../../src/modules/CategoricalColorNamespace'; -import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; +import { registerScheme } from '../../../src/modules/ColorSchemeManager'; -describe.only('CategoricalColorNamespace', () => { +describe('CategoricalColorNamespace', () => { before(() => { - ColorSchemeManager.registerScheme('testColors', ['red', 'green', 'blue']); - ColorSchemeManager.registerScheme('testColors2', ['red', 'green', 'blue']); + 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'); diff --git a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js new file mode 100644 index 0000000000000..d3ce57b1bb08e --- /dev/null +++ b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js @@ -0,0 +1,12 @@ +import { it, describe } from 'mocha'; +import { expect } from 'chai'; +import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; + +describe.only('ColorSchemeManager', () => { + it('The class constructor cannot be accessed directly', () => { + expect(ColorSchemeManager).to.not.be.a('Function'); + }); + it('is a singleton class that returns an instance via static function getInstance()', () => { + + }); +}); diff --git a/superset/assets/src/modules/ColorSchemeManager.js b/superset/assets/src/modules/ColorSchemeManager.js index 5cb356933ac68..b77bf9d8f03ee 100644 --- a/superset/assets/src/modules/ColorSchemeManager.js +++ b/superset/assets/src/modules/ColorSchemeManager.js @@ -59,7 +59,7 @@ function getInstance() { return singleton; } -const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.prototype) +export const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.prototype) .filter(fn => fn !== 'constructor') .reduce((all, fn) => { const functions = all; @@ -67,16 +67,13 @@ const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.prototype) return getInstance()[fn](...args); }; return functions; - }, {}); + }, { getInstance }); -ColorSchemeManager.getInstance = getInstance; -Object.assign(ColorSchemeManager, staticFunctions); - -export default ColorSchemeManager; +export default staticFunctions; // These registration code eventually should go into per-app configuration // when we migrate to the plug-in system. -ColorSchemeManager +getInstance() .registerScheme('bnbColors', airbnb.bnbColors) .registerSchemes(categoricalSchemes) .registerScheme('lyftColors', lyft.lyftColors) From 47c5f20d2b45f34c01dbb439531598952966ca41 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 17:41:13 -0700 Subject: [PATCH 24/33] add unit tests for color scheme manager --- .../explore/components/ColorScheme_spec.jsx | 4 +- .../modules/CategoricalColorNameSpace_spec.js | 6 +- .../modules/ColorSchemeManager_spec.js | 138 +++++++++++++++++- .../spec/javascripts/modules/colors_spec.jsx | 4 +- .../components/controls/AnnotationLayer.jsx | 4 +- superset/assets/src/explore/controls.jsx | 4 +- .../src/modules/CategoricalColorNamespace.js | 12 +- .../assets/src/modules/ColorSchemeManager.js | 30 +++- 8 files changed, 172 insertions(+), 30 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx b/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx index 73431a09291de..10e582b31ecbd 100644 --- a/superset/assets/spec/javascripts/explore/components/ColorScheme_spec.jsx +++ b/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 ColorSchemeManager from '../../../../src/modules/ColorSchemeManager'; +import { getAllSchemes } from '../../../../src/modules/ColorSchemeManager'; const defaultProps = { - options: Object.keys(ColorSchemeManager.getAllSchemes()).map(s => ([s, s])), + options: Object.keys(getAllSchemes()).map(s => ([s, s])), }; describe('ColorSchemeControl', () => { diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index 041e4f1c93a6b..c7998f2e63c5a 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -15,7 +15,7 @@ describe('CategoricalColorNamespace', () => { it('The class constructor cannot be accessed directly', () => { expect(CategoricalColorNamespace).to.not.be.a('Function'); }); - describe('static function getNamespace()', () => { + describe('static getNamespace()', () => { it('returns default namespace if name is not specified', () => { const namespace = getNamespace(); expect(namespace !== undefined).to.equal(true); @@ -78,7 +78,7 @@ describe('CategoricalColorNamespace', () => { expect(scale2.getColor('dog')).to.not.equal('black'); }); }); - describe('static function getScale()', () => { + describe('static getScale()', () => { it('getScale() returns a CategoricalColorScale with default scheme in default namespace', () => { const scale = getScale(); expect(scale).to.not.equal(undefined); @@ -98,7 +98,7 @@ describe('CategoricalColorNamespace', () => { expect(scale).to.equal(scale2); }); }); - describe('static function getColor()', () => { + describe('static getColor()', () => { it('getColor(value) returns a color from default scheme in default namespace', () => { const value = 'dog'; const color = getColor(value); diff --git a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js index d3ce57b1bb08e..baa7026fefc3e 100644 --- a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js +++ b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js @@ -1,12 +1,142 @@ -import { it, describe } from 'mocha'; +import { it, describe, beforeEach } from 'mocha'; import { expect } from 'chai'; -import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; +import ColorSchemeManager, { + getInstance, + getScheme, + getAllSchemes, + getDefaultSchemeName, + setDefaultSchemeName, + registerScheme, + registerMultipleSchemes, +} from '../../../src/modules/ColorSchemeManager'; describe.only('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'); }); - it('is a singleton class that returns an instance via static function getInstance()', () => { - + 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']); + }); + }); + }); diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index 390dadf3d2f03..d7fb9bda3806f 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -1,9 +1,9 @@ import { it, describe } from 'mocha'; import { expect } from 'chai'; import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; -import ColorSchemeManager from '../../../src/modules/ColorSchemeManager'; +import { getAllSchemes } from '../../../src/modules/ColorSchemeManager'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getAllSchemes(); +const ALL_COLOR_SCHEMES = getAllSchemes(); describe('colors', () => { it('default to bnbColors', () => { diff --git a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx index 86f220cb78a2d..3238f4f3c9ce1 100644 --- a/superset/assets/src/explore/components/controls/AnnotationLayer.jsx +++ b/superset/assets/src/explore/components/controls/AnnotationLayer.jsx @@ -26,7 +26,7 @@ import { nonEmpty } from '../../validators'; import vizTypes from '../../visTypes'; import { t } from '../../../locales'; -import ColorSchemeManager from '../../../modules/ColorSchemeManager'; +import { getScheme } from '../../../modules/ColorSchemeManager'; const AUTOMATIC_COLOR = ''; @@ -478,7 +478,7 @@ export default class AnnotationLayer extends React.PureComponent { renderDisplayConfiguration() { const { color, opacity, style, width, showMarkers, hideLine, annotationType } = this.state; - const colorScheme = [...ColorSchemeManager.getScheme(this.props.colorScheme)]; + const colorScheme = [...getScheme(this.props.colorScheme)]; if (color && color !== AUTOMATIC_COLOR && !colorScheme.find(x => x.toLowerCase() === color.toLowerCase())) { colorScheme.push(color); diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index b2a411d84d328..2a04c7354cdd3 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -50,10 +50,10 @@ import { defaultViewport } from '../modules/geo'; import ColumnOption from '../components/ColumnOption'; import OptionDescription from '../components/OptionDescription'; import { t } from '../locales'; -import ColorSchemeManager from '../modules/ColorSchemeManager'; +import { getAllSchemes } from '../modules/ColorSchemeManager'; import sequentialSchemes from '../modules/colorSchemes/sequential'; -const ALL_COLOR_SCHEMES = ColorSchemeManager.getAllSchemes(); +const ALL_COLOR_SCHEMES = getAllSchemes(); const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format'; diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js index 9e46bb345231f..bf9f81f7792d5 100644 --- a/superset/assets/src/modules/CategoricalColorNamespace.js +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -1,5 +1,5 @@ import CategoricalColorScale from './CategoricalColorScale'; -import ColorSchemeManager from './ColorSchemeManager'; +import { getScheme, getDefaultSchemeName } from './ColorSchemeManager'; class CategoricalColorNamespace { constructor(name) { @@ -9,13 +9,13 @@ class CategoricalColorNamespace { } getScale(schemeName) { - const name = schemeName || ColorSchemeManager.getDefaultSchemeName(); + const name = schemeName || getDefaultSchemeName(); const scale = this.scales[name]; if (scale) { return scale; } const newScale = new CategoricalColorScale( - ColorSchemeManager.getScheme(name), + getScheme(name), this.forcedItems, ); this.scales[name] = newScale; @@ -58,9 +58,3 @@ export function getScale(scheme, namespace) { return getNamespace(namespace) .getScale(scheme); } - -export default { - getNamespace, - getScale, - getColor, -}; diff --git a/superset/assets/src/modules/ColorSchemeManager.js b/superset/assets/src/modules/ColorSchemeManager.js index b77bf9d8f03ee..9654aca6ce389 100644 --- a/superset/assets/src/modules/ColorSchemeManager.js +++ b/superset/assets/src/modules/ColorSchemeManager.js @@ -8,7 +8,7 @@ class ColorSchemeManager { this.defaultSchemeName = undefined; } - clearSchemes() { + clearScheme() { this.schemes = {}; return this; } @@ -39,7 +39,7 @@ class ColorSchemeManager { return this; } - registerSchemes(multipleSchemes) { + registerMultipleSchemes(multipleSchemes) { Object.assign(this.schemes, multipleSchemes); // If there is no default, set the first scheme as default const keys = Object.keys(multipleSchemes); @@ -52,14 +52,14 @@ class ColorSchemeManager { let singleton; -function getInstance() { +export function getInstance() { if (!singleton) { singleton = new ColorSchemeManager(); } return singleton; } -export const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.prototype) +const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.prototype) .filter(fn => fn !== 'constructor') .reduce((all, fn) => { const functions = all; @@ -69,12 +69,30 @@ export const staticFunctions = Object.getOwnPropertyNames(ColorSchemeManager.pro return functions; }, { getInstance }); -export default staticFunctions; +const { + clearScheme, + getScheme, + getAllSchemes, + getDefaultSchemeName, + setDefaultSchemeName, + registerScheme, + registerMultipleSchemes, +} = staticFunctions; + +export { + clearScheme, + getScheme, + getAllSchemes, + getDefaultSchemeName, + setDefaultSchemeName, + registerScheme, + registerMultipleSchemes, +}; // These registration code eventually should go into per-app configuration // when we migrate to the plug-in system. getInstance() .registerScheme('bnbColors', airbnb.bnbColors) - .registerSchemes(categoricalSchemes) + .registerMultipleSchemes(categoricalSchemes) .registerScheme('lyftColors', lyft.lyftColors) .setDefaultSchemeName('bnbColors'); From e2dba45ee95457e3ec58024cb821907e54f1c754 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 17:44:20 -0700 Subject: [PATCH 25/33] check returns for chaining --- .../javascripts/modules/CategoricalColorNameSpace_spec.js | 7 ++++++- .../spec/javascripts/modules/ColorSchemeManager_spec.js | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index c7998f2e63c5a..2d33566d72b45 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -7,7 +7,7 @@ import CategoricalColorNamespace, { } from '../../../src/modules/CategoricalColorNamespace'; import { registerScheme } from '../../../src/modules/ColorSchemeManager'; -describe('CategoricalColorNamespace', () => { +describe.only('CategoricalColorNamespace', () => { before(() => { registerScheme('testColors', ['red', 'green', 'blue']); registerScheme('testColors2', ['red', 'green', 'blue']); @@ -77,6 +77,11 @@ describe('CategoricalColorNamespace', () => { 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', () => { diff --git a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js index baa7026fefc3e..ffb950b559ebb 100644 --- a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js +++ b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js @@ -138,5 +138,4 @@ describe.only('ColorSchemeManager', () => { expect(scheme2).to.deep.equal(['brown', 'purple']); }); }); - }); From 500f44123757419fc8c95b716be045db134ccca7 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 17:47:10 -0700 Subject: [PATCH 26/33] complete unit test for the new classes --- .../javascripts/modules/CategoricalColorNameSpace_spec.js | 2 +- .../spec/javascripts/modules/CategoricalColorScale_spec.js | 5 +++++ .../spec/javascripts/modules/ColorSchemeManager_spec.js | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index 2d33566d72b45..c9967f67297b0 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -7,7 +7,7 @@ import CategoricalColorNamespace, { } from '../../../src/modules/CategoricalColorNamespace'; import { registerScheme } from '../../../src/modules/ColorSchemeManager'; -describe.only('CategoricalColorNamespace', () => { +describe('CategoricalColorNamespace', () => { before(() => { registerScheme('testColors', ['red', 'green', 'blue']); registerScheme('testColors2', ['red', 'green', 'blue']); diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js index 58d5e2c3f314a..1d3ee002238f2 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -56,6 +56,11 @@ describe('CategoricalColorScale', () => { 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', () => { diff --git a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js index ffb950b559ebb..236b1e43a0ac3 100644 --- a/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js +++ b/superset/assets/spec/javascripts/modules/ColorSchemeManager_spec.js @@ -10,7 +10,7 @@ import ColorSchemeManager, { registerMultipleSchemes, } from '../../../src/modules/ColorSchemeManager'; -describe.only('ColorSchemeManager', () => { +describe('ColorSchemeManager', () => { beforeEach(() => { const m = getInstance(); m.clearScheme(); From 1312dd56bb6a1c18f77f5ba82ab8e92a0b1d2647 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Mon, 10 Sep 2018 17:58:01 -0700 Subject: [PATCH 27/33] fix color tests --- .../spec/javascripts/modules/colors_spec.jsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index d7fb9bda3806f..b4b51d42c0959 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -1,14 +1,17 @@ -import { it, describe } from 'mocha'; +import { it, describe, before } from 'mocha'; import { expect } from 'chai'; import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; -import { getAllSchemes } from '../../../src/modules/ColorSchemeManager'; +import { getAllSchemes, setDefaultSchemeName } from '../../../src/modules/ColorSchemeManager'; const ALL_COLOR_SCHEMES = getAllSchemes(); describe('colors', () => { + before(() => { + setDefaultSchemeName('bnbColors'); + }); it('default to bnbColors', () => { const color1 = getColorFromScheme('CA'); - expect(color1).to.equal(ALL_COLOR_SCHEMES.bnbColors[0]); + expect(ALL_COLOR_SCHEMES.bnbColors).to.include(color1); }); it('getColorFromScheme series with same scheme should have the same color', () => { const color1 = getColorFromScheme('CA', 'bnbColors'); @@ -16,10 +19,9 @@ describe('colors', () => { const color3 = getColorFromScheme('CA', 'bnbColors'); const color4 = getColorFromScheme('NY', 'bnbColors'); - expect(color1).to.equal(ALL_COLOR_SCHEMES.bnbColors[0]); - expect(color2).to.equal(ALL_COLOR_SCHEMES.googleCategory20c[0]); expect(color1).to.equal(color3); - expect(color4).to.equal(ALL_COLOR_SCHEMES.bnbColors[1]); + expect(color1).to.not.equal(color2); + expect(color1).to.not.equal(color4); }); it('getColorFromScheme forcing colors persists through calls', () => { expect(getColorFromScheme('boys', 'bnbColors', 'blue')).to.equal('blue'); From 9dbccd6a9016457bfaecd847ef703efc079d25d2 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Sep 2018 16:39:08 -0700 Subject: [PATCH 28/33] update unit tests --- .../modules/CategoricalColorNameSpace_spec.js | 3 ++- .../modules/CategoricalColorScale_spec.js | 26 ++++++++++++++++++- .../src/modules/CategoricalColorNamespace.js | 2 +- .../src/modules/CategoricalColorScale.js | 5 ++++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index c9967f67297b0..a0a185c814917 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -4,6 +4,7 @@ import CategoricalColorNamespace, { getNamespace, getScale, getColor, + DEFAULT_NAMESPACE, } from '../../../src/modules/CategoricalColorNamespace'; import { registerScheme } from '../../../src/modules/ColorSchemeManager'; @@ -19,7 +20,7 @@ describe('CategoricalColorNamespace', () => { it('returns default namespace if name is not specified', () => { const namespace = getNamespace(); expect(namespace !== undefined).to.equal(true); - expect(namespace.name).to.equal('GLOBAL'); + expect(namespace.name).to.equal(DEFAULT_NAMESPACE); }); it('returns namespace with specified name', () => { const namespace = getNamespace('myNamespace'); diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js index 1d3ee002238f2..adbd2c71da575 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -1,3 +1,4 @@ +import 'babel-polyfill'; import { it, describe } from 'mocha'; import { expect } from 'chai'; import CategoricalColorScale from '../../../src/modules/CategoricalColorScale'; @@ -8,7 +9,7 @@ describe('CategoricalColorScale', () => { }); describe('new CategoricalColorScale(colors, sharedForcedColors)', () => { - it('can create new scale when sharedForcedItem is not given', () => { + it('can create new scale when sharedForcedColors is not given', () => { const scale = new CategoricalColorScale(['blue', 'red', 'green']); expect(scale).to.be.instanceOf(CategoricalColorScale); }); @@ -41,6 +42,29 @@ describe('CategoricalColorScale', () => { 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', () => { diff --git a/superset/assets/src/modules/CategoricalColorNamespace.js b/superset/assets/src/modules/CategoricalColorNamespace.js index bf9f81f7792d5..d022bb25fbdb9 100644 --- a/superset/assets/src/modules/CategoricalColorNamespace.js +++ b/superset/assets/src/modules/CategoricalColorNamespace.js @@ -36,7 +36,7 @@ class CategoricalColorNamespace { } const namespaces = {}; -const DEFAULT_NAMESPACE = 'GLOBAL'; +export const DEFAULT_NAMESPACE = 'GLOBAL'; export function getNamespace(name = DEFAULT_NAMESPACE) { const instance = namespaces[name]; diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index a4466b597e60b..db38e56fbea76 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -10,6 +10,11 @@ export function cleanValue(value) { } export default class CategoricalColorScale { + /** + * Constructor + * @param {*} colors an array of colors + * @param {*} sharedForcedColors optional parameter that comes from parent (usually CategoricalColorNamespace) and supersede this.forcedColors + */ constructor(colors, sharedForcedColors) { this.colors = colors; this.sharedForcedColors = sharedForcedColors; From d08be398e0e01c7dd079c21a689620a83a34cae2 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Sep 2018 16:43:20 -0700 Subject: [PATCH 29/33] update unit tests --- .../javascripts/modules/CategoricalColorNameSpace_spec.js | 2 +- .../spec/javascripts/modules/CategoricalColorScale_spec.js | 6 +++--- superset/assets/src/modules/CategoricalColorScale.js | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js index a0a185c814917..1696dd28aa33b 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorNameSpace_spec.js @@ -80,7 +80,7 @@ describe('CategoricalColorNamespace', () => { }); it('returns the namespace instance', () => { const ns1 = getNamespace('test-set-scale3.1'); - const ns2 =ns1.setColor('dog', 'black'); + const ns2 = ns1.setColor('dog', 'black'); expect(ns1).to.equal(ns2); }); }); diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js index adbd2c71da575..85df96abbf594 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -53,15 +53,15 @@ describe('CategoricalColorScale', () => { scale.getColor('donkey'), scale.getColor('goat'), ]; - colors.forEach(color => { - if(colorSet[color]) { + 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 => { + ['blue', 'red', 'green'].forEach((color) => { expect(colorSet[color]).to.equal(2); }); }); diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index db38e56fbea76..05b9394960133 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -13,7 +13,8 @@ export default class CategoricalColorScale { /** * Constructor * @param {*} colors an array of colors - * @param {*} sharedForcedColors optional parameter that comes from parent (usually CategoricalColorNamespace) and supersede this.forcedColors + * @param {*} sharedForcedColors optional parameter that comes from parent + * (usually CategoricalColorNamespace) and supersede this.forcedColors */ constructor(colors, sharedForcedColors) { this.colors = colors; From 06970deb56a175b7ee4737644f36648191c70385 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Sep 2018 21:50:24 -0700 Subject: [PATCH 30/33] move color scheme registration to common --- superset/assets/src/common.js | 13 ++++++++++++- superset/assets/src/modules/ColorSchemeManager.js | 12 ------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/superset/assets/src/common.js b/superset/assets/src/common.js index 67ce4982d2041..6815e0cb70e94 100644 --- a/superset/assets/src/common.js +++ b/superset/assets/src/common.js @@ -1,5 +1,10 @@ /* eslint-disable global-require */ import $ from 'jquery'; +import airbnb from './modules/colorSchemes/airbnb'; +import categoricalSchemes from './modules/colorSchemes/categorical'; +import lyft from './modules/colorSchemes/lyft'; +import { getInstance } from './modules/ColorSchemeManager'; + // Everything imported in this file ends up in the common entry file // be mindful of double-imports @@ -25,8 +30,14 @@ $(document).ready(function () { }); }); +getInstance() + .registerScheme('bnbColors', airbnb.bnbColors) + .registerMultipleSchemes(categoricalSchemes) + .registerScheme('lyftColors', lyft.lyftColors) + .setDefaultSchemeName('bnbColors'); + export function appSetup() { - // A set of hacks to allow apps to run within a FAB template + // A set of hacks to allow apps to run within a FAB template // this allows for the server side generated menus to function window.$ = $; window.jQuery = $; diff --git a/superset/assets/src/modules/ColorSchemeManager.js b/superset/assets/src/modules/ColorSchemeManager.js index 9654aca6ce389..9d21d2628e245 100644 --- a/superset/assets/src/modules/ColorSchemeManager.js +++ b/superset/assets/src/modules/ColorSchemeManager.js @@ -1,7 +1,3 @@ -import airbnb from './colorSchemes/airbnb'; -import categoricalSchemes from './colorSchemes/categorical'; -import lyft from './colorSchemes/lyft'; - class ColorSchemeManager { constructor() { this.schemes = {}; @@ -88,11 +84,3 @@ export { registerScheme, registerMultipleSchemes, }; - -// These registration code eventually should go into per-app configuration -// when we migrate to the plug-in system. -getInstance() - .registerScheme('bnbColors', airbnb.bnbColors) - .registerMultipleSchemes(categoricalSchemes) - .registerScheme('lyftColors', lyft.lyftColors) - .setDefaultSchemeName('bnbColors'); From 527e2445b36c31eeed70853478aaf0a52c7916e5 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Sep 2018 21:57:32 -0700 Subject: [PATCH 31/33] update unit test --- .../spec/javascripts/modules/colors_spec.jsx | 14 +++++++++----- superset/assets/src/common.js | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/colors_spec.jsx b/superset/assets/spec/javascripts/modules/colors_spec.jsx index b4b51d42c0959..be8823302034c 100644 --- a/superset/assets/spec/javascripts/modules/colors_spec.jsx +++ b/superset/assets/spec/javascripts/modules/colors_spec.jsx @@ -1,17 +1,21 @@ import { it, describe, before } from 'mocha'; import { expect } from 'chai'; import { getColorFromScheme, hexToRGB } from '../../../src/modules/colors'; -import { getAllSchemes, setDefaultSchemeName } from '../../../src/modules/ColorSchemeManager'; - -const ALL_COLOR_SCHEMES = getAllSchemes(); +import { getInstance } from '../../../src/modules/ColorSchemeManager'; +import airbnb from '../../../src/modules/colorSchemes/airbnb'; +import categoricalSchemes from '../../../src/modules/colorSchemes/categorical'; describe('colors', () => { before(() => { - setDefaultSchemeName('bnbColors'); + // Register color schemes + getInstance() + .registerScheme('bnbColors', airbnb.bnbColors) + .registerMultipleSchemes(categoricalSchemes) + .setDefaultSchemeName('bnbColors'); }); it('default to bnbColors', () => { const color1 = getColorFromScheme('CA'); - expect(ALL_COLOR_SCHEMES.bnbColors).to.include(color1); + expect(airbnb.bnbColors).to.include(color1); }); it('getColorFromScheme series with same scheme should have the same color', () => { const color1 = getColorFromScheme('CA', 'bnbColors'); diff --git a/superset/assets/src/common.js b/superset/assets/src/common.js index 6815e0cb70e94..779a1692e8a23 100644 --- a/superset/assets/src/common.js +++ b/superset/assets/src/common.js @@ -30,6 +30,7 @@ $(document).ready(function () { }); }); +// Register color schemes getInstance() .registerScheme('bnbColors', airbnb.bnbColors) .registerMultipleSchemes(categoricalSchemes) From 1cb5d61691ed98a220ad8a2f1312c4fe16cf8cb5 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Sep 2018 21:59:15 -0700 Subject: [PATCH 32/33] rename sharedForcedColors to parentForcedColors --- .../modules/CategoricalColorScale_spec.js | 14 +++++++------- .../assets/src/modules/CategoricalColorScale.js | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js index 85df96abbf594..0eabffa56df7b 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -8,16 +8,16 @@ describe('CategoricalColorScale', () => { expect(CategoricalColorScale !== undefined).to.equal(true); }); - describe('new CategoricalColorScale(colors, sharedForcedColors)', () => { - it('can create new scale when sharedForcedColors is not given', () => { + 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 sharedForcedColors is given', () => { - const sharedForcedColors = {}; - const scale = new CategoricalColorScale(['blue', 'red', 'green'], sharedForcedColors); + 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.sharedForcedColors).to.equal(sharedForcedColors); + expect(scale.parentForcedColors).to.equal(parentForcedColors); }); }); describe('.getColor(value)', () => { @@ -72,7 +72,7 @@ describe('CategoricalColorScale', () => { scale.setColor('pig', 'pink'); expect(scale.getColor('pig')).to.equal('pink'); }); - it('does not override sharedForcedColors', () => { + 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); diff --git a/superset/assets/src/modules/CategoricalColorScale.js b/superset/assets/src/modules/CategoricalColorScale.js index 05b9394960133..eab70d218424d 100644 --- a/superset/assets/src/modules/CategoricalColorScale.js +++ b/superset/assets/src/modules/CategoricalColorScale.js @@ -13,12 +13,12 @@ export default class CategoricalColorScale { /** * Constructor * @param {*} colors an array of colors - * @param {*} sharedForcedColors optional parameter that comes from parent + * @param {*} parentForcedColors optional parameter that comes from parent * (usually CategoricalColorNamespace) and supersede this.forcedColors */ - constructor(colors, sharedForcedColors) { + constructor(colors, parentForcedColors) { this.colors = colors; - this.sharedForcedColors = sharedForcedColors; + this.parentForcedColors = parentForcedColors; this.forcedColors = {}; this.seen = {}; this.fn = value => this.getColor(value); @@ -27,9 +27,9 @@ export default class CategoricalColorScale { getColor(value) { const cleanedValue = cleanValue(value); - const sharedColor = this.sharedForcedColors && this.sharedForcedColors[cleanedValue]; - if (sharedColor) { - return sharedColor; + const parentColor = this.parentForcedColors && this.parentForcedColors[cleanedValue]; + if (parentColor) { + return parentColor; } const forcedColor = this.forcedColors[cleanedValue]; From 1cf4695a21ef9a1dbbaf70e9afdcf0a197a29e26 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Sep 2018 21:59:37 -0700 Subject: [PATCH 33/33] remove import --- .../spec/javascripts/modules/CategoricalColorScale_spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js index 0eabffa56df7b..fc2e2ea99030e 100644 --- a/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js +++ b/superset/assets/spec/javascripts/modules/CategoricalColorScale_spec.js @@ -1,4 +1,3 @@ -import 'babel-polyfill'; import { it, describe } from 'mocha'; import { expect } from 'chai'; import CategoricalColorScale from '../../../src/modules/CategoricalColorScale';