Merge pull request #2303 from ada-x64/fix-xy-scatter-plots-rebased

Fix XY-Scatter Plots Colors - Rebased
finos · Jul 13, 2023 · 2d495d9 · 2d495d9
2 parents 2e5c67b + dc8ee09
commit 2d495d9
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 29 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
@@ -11,16 +11,22 @@
 // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
 
 import * as fc from "d3fc";
+import * as d3 from "d3";
 import { axisFactory } from "../axis/axisFactory";
 import { chartCanvasFactory } from "../axis/chartFactory";
 import {
     pointSeriesCanvas,
     symbolTypeFromGroups,
 } from "../series/pointSeriesCanvas";
 import { pointData } from "../data/pointData";
-import { seriesColorsFromGroups } from "../series/seriesColors";
+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";
@@ -49,21 +55,36 @@ function interpolate_scale([x1, y1], [x2, y2]) {
 function xyScatter(container, settings) {
     const data = pointData(settings, filterDataByGroup(settings));
     const symbols = symbolTypeFromGroups(settings);
-    const useGroupColors =
-        settings.realValues.length <= 2 || settings.realValues[2] === null;
     let color = null;
     let legend = null;
 
-    if (useGroupColors) {
-        color = seriesColorsFromGroups(settings);
-
-        legend = symbolLegend()
-            .settings(settings)
-            .scale(symbols)
-            .color(useGroupColors ? color : null);
+    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]
@@ -130,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
@@ -40,9 +40,7 @@ export function pointSeriesCanvas(
     }
 
     series.decorate((context, d) => {
-        const colorValue = color(
-            d.colorValue !== undefined ? d.colorValue : seriesKey
-        );
+        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
@@ -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,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 !== undefined) {
+            decoded.opacity = opacity;
+        }
         return decoded + "";
     };
 }