Fix columns_config and plugin_config bugs

Signed-off-by: Andrew Stein <steinlink@gmail.com>

Author: texodus

.github/workflows/build.yaml

@@ -28,7 +28,7 @@ on:
             - docs/
             - examples/
             - rust/perspective-python/README.md
-    pull_request_target:
+    pull_request:
         branches:
             - master
     workflow_dispatch:

packages/viewer-charts/src/ts/axis/categorical-axis.ts

@@ -56,8 +56,6 @@ function categoryIndexToPixelY(layout: PlotLayout, index: number): number {
     return layout.dataToPixel(0, index).py;
 }
 
-export const categoryIndexToPixel = categoryIndexToPixelX;
-
 function leafLevelLayout(
     numRows: number,
     longestCharCount: number,

packages/viewer-charts/src/ts/charts/candlestick/candlestick-render.ts

@@ -125,13 +125,14 @@ export function renderCandlestickFrame(
               yMax: chart._yDomain.max,
           };
 
-    // Auto-fit the price axis to the visible X window. Skipped at
-    // default zoom (the refit equals `_yDomain` there and would only
-    // churn baselines).
+    // Auto-fit the price axis to the visible X window. Skipped when X
+    // is at default zoom (the refit equals `_yDomain` there and would
+    // only churn baselines) — Y-axis pan/zoom alone shouldn't trigger
+    // an X-window refit.
     if (
         chart._autoFitValue &&
         chart._zoomController &&
-        !chart._zoomController.isDefault()
+        !chart._zoomController.isXDefault()
     ) {
         const fit = computeVisibleCandleExtent(chart, vis.xMin, vis.xMax);
         if (fit.hasFit) {

packages/viewer-charts/src/ts/charts/candlestick/candlestick.ts

@@ -33,6 +33,7 @@ import {
 } from "./candlestick-interact";
 import { BodyWickGlyph } from "./glyphs/draw-candlesticks";
 import { OHLCGlyph } from "./glyphs/draw-ohlc";
+import { expandDomainInPlace } from "../common/expand-domain";
 
 /**
  * Per-frame memo of the auto-fit Y extent for a {@link CandlestickChart},
@@ -266,38 +267,18 @@ export class CandlestickChart extends CategoricalYChart {
             scratchCandles: this._candles,
         });
         // `domain_mode: "expand"` post-build union — mirrors the series
-        // pipeline. Mutate the pipeline result in place so the
+        // pipeline. `expandDomainInPlace` mutates `result.*` so the
         // assignments below pick up the grown extent automatically.
         if (this._pluginConfig.domain_mode === "expand") {
-            if (this._expandedYDomain) {
-                result.yDomain.min = Math.min(
-                    this._expandedYDomain.min,
-                    result.yDomain.min,
-                );
-                result.yDomain.max = Math.max(
-                    this._expandedYDomain.max,
-                    result.yDomain.max,
-                );
-            }
-
-            this._expandedYDomain = { ...result.yDomain };
-
+            this._expandedYDomain = expandDomainInPlace(
+                this._expandedYDomain,
+                result.yDomain,
+            );
             if (result.numericCategoryDomain) {
-                if (this._expandedCategoryDomain) {
-                    result.numericCategoryDomain.min = Math.min(
-                        this._expandedCategoryDomain.min,
-                        result.numericCategoryDomain.min,
-                    );
-                    result.numericCategoryDomain.max = Math.max(
-                        this._expandedCategoryDomain.max,
-                        result.numericCategoryDomain.max,
-                    );
-                }
-
-                this._expandedCategoryDomain = {
-                    min: result.numericCategoryDomain.min,
-                    max: result.numericCategoryDomain.max,
-                };
+                this._expandedCategoryDomain = expandDomainInPlace(
+                    this._expandedCategoryDomain,
+                    result.numericCategoryDomain,
+                );
             }
         } else {
             this._expandedYDomain = null;

packages/viewer-charts/src/ts/charts/chart-base.ts

@@ -46,6 +46,14 @@ import type { ViewConfig } from "@perspective-dev/client";
 import { resolveThemeFromVars, type Theme } from "../theme/theme";
 import { requestRender as scheduleRender } from "../render/scheduler";
 
+// TODO I don't know if this is the behavior we want. On the plus side, this
+// ad-hoc formatter scales well to small and large data ranges, making a good
+// guess at the right format without user input. On the minus side, this
+// behavior is inconsistent with datagrid and the rest of the app, and the ad-hoc
+// surprising behavior when overriding one field in `number_format` and suddenly
+// the entire formatter is replaced.
+const REGRESSION_BEHAVIOR = true;
+
 /**
  * Locale-aware fallback formatter applied to numeric tooltip / legend
  * values when the column has no `number_format` configured. Two
@@ -55,9 +63,12 @@ import { requestRender as scheduleRender } from "../render/scheduler";
 const DEFAULT_VALUE_FORMATTER: (v: number) => string = ((): ((
     v: number,
 ) => string) => {
-    return formatTickValue;
-    // const intl = createNumberFormatter("float");
-    // return (v) => intl.format(v);
+    if (REGRESSION_BEHAVIOR) {
+        return formatTickValue;
+    } else {
+        const intl = createNumberFormatter("float");
+        return (v) => intl.format(v);
+    }
 })();
 
 /**
@@ -69,9 +80,12 @@ const DEFAULT_VALUE_FORMATTER: (v: number) => string = ((): ((
 const DEFAULT_DATETIME_FORMATTER: (v: number) => string = ((): ((
     v: number,
 ) => string) => {
-    return formatDateTickValue;
-    // const intl = createDatetimeFormatter();
-    // return (v) => intl.format(v);
+    if (REGRESSION_BEHAVIOR) {
+        return formatDateTickValue;
+    } else {
+        const intl = createDatetimeFormatter();
+        return (v) => intl.format(v);
+    }
 })();
 
 /**

packages/viewer-charts/src/ts/charts/common/expand-domain.ts

@@ -0,0 +1,40 @@
+// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
+// ┃ ██████ ██████ ██████       █      █      █      █      █ █▄  ▀███ █       ┃
+// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█  ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄  ▀█ █ ▀▀▀▀▀ ┃
+// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄   █ ▄▄▄▄▄ ┃
+// ┃ █      ██████ █  ▀█▄       █ ██████      █      ███▌▐███ ███████▄ █       ┃
+// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
+// ┃ Copyright (c) 2017, the Perspective Authors.                              ┃
+// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃
+// ┃ This file is part of the Perspective library, distributed under the terms ┃
+// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
+// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
+
+/**
+ * Numeric extent — used by series + candlestick build pipelines for
+ * value / category axis domains.
+ */
+export interface Domain {
+    min: number;
+    max: number;
+}
+
+/**
+ * Union `next` (a freshly-computed extent) with `prev` (the prior
+ * accumulator) IN PLACE on `next`, then return a fresh copy to store
+ * back as the new accumulator. Idempotent when `prev` is null — `next`
+ * is left untouched.
+ *
+ * Used by the `domain_mode: "expand"` mirror-back step in the series /
+ * candlestick / cartesian build pipelines: mutating `next` in place
+ * means every downstream assignment that reads from the pipeline
+ * result struct automatically picks up the grown extent.
+ */
+export function expandDomainInPlace(prev: Domain | null, next: Domain): Domain {
+    if (prev) {
+        next.min = Math.min(prev.min, next.min);
+        next.max = Math.max(prev.max, next.max);
+    }
+
+    return { min: next.min, max: next.max };
+}

packages/viewer-charts/src/ts/charts/common/tree-chart.ts

@@ -11,9 +11,25 @@
 // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
 
 import { AbstractChart } from "../chart-base";
+import type { ColumnDataMap } from "../../data/view-reader";
 import { NodeStore, NULL_NODE } from "./node-store";
 import { LazyTooltip } from "../../interaction/lazy-tooltip";
 
+/**
+ * Sentinel fallback for the Size slot when the user hasn't picked one:
+ * use the first non-metadata column in the incoming view. Tree charts
+ * still need *some* numeric-ish column to size geometry.
+ */
+export function firstNonMetadataColumn(columns: ColumnDataMap): string {
+    for (const k of columns.keys()) {
+        if (!k.startsWith("__")) {
+            return k;
+        }
+    }
+
+    return "";
+}
+
 /**
  * Shared state for hierarchical charts (treemap, sunburst). Holds the
  * tree store + streaming-insert scaffolding + per-row tooltip data

packages/viewer-charts/src/ts/charts/common/tree-chrome.ts

@@ -10,7 +10,17 @@
 // ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
 // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
 
-import type { Context2D } from "../canvas-types";
+import type { Canvas2D, Context2D } from "../canvas-types";
+import type { PlotRect } from "../../layout/plot-layout";
+import { PlotLayout } from "../../layout/plot-layout";
+import type { GradientStop } from "../../theme/gradient";
+import type { Vec3 } from "../../theme/palette";
+import type { Theme } from "../../theme/theme";
+import {
+    renderCategoricalLegend,
+    renderCategoricalLegendAt,
+    renderLegend,
+} from "../../axis/legend";
 import type { TreeChartBase } from "./tree-chart";
 import { drawTooltipBox } from "./draw-tooltip-box";
 
@@ -121,3 +131,78 @@ export function renderTreeTooltip(
         fontFamily,
     );
 }
+
+/**
+ * Paint a color legend (categorical swatches or numeric gradient bar)
+ * for a tree chart. Shared by sunburst + treemap; both consult
+ * `_colorMode` / `_uniqueColorLabels.size` / `_colorMin..max` the same
+ * way.
+ *
+ * `categoricalRect`, when non-null, is used as the explicit rect for
+ * the categorical-swatch variant (sunburst's faceted mode passes
+ * `FacetGrid.legendRect` here). Numeric mode always derives from a
+ * synthetic single-plot `PlotLayout` to match the legacy per-chart
+ * branch — its gradient bar's vertical span doesn't fit the
+ * categorical legend's compact rect.
+ *
+ * Returns silently when the color slot is empty, when categorical mode
+ * has only one label, or when numeric mode has a degenerate
+ * (`min >= max`) extent.
+ */
+export function renderTreeColorLegend(
+    chart: TreeChartBase,
+    canvas: Canvas2D,
+    palette: Vec3[],
+    stops: GradientStop[],
+    theme: Theme,
+    cssWidth: number,
+    cssHeight: number,
+    categoricalRect: PlotRect | null = null,
+): void {
+    if (chart._colorMode === "series" && chart._uniqueColorLabels.size > 1) {
+        if (categoricalRect) {
+            renderCategoricalLegendAt(
+                canvas,
+                categoricalRect,
+                chart._uniqueColorLabels,
+                palette,
+                theme,
+            );
+        } else {
+            renderCategoricalLegend(
+                canvas,
+                syntheticLegendLayout(cssWidth, cssHeight),
+                chart._uniqueColorLabels,
+                palette,
+                theme,
+            );
+        }
+    } else if (
+        chart._colorMode === "numeric" &&
+        chart._colorMin < chart._colorMax
+    ) {
+        renderLegend(
+            canvas,
+            syntheticLegendLayout(cssWidth, cssHeight),
+            {
+                min: chart._colorMin,
+                max: chart._colorMax,
+                label: chart._colorName,
+            },
+            stops,
+            theme,
+            chart.getColumnFormatter(chart._colorName, "value"),
+        );
+    }
+}
+
+function syntheticLegendLayout(
+    cssWidth: number,
+    cssHeight: number,
+): PlotLayout {
+    return new PlotLayout(cssWidth, cssHeight, {
+        hasXLabel: false,
+        hasYLabel: false,
+        hasLegend: true,
+    });
+}