From ef1e0e8034ff174fbcf4262b1b56d5d826efdc04 Mon Sep 17 00:00:00 2001 From: Pearl Lamptey Date: Sat, 24 Jun 2023 15:52:36 +0000 Subject: [PATCH 1/2] fix legend color and chart color mismatch --- .../src/js/charts/xy-scatter.js | 20 ++++++++++++++++--- .../src/js/series/pointSeriesCanvas.js | 7 +++---- .../src/js/series/seriesColors.js | 4 +++- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js b/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js index cc5a17f038..daa719c4f6 100644 --- a/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js +++ b/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js @@ -18,7 +18,10 @@ import { symbolTypeFromGroups, } from "../series/pointSeriesCanvas"; import { pointData } from "../data/pointData"; -import { seriesColorsFromGroups } from "../series/seriesColors"; +import { + seriesColorsFromGroups, + seriesColorsFromDistinct, +} from "../series/seriesColors"; import { seriesLinearRange, seriesColorRange } from "../series/seriesRange"; import { symbolLegend } from "../legend/legend"; import { colorRangeLegend } from "../legend/colorRangeLegend"; @@ -49,18 +52,28 @@ function interpolate_scale([x1, y1], [x2, y2]) { function xyScatter(container, settings) { const data = pointData(settings, filterDataByGroup(settings)); const symbols = symbolTypeFromGroups(settings); + const color_column = settings.realValues[2]; + const color_column_type = settings.mainValues.find( + (x) => x.name === color_column + )?.type; const useGroupColors = settings.realValues.length <= 2 || settings.realValues[2] === null; let color = null; let legend = null; + let useSeriesKey = false; - if (useGroupColors) { + if (color_column_type === "string") { + color = seriesColorsFromDistinct(settings, data); + legend = symbolLegend().settings(settings).scale(symbols).color(color); + useSeriesKey = true; + } else if (useGroupColors) { color = seriesColorsFromGroups(settings); legend = symbolLegend() .settings(settings) .scale(symbols) .color(useGroupColors ? color : null); + useSeriesKey = true; } else { color = seriesColorRange(settings, data, "colorValue"); legend = colorRangeLegend().scale(color); @@ -85,7 +98,8 @@ function xyScatter(container, settings) { color, label, symbols, - scale_factor + scale_factor, + useSeriesKey ) ) ); diff --git a/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js b/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js index 59af0be082..4b078d04a3 100644 --- a/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js +++ b/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js @@ -26,7 +26,8 @@ export function pointSeriesCanvas( color, label, symbols, - scale_factor = 1 + scale_factor = 1, + useSeriesKey = False ) { let series = seriesCanvasPoint() .crossValue((d) => d.x) @@ -40,9 +41,7 @@ export function pointSeriesCanvas( } series.decorate((context, d) => { - const colorValue = color( - d.colorValue !== undefined ? d.colorValue : seriesKey - ); + let colorValue = useSeriesKey ? color(seriesKey) : color(d.colorValue); const opacity = settings.colorStyles && settings.colorStyles.opacity; if (label) { diff --git a/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js b/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js index a165d082a5..3758d99bc0 100644 --- a/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js +++ b/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js @@ -103,7 +103,9 @@ export function withOpacity(color, opacity = 0.5) { export function setOpacity(opacity) { return (color) => { const decoded = d3.color(color); - decoded.opacity = opacity; + if (decoded != null) { + decoded.opacity = opacity; + } return decoded + ""; }; } From dc8ee092a6d5d501faa3db16153b384e791c1c65 Mon Sep 17 00:00:00 2001 From: ada mandala Date: Wed, 5 Jul 2023 12:14:36 -0500 Subject: [PATCH 2/2] ensure coverage for all cases; remove useSeriesKey argument; add seriesColorFromField and refactor for readability; use monocolored legends --- .../src/js/charts/xy-scatter.js | 59 +++++++++++-------- .../src/js/series/pointSeriesCanvas.js | 5 +- .../src/js/series/seriesColors.js | 39 ++++++++---- 3 files changed, 62 insertions(+), 41 deletions(-) diff --git a/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js b/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js index daa719c4f6..47a4ed6093 100644 --- a/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js +++ b/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js @@ -11,6 +11,7 @@ // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ import * as fc from "d3fc"; +import * as d3 from "d3"; import { axisFactory } from "../axis/axisFactory"; import { chartCanvasFactory } from "../axis/chartFactory"; import { @@ -19,11 +20,13 @@ import { } from "../series/pointSeriesCanvas"; import { pointData } from "../data/pointData"; import { + seriesColorsFromField, seriesColorsFromGroups, seriesColorsFromDistinct, + colorScale, } from "../series/seriesColors"; import { seriesLinearRange, seriesColorRange } from "../series/seriesRange"; -import { symbolLegend } from "../legend/legend"; +import { symbolLegend, colorLegend, colorGroupLegend } from "../legend/legend"; import { colorRangeLegend } from "../legend/colorRangeLegend"; import { filterDataByGroup } from "../legend/filter"; import withGridLines from "../gridlines/gridlines"; @@ -52,31 +55,36 @@ function interpolate_scale([x1, y1], [x2, y2]) { function xyScatter(container, settings) { const data = pointData(settings, filterDataByGroup(settings)); const symbols = symbolTypeFromGroups(settings); - const color_column = settings.realValues[2]; - const color_column_type = settings.mainValues.find( - (x) => x.name === color_column - )?.type; - const useGroupColors = - settings.realValues.length <= 2 || settings.realValues[2] === null; let color = null; let legend = null; - let useSeriesKey = false; - - if (color_column_type === "string") { - color = seriesColorsFromDistinct(settings, data); - legend = symbolLegend().settings(settings).scale(symbols).color(color); - useSeriesKey = true; - } else if (useGroupColors) { - color = seriesColorsFromGroups(settings); - - legend = symbolLegend() - .settings(settings) - .scale(symbols) - .color(useGroupColors ? color : null); - useSeriesKey = true; + + const colorByField = 2; + const colorByValue = settings.realValues[colorByField]; + let hasColorBy = colorByValue !== null && colorByValue !== undefined; + let isColoredByString = + settings.mainValues.find((x) => x.name === colorByValue)?.type === + "string"; + let hasSplitBy = settings.splitValues.length > 0; + + if (hasColorBy) { + if (isColoredByString) { + if (hasSplitBy) { + color = seriesColorsFromDistinct(settings, data); + // TODO: Legend should have cartesian product labels (ColorBy|SplitBy) + // For now, just use monocolor legends. + legend = symbolLegend().settings(settings).scale(symbols); + } else { + color = seriesColorsFromField(settings, colorByField); + legend = colorLegend().settings(settings).scale(color); + } + } else { + color = seriesColorRange(settings, data, "colorValue"); + legend = colorRangeLegend().scale(color); + } } else { - color = seriesColorRange(settings, data, "colorValue"); - legend = colorRangeLegend().scale(color); + // always use default color + color = colorScale().settings(settings).domain([""])(); + legend = symbolLegend().settings(settings).scale(symbols); } const size = settings.realValues[3] @@ -98,8 +106,7 @@ function xyScatter(container, settings) { color, label, symbols, - scale_factor, - useSeriesKey + scale_factor ) ) ); @@ -144,7 +151,7 @@ function xyScatter(container, settings) { .xValueName("x") .yValueName("y") .yScale(yAxis.scale) - .color(useGroupColors && color) + .color(!hasColorBy && color) .size(size) .data(data); diff --git a/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js b/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js index 4b078d04a3..7f22d98084 100644 --- a/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js +++ b/packages/perspective-viewer-d3fc/src/js/series/pointSeriesCanvas.js @@ -26,8 +26,7 @@ export function pointSeriesCanvas( color, label, symbols, - scale_factor = 1, - useSeriesKey = False + scale_factor = 1 ) { let series = seriesCanvasPoint() .crossValue((d) => d.x) @@ -41,7 +40,7 @@ export function pointSeriesCanvas( } series.decorate((context, d) => { - let colorValue = useSeriesKey ? color(seriesKey) : color(d.colorValue); + const colorValue = color(d.colorValue); const opacity = settings.colorStyles && settings.colorStyles.opacity; if (label) { diff --git a/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js b/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js index 3758d99bc0..27e8a11073 100644 --- a/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js +++ b/packages/perspective-viewer-d3fc/src/js/series/seriesColors.js @@ -20,23 +20,38 @@ export function seriesColors(settings) { return colorScale().settings(settings).domain(domain)(); } +// TODO: We're iterating over all the data here to get the unique values for each colorBy field. +// This is the only way to do it since we don't know the range of these values ahead of time. +// This should be WASM-side code. +export function seriesColorsFromField(settings, field) { + const data = settings.data; + const key = settings.realValues[field]; + // alt: + // const domain = [...new Set(data.map((obj) => obj[key]))].sort(); + const domain = data + .reduce((accum, obj) => { + const val = obj[key]; + return accum.includes(val) ? accum : [...accum, val]; + }, []) + .sort(); + return colorScale().settings(settings).domain(domain)(); +} + export function seriesColorsFromDistinct(settings, data) { let domain = Array.from(new Set(data)); return colorScale().settings(settings).domain(domain)(); } export function seriesColorsFromGroups(settings) { - const col = - settings.data && settings.data.length > 0 ? settings.data[0] : {}; - const domain = []; - Object.keys(col).forEach((key) => { - if (key !== "__ROW_PATH__") { - const group = groupFromKey(key); - if (!domain.includes(group)) { - domain.push(group); - } - } - }); + const col = settings.data[0] ?? {}; + // alt: + // const domain = [...new Set(Object.keys(col).filter(k => k !== "__ROW_PATH__").map(k => groupFromKey(k)))]; + const domain = Object.keys(col).reduce((accum, key) => { + if (key === "__ROW_PATH__") return accum; + const group = groupFromKey(key); + return accum.includes(group) ? accum : [...accum, group]; + }, []); + return colorScale().settings(settings).domain(domain)(); } @@ -103,7 +118,7 @@ export function withOpacity(color, opacity = 0.5) { export function setOpacity(opacity) { return (color) => { const decoded = d3.color(color); - if (decoded != null) { + if (decoded !== null && decoded !== undefined) { decoded.opacity = opacity; } return decoded + "";