Skip to content
Closed
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
68 changes: 68 additions & 0 deletions component/src/charts/__tests__/axis-label-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { describe, it, expect } from "vitest";
import { buildCategoryAxisLabel } from "../chart-utils";

describe("buildCategoryAxisLabel", () => {
it("returns default config for small category count", () => {
const result = buildCategoryAxisLabel(5);
expect(result.rotate).toBe(0);
expect(result.formatter).toBeUndefined();
});

it("rotates labels at 30° when categories >= 8", () => {
const result = buildCategoryAxisLabel(8);
expect(result.rotate).toBe(30);
});

it("rotates labels at 45° when categories >= 15", () => {
const result = buildCategoryAxisLabel(15);
expect(result.rotate).toBe(45);
});

it("truncates labels longer than 15 chars with ellipsis", () => {
const result = buildCategoryAxisLabel(10);
expect(result.formatter).toBeDefined();
const fmt = result.formatter as (value: string) => string;
expect(fmt("Short")).toBe("Short");
expect(fmt("This is a very long label text")).toBe("This is a very\u2026");
});

it("respects custom maxLength", () => {
const result = buildCategoryAxisLabel(10, { maxLabelLength: 8 });
const fmt = result.formatter as (value: string) => string;
expect(fmt("12345678")).toBe("12345678");
expect(fmt("123456789")).toBe("1234567\u2026");
});

it("respects rotation override", () => {
const result = buildCategoryAxisLabel(100, { rotateOverride: 60 });
expect(result.rotate).toBe(60);
});

it("returns rotate 0 with override of 0", () => {
const result = buildCategoryAxisLabel(20, { rotateOverride: 0 });
expect(result.rotate).toBe(0);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it("always includes tooltip config for full text", () => {
const result = buildCategoryAxisLabel(10);
expect(result.tooltip).toEqual({ show: true });
});

it("returns show: false when compact is true", () => {
const result = buildCategoryAxisLabel(10, { compact: true });
expect(result.show).toBe(false);
});

it("normalizes -1 sentinel to automatic rotation", () => {
// -1 is the "automatic" sentinel from the UI; it should fall through
// to the category-count heuristic, not produce rotate: -1
const few = buildCategoryAxisLabel(5, { rotateOverride: -1 });
expect(few.rotate).toBe(0);

const medium = buildCategoryAxisLabel(10, { rotateOverride: -1 });
expect(medium.rotate).toBe(30);

const many = buildCategoryAxisLabel(20, { rotateOverride: -1 });
expect(many.rotate).toBe(45);
});
});
2 changes: 2 additions & 0 deletions component/src/charts/__tests__/base-chart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ vi.mock("echarts/components", () => ({
DataZoomComponent: vi.fn(),
AriaComponent: vi.fn(),
RadarComponent: vi.fn(),
MarkLineComponent: vi.fn(),
GraphicComponent: vi.fn(),
}));

describe("BaseChart", () => {
Expand Down
19 changes: 15 additions & 4 deletions component/src/charts/bar-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
resolveShowLegend,
buildCompactGrid,
resolveItemColor,
buildCategoryAxisLabel,
} from "./chart-utils";
import { parseColorThresholds } from "./color-threshold";
import type { StylingRule } from "./styling-rule";
Expand All @@ -34,6 +35,8 @@ export interface BarChartProps extends Omit<BaseChartProps, "options"> {
xAxisLabel?: string;
/** Y-axis name label */
yAxisLabel?: string;
/** Override axis label rotation angle (0-90). Omit for automatic. */
axisLabelRotation?: number;
/** @deprecated Use stylingRules instead. JSON string of thresholds for per-bar coloring */
colorThresholds?: string;
/** Rule-based styling rules */
Expand Down Expand Up @@ -62,6 +65,7 @@ function BarChart({
showGridLines = true,
xAxisLabel,
yAxisLabel,
axisLabelRotation,
colorThresholds,
stylingRules,
paramValues,
Expand All @@ -80,13 +84,20 @@ function BarChart({
const effectiveBarWidth = barWidth > 0 ? barWidth : undefined;
const thresholds = stylingRules ? [] : parseColorThresholds(colorThresholds ?? "");

const categoryLabels = data.map((d) => d.label);
const axisLabelConfig = buildCategoryAxisLabel(categoryLabels.length, {
compact,
rotateOverride: axisLabelRotation,
});
Comment on lines +88 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Sentinel value -1 not normalized before passing to utility.

When axisLabelRotation is -1 (the schema default for "automatic"), it's passed directly to buildCategoryAxisLabel as rotateOverride. The utility treats any defined value as an explicit override, resulting in rotate: -1 — an invalid ECharts angle.

Normalize -1 to undefined before passing:

🐛 Proposed fix
     const axisLabelConfig = buildCategoryAxisLabel(categoryLabels.length, {
       compact,
-      rotateOverride: axisLabelRotation,
+      rotateOverride: axisLabelRotation === -1 ? undefined : axisLabelRotation,
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const axisLabelConfig = buildCategoryAxisLabel(categoryLabels.length, {
compact,
rotateOverride: axisLabelRotation,
});
const axisLabelConfig = buildCategoryAxisLabel(categoryLabels.length, {
compact,
rotateOverride: axisLabelRotation === -1 ? undefined : axisLabelRotation,
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@component/src/charts/bar-chart.tsx` around lines 88 - 91, axisLabelRotation
may be the sentinel -1 (meaning "automatic") but is passed directly as
rotateOverride to buildCategoryAxisLabel causing an invalid angle; normalize
axisLabelRotation to undefined when it equals -1 before calling
buildCategoryAxisLabel (e.g., compute a local rotateOverride = axisLabelRotation
=== -1 ? undefined : axisLabelRotation and pass that) so buildCategoryAxisLabel
and axisLabelConfig receive undefined for automatic mode rather than -1.


const categoryAxis = {
type: "category" as const,
data: data.map((d) => d.label),
axisLabel: { show: !compact },
data: categoryLabels,
axisLabel: axisLabelConfig,
axisPointer: { type: "shadow" as const },
name: compact ? undefined : (isHorizontal ? yAxisLabel : xAxisLabel),
nameLocation: "middle" as const,
nameGap: 30,
nameGap: axisLabelConfig.rotate > 0 ? 50 : 30,
};
const valueAxis = {
type: "value" as const,
Expand Down Expand Up @@ -123,7 +134,7 @@ function BarChart({
emphasis: seriesKeys.length > 1 ? { focus: "series" as const } : {},
})),
};
}, [data, orientation, stacked, showValues, showLegend, barWidth, barGap, showGridLines, xAxisLabel, yAxisLabel, colorThresholds, stylingRules, paramValues, compact, hideLegend]);
}, [data, orientation, stacked, showValues, showLegend, barWidth, barGap, showGridLines, xAxisLabel, yAxisLabel, axisLabelRotation, colorThresholds, stylingRules, paramValues, compact, hideLegend]);

return (
<div ref={containerRef} className="h-full w-full">
Expand Down
4 changes: 4 additions & 0 deletions component/src/charts/base-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
DataZoomComponent,
AriaComponent,
RadarComponent,
MarkLineComponent,
GraphicComponent,
} from "echarts/components";
import { CanvasRenderer } from "echarts/renderers";
import type { EChartsOption } from "echarts";
Expand All @@ -35,6 +37,8 @@ echarts.use([
DataZoomComponent,
AriaComponent,
RadarComponent,
MarkLineComponent,
GraphicComponent,
CanvasRenderer,
]);

Expand Down
66 changes: 66 additions & 0 deletions component/src/charts/chart-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,72 @@ import { resolveThresholdColor } from "./color-threshold";
import type { StylingRule } from "./styling-rule";
import { resolveStylingRuleColor } from "./styling-rule";

// ---------------------------------------------------------------------------
// Axis label auto-rotation and truncation
// ---------------------------------------------------------------------------

export interface CategoryAxisLabelOptions {
/** Override the automatic rotation angle. -1 means automatic (sentinel). */
rotateOverride?: number;
/** Maximum label length before truncation (default: 15). */
maxLabelLength?: number;
/** Whether the chart is in compact mode (hides labels). */
compact?: boolean;
}

export interface CategoryAxisLabelConfig {
show: boolean;
rotate: number;
formatter?: (value: string) => string;
tooltip: { show: boolean };
}

/**
* Compute axis label rotation and truncation based on category count.
* - 8+ categories: rotate 30°
* - 15+ categories: rotate 45°
* - Labels longer than maxLabelLength are truncated with ellipsis (U+2026)
* - ECharts axisPointer tooltip shows the full text on hover
*
* A `rotateOverride` of -1 is the "automatic" sentinel from the UI and is
* normalized to undefined so the category-count heuristic applies.
*/
export function buildCategoryAxisLabel(
categoryCount: number,
options: CategoryAxisLabelOptions = {},
): CategoryAxisLabelConfig {
const { maxLabelLength = 15, compact = false } = options;
// Normalize -1 sentinel (automatic mode) to undefined so ECharts uses its
// default auto-rotation instead of receiving an invalid rotate: -1.
const rotateOverride = options.rotateOverride === -1 ? undefined : options.rotateOverride;

let rotate: number;
if (rotateOverride !== undefined) {
rotate = rotateOverride;
} else if (categoryCount >= 15) {
rotate = 45;
} else if (categoryCount >= 8) {
rotate = 30;
} else {
rotate = 0;
}

const needsTruncation = categoryCount >= 8;
const formatter = needsTruncation
? (value: string) =>
value.length > maxLabelLength
? value.slice(0, maxLabelLength - 1) + "\u2026"
: value
: undefined;

return {
show: !compact,
rotate,
formatter,
tooltip: { show: true },
};
}

/** Detect whether the document is currently in dark mode. */
export function isDark(): boolean {
if (typeof document === "undefined") return false;
Expand Down
1 change: 1 addition & 0 deletions component/src/components/composed/chart-options-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const barOptions: ChartOptionDef[] = [
{ key: "xAxisLabel", label: "X-Axis Label", type: "text", default: "", category: "Labels", description: "Custom label displayed below the horizontal axis." },
{ key: "yAxisLabel", label: "Y-Axis Label", type: "text", default: "", category: "Labels", description: "Custom label displayed beside the vertical axis." },
{ key: "showGridLines", label: "Show Grid Lines", type: "boolean", default: true, category: "Style", description: "Show faint horizontal reference lines behind the bars." },
{ key: "axisLabelRotation", label: "Axis Label Rotation (°)", type: "number", default: -1, category: "Labels", description: "Override axis label rotation angle (0-90). Set to -1 for automatic (rotates at 8+ categories)." },
];

const lineOptions: ChartOptionDef[] = [
Expand Down
2 changes: 2 additions & 0 deletions component/vitest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ vi.mock("echarts/components", () => ({
DataZoomComponent: vi.fn(),
AriaComponent: vi.fn(),
RadarComponent: vi.fn(),
MarkLineComponent: vi.fn(),
GraphicComponent: vi.fn(),
}));

vi.mock("echarts/renderers", () => ({
Expand Down
Loading