Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion web-common/src/features/components/charts/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,11 @@ export function createCartesianMultiValueTooltipChannel(
return multiValueTooltipChannel;
}

/**
* Hover target layer shared by the cartesian charts. `xField` is the raw x
* field name: it is escaped for the encoding here and used as is to look up
* the chart's domain values, which are keyed by raw names.
*/
export function buildHoverRuleLayer(args: {
xField?: string;
defaultTooltip: TooltipValue[];
Expand All @@ -595,6 +600,7 @@ export function buildHoverRuleLayer(args: {
isDarkMode = false,
isInteractive = false,
} = args;
const sanitizedXField = xField ? sanitizeValueForVega(xField) : undefined;

const params: SelectionParameter[] = [
{
Expand Down Expand Up @@ -632,7 +638,7 @@ export function buildHoverRuleLayer(args: {
},
encoding: {
x: {
field: xField,
field: sanitizedXField,
...(xBand !== undefined ? { bandPosition: xBand } : {}),
...(xSort && xField ? { sort: domainValues?.[xField] } : {}),
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { describe, expect, it } from "vitest";
import type { CartesianChartSpec } from "../CartesianChartProvider";
import { at, chartData } from "../test-fixtures";
import { generateVLAreaChartSpec } from "./spec";

describe("generateVLAreaChartSpec with a dotted color dimension", () => {
it("escapes the dimension in the hover pivot", () => {
// Nested Druid columns surface as dimensions whose names contain dots,
// which Vega would otherwise read as a nested field path.
const dimension = "query.context.metaData.type";
const data = chartData();
data.fields[dimension] = { name: dimension, displayName: "Query Type" };
data.domainValues = {
...data.domainValues,
[dimension]: ["Scheduled Report", "Other"],
};

const config: CartesianChartSpec = {
metrics_view: "druid_requests",
x: { field: "__time", type: "temporal" },
y: { field: "count", type: "quantitative", zeroBasedOrigin: true },
color: { field: dimension, type: "nominal" },
};
const spec = generateVLAreaChartSpec(config, data);
expect(at(spec, "layer.1.transform.0.pivot")).toBe(
"query\\.context\\.metaData\\.type",
);
expect(at(spec, "layer.0.encoding.color.field")).toBe(
"query\\.context\\.metaData\\.type",
);
});
});

describe("generateVLAreaChartSpec with a dotted x dimension", () => {
it("escapes the x field once in the hover layer and keeps the custom sort", () => {
// The hover helper escapes the x field itself. Passing it pre-escaped
// escaped the backslash a second time, so the hover rule pointed at a
// field that does not exist and tooltips never showed.
const dimension = "event.time";
const data = chartData();
data.fields[dimension] = { name: dimension, displayName: "Event Time" };
data.domainValues = { [dimension]: ["Other", "Scheduled Report"] };

const spec = generateVLAreaChartSpec(
{
metrics_view: "druid_requests",
x: { field: dimension, type: "nominal", sort: "-y" },
y: { field: "count", type: "quantitative", zeroBasedOrigin: true },
},
data,
);
expect(at(spec, "layer.1.encoding.x.field")).toBe("event\\.time");
expect(at(spec, "layer.1.encoding.x.sort")).toEqual([
"Other",
"Scheduled Report",
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function generateVLAreaChartSpec(
layer: inner,
},
buildHoverRuleLayer({
xField: sanitizedXField,
xField: config.x?.field,
domainValues: data.domainValues,
defaultTooltip: defaultTooltipChannel,
multiValueTooltipChannel,
Expand All @@ -82,7 +82,7 @@ export function generateVLAreaChartSpec(
colorField &&
multiValueTooltipChannel?.length
? {
field: colorField,
field: sanitizeValueForVega(colorField),
value: sanitizedYField,
groupby: [sanitizedXField],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,50 @@ describe("generateVLStackedBarChartSpec orientation", () => {
expect(() => compile(spec as TopLevelSpec)).not.toThrow();
});
});

describe("generateVLBarChartSpec with a dotted color dimension", () => {
it("escapes the dimension in the hover pivot and the xOffset", () => {
// Nested Druid columns surface as dimensions whose names contain dots,
// which Vega would otherwise read as a nested field path.
const dimension = "query.context.metaData.type";
const data = chartData();
data.fields[dimension] = { name: dimension, displayName: "Query Type" };
data.domainValues = {
...data.domainValues,
[dimension]: ["Scheduled Report", "Other"],
};

const spec = generateVLBarChartSpec(
{ ...base, color: { field: dimension, type: "nominal" } },
data,
);
expect(at(spec, "layer.0.transform.0.pivot")).toBe(
"query\\.context\\.metaData\\.type",
);
expect(at(spec, "layer.1.encoding.xOffset.field")).toBe(
"query\\.context\\.metaData\\.type",
);
});
});

describe("generateVLBarChartSpec with a dotted x dimension", () => {
it("keeps the custom x sort values for a dotted x dimension", () => {
// The hover layer looks the sort values up by the raw name.
const dimension = "query.context.metaData.type";
const data = chartData();
data.fields[dimension] = { name: dimension, displayName: "Query Type" };
data.domainValues = { [dimension]: ["Other", "Scheduled Report"] };

const spec = generateVLBarChartSpec(
{ ...base, x: { field: dimension, type: "nominal", sort: "-y" } },
data,
);
expect(at(spec, "layer.0.encoding.x.field")).toBe(
"query\\.context\\.metaData\\.type",
);
expect(at(spec, "layer.0.encoding.x.sort")).toEqual([
"Other",
"Scheduled Report",
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export function generateVLBarChartSpec(
const isInteractive = !!config.isInteractive && !horizontal;

const hoverRuleLayer = buildHoverRuleLayer({
xField: sanitizedXField,
xField: config.x?.field,
domainValues: data.domainValues,
isBarMark: true,
defaultTooltip: defaultTooltipChannel,
Expand Down Expand Up @@ -104,7 +104,7 @@ export function generateVLBarChartSpec(
!hasComparison
? {
xOffset: {
field: config.color.field,
field: sanitizeValueForVega(config.color.field),
title:
data.fields[config.color.field]?.displayName ||
config.color.field,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function generateVLLineChartSpec(
const hasComparison = data.hasComparison;

const hoverRuleLayer = buildHoverRuleLayer({
xField: sanitizedXField,
xField: config.x?.field,
domainValues: data.domainValues,
defaultTooltip: defaultTooltipChannel,
multiValueTooltipChannel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,32 @@ describe("generateVLMultiMetricChartSpec orientation", () => {
}
});
});

describe("generateVLMultiMetricChartSpec with dotted measure names", () => {
it("escapes the folded measures and keeps the raw names as output columns", () => {
const measures = ["post.count", "comment.count"];
const data = chartData();
for (const measure of measures) {
data.fields[measure] = { name: measure, displayName: measure };
}

const spec = generateVLMultiMetricChartSpec(
{
...base,
y: {
field: measures[0],
fields: measures,
type: "quantitative",
zeroBasedOrigin: true,
},
},
data,
"grouped_bar",
);
expect(at(spec, "transform.0.fold")).toEqual([
"post\\.count",
"comment\\.count",
]);
expect(at(spec, "transform.0.as")).toEqual(["Measure", "value"]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function generateVLMultiMetricChartSpec(

const transforms: Transform[] = [
{
fold: fieldsToFold,
fold: fieldsToFold.map((field) => sanitizeValueForVega(field)),
as: [measureField, valueField],
},
];
Expand Down Expand Up @@ -242,7 +242,7 @@ export function generateVLMultiMetricChartSpec(
}

const hoverRuleLayer = buildHoverRuleLayer({
xField,
xField: config.x?.field,
defaultTooltip: [],
multiValueTooltipChannel,
primaryColor: data.theme.primary,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { sanitizeFieldName } from "@rilldata/web-common/components/vega/util";
import { ColorWithComparisonField } from "@rilldata/web-common/features/components/charts/comparison-builder";
import type { Scene } from "vega";
import type { TopLevelSpec } from "vega-lite";
import { describe, expect, it } from "vitest";
import type { CartesianChartSpec } from "../CartesianChartProvider";
import { at, chartData, stubCanvasContext } from "../test-fixtures";
import { generateVLStackedBarChartSpec } from "./default";

// A nested Druid column surfaces as a dimension whose name contains dots.
// Vega reads an unescaped dotted field as a nested lookup, so every place the
// spec names the dimension has to escape it or the chart renders nothing.
const dimension = "query.context.metaData.type";
const escapedDimension = "query\\.context\\.metaData\\.type";
const values = ["Scheduled Report", "Other"];

const config: CartesianChartSpec = {
metrics_view: "druid_requests",
x: { field: "__time", type: "temporal" },
y: { field: "count", type: "quantitative", zeroBasedOrigin: true },
color: { field: dimension, type: "nominal", values },
};

function data(hasComparison = false) {
const result = chartData({ hasComparison });
result.fields = {
__time: { name: "__time", displayName: "Time" },
count: { name: "count", displayName: "Count" },
[dimension]: { name: dimension, displayName: "Query Type" },
};
result.domainValues = { [dimension]: values };
return result;
}

describe("generateVLStackedBarChartSpec with a dotted color dimension", () => {
it("escapes the dimension in the pivot and color encodings", () => {
const spec = generateVLStackedBarChartSpec(config, data());
expect(at(spec, "layer.0.transform.0.pivot")).toBe(escapedDimension);
expect(at(spec, "layer.1.encoding.color.field")).toBe(escapedDimension);
});

it("pivots on the synthetic color field in comparison mode", () => {
const spec = generateVLStackedBarChartSpec(config, data(true));
expect(at(spec, "layer.0.transform.0.pivot")).toBe(
ColorWithComparisonField,
);
});

it("renders a bar per row when rows are keyed by the dotted dimension", async () => {
stubCanvasContext();
const { compile } = await import("vega-lite");
const vega = await import("vega");
// The app registers a formatter per measure for tooltips (see Chart.svelte).
vega.expressionFunction(sanitizeFieldName("count"), (value: unknown) =>
String(value),
);

const spec = generateVLStackedBarChartSpec(config, data());
const rows = [
{ __time: "2026-10-27T00:00:00Z", count: 2, [dimension]: values[0] },
{ __time: "2026-10-27T00:00:00Z", count: 3, [dimension]: values[1] },
{ __time: "2026-10-27T01:00:00Z", count: 4, [dimension]: values[0] },
{ __time: "2026-10-27T01:00:00Z", count: 1, [dimension]: values[1] },
];

// Vega reports dataflow failures through its logger instead of throwing.
const errors: unknown[] = [];
const view = new vega.View(vega.parse(compile(spec as TopLevelSpec).spec), {
renderer: "none",
})
.logger(
vega.logger(vega.Error, "error", (_method, _level, args) => {
errors.push(...Array.from(args as ArrayLike<unknown>));
}),
)
.width(400)
.height(200);
view.data("metrics-view", rows);
await view.runAsync();

expect(errors).toEqual([]);
// At runtime `scenegraph()` returns the Scenegraph wrapper, not the root mark.
const { root } = view.scenegraph() as unknown as { root: Scene };
expect(countBars(root)).toBe(rows.length);
});
});

/** Counts the painted bar marks, skipping the transparent hover targets. */
function countBars(mark: Scene): number {
const own =
mark.marktype === "rect"
? mark.items.filter(
(bar: { fill?: string; height?: number }) =>
bar.fill !== "transparent" && (bar.height ?? 0) > 0,
).length
: 0;
return mark.items
.flatMap((group) => group.items ?? [])
.reduce((sum, child) => sum + countBars(child), own);
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function generateVLStackedBarChartSpec(
const isInteractive = !!config.isInteractive && !horizontal;

const hoverRuleLayer = buildHoverRuleLayer({
xField: sanitizedXField,
xField: config.x?.field,
domainValues: data.domainValues,
isBarMark: true,
defaultTooltip: defaultTooltipChannel,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import type { TopLevelSpec } from "vega-lite";
import { describe, expect, it } from "vitest";
import type { CartesianChartSpec } from "../CartesianChartProvider";
import { at, chartData, stubCanvasContext } from "../test-fixtures";
import {
at,
chartData,
stubCanvasContext,
vegaFieldReferences,
} from "../test-fixtures";
import { generateVLStackedBarNormalizedSpec } from "./normalized";

const base: CartesianChartSpec = {
Expand Down Expand Up @@ -57,3 +62,25 @@ describe("generateVLStackedBarNormalizedSpec orientation", () => {
expect(() => compile(spec as TopLevelSpec)).not.toThrow();
});
});

describe("generateVLStackedBarNormalizedSpec with a dotted x dimension", () => {
it("escapes the dimension in the percentage groupby and the hover encoding", () => {
// Nested Druid columns surface as dimensions whose names contain dots,
// which Vega would otherwise read as a nested field path.
const dimension = "query.context.metaData.type";
const data = chartData({ colorValues: ["us", "eu"] });
data.fields[dimension] = { name: dimension, displayName: "Query Type" };

const spec = generateVLStackedBarNormalizedSpec(
{ ...base, x: { field: dimension, type: "nominal" } },
data,
);
expect(at(spec, "transform.0.groupby")).toEqual([
"query\\.context\\.metaData\\.type",
]);
expect(at(spec, "layer.0.encoding.x.field")).toBe(
"query\\.context\\.metaData\\.type",
);
expect(vegaFieldReferences(spec)).not.toContain(dimension);
});
});
Loading
Loading