ensure coverage for all cases; remove useSeriesKey argument;

add seriesColorFromField and refactor for readability; use monocolored legends
jordanabinader · Jul 11, 2023 · dc8ee09 · dc8ee09
1 parent ef1e0e8
commit dc8ee09
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 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
@@ -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
@@ -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
@@ -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 + "";