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: 4 additions & 4 deletions app/src/app/(auth)/signup/__tests__/page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ describe("SignupPage", () => {
it("shows error when passwords do not match", async () => {
mockFetchBootstrapStatus(false, true);

const user = userEvent.setup();
const user = userEvent.setup({ delay: null });
render(<SignupPage />);

await waitFor(() => {
Expand All @@ -263,7 +263,7 @@ describe("SignupPage", () => {
error: "Email already registered",
});

const user = userEvent.setup();
const user = userEvent.setup({ delay: null });
render(<SignupPage />);

await waitFor(() => {
Expand All @@ -286,7 +286,7 @@ describe("SignupPage", () => {
mockSignup.mockResolvedValue({ success: true });
mockSignIn.mockResolvedValue({ error: null });

const user = userEvent.setup();
const user = userEvent.setup({ delay: null });
render(<SignupPage />);

await waitFor(() => {
Expand All @@ -309,7 +309,7 @@ describe("SignupPage", () => {
mockSignup.mockResolvedValue({ success: true });
mockSignIn.mockResolvedValue({ error: "some-error" });

const user = userEvent.setup();
const user = userEvent.setup({ delay: null });
render(<SignupPage />);

await waitFor(() => {
Expand Down
1 change: 1 addition & 0 deletions app/src/plugins/bar/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function BarPluginComponent({
<BarChart
data={(data as BarChartDataPoint[]) ?? []}
orientation={settings.orientation}
stackMode={settings.stackMode}
stacked={settings.stacked}
Comment on lines +36 to 37

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 | 🟠 Major | ⚡ Quick win

Preserve legacy fallback by not always forcing stackMode.

Line 36 currently passes a schema-defaulted value, which can mask “unset” and break legacy stacked: true behavior. Pass stackMode only when it was explicitly provided in raw settings.

Suggested fix
 function BarPluginComponent({
   data,
   settings: raw,
@@
 }: PluginProps) {
   const onClick = useEChartsClick(onChartClick, data);
   const settings = barSettingsSchema.parse(raw);
+  const hasExplicitStackMode =
+    raw != null &&
+    typeof raw === "object" &&
+    "stackMode" in (raw as Record<string, unknown>);
 
   return (
     <BarChart
       data={(data as BarChartDataPoint[]) ?? []}
       orientation={settings.orientation}
-      stackMode={settings.stackMode}
+      stackMode={hasExplicitStackMode ? settings.stackMode : undefined}
       stacked={settings.stacked}
       showValues={settings.showValues}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/plugins/bar/component.tsx` around lines 36 - 37, The component is
always passing the schema-defaulted settings.stackMode which masks “unset”;
change the JSX to only supply the stackMode prop when the raw/unprocessed
setting was explicitly provided (e.g., check rawSettings.stackMode !==
undefined) and otherwise omit the prop, while keeping stacked={settings.stacked}
unchanged; in short, replace the unconditional stackMode={settings.stackMode}
with a conditional spread/prop that only sets stackMode when the original raw
setting exists.

showValues={settings.showValues}
showLegend={settings.showLegend}
Expand Down
2 changes: 2 additions & 0 deletions app/src/plugins/bar/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { z } from "zod";
export const barSettingsSchema = z
.object({
orientation: z.enum(["vertical", "horizontal"]).default("vertical"),
stackMode: z.enum(["none", "stacked", "percent"]).default("none"),
/** @deprecated Use stackMode instead. Kept for backward compatibility. */
stacked: z.boolean().default(false),
showValues: z.boolean().default(false),
showLegend: z.boolean().default(true),
Expand Down
178 changes: 118 additions & 60 deletions component/src/charts/__tests__/bar-chart.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { render, screen } from "@testing-library/react";
import { describe, it, expect, vi, beforeEach } from "vitest";
import type { BarChartProps } from "../bar-chart";
import { BarChart } from "../bar-chart";

// echarts/charts, echarts/components, echarts/renderers are mocked globally
Expand All @@ -21,6 +22,12 @@ vi.mock("echarts/core", () => {
return { use, init, registerTheme, default: { use, init, registerTheme } };
});

/** Render BarChart and return the ECharts options passed to setOption. */
function renderBarOptions(props: BarChartProps) {
render(<BarChart {...props} />);
return mockSetOption.mock.calls[0][0];
}

const sampleData = [
{ label: "Product A", value: 100 },
{ label: "Product B", value: 200 },
Expand Down Expand Up @@ -49,43 +56,40 @@ describe("BarChart", () => {
});

it("builds bar series from data", () => {
render(<BarChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series).toHaveLength(1);
expect(optionsCall.series[0].type).toBe("bar");
expect(optionsCall.series[0].data).toEqual([100, 200, 150]);
const opts = renderBarOptions({ data: sampleData });
expect(opts.series).toHaveLength(1);
expect(opts.series[0].type).toBe("bar");
expect(opts.series[0].data).toEqual([100, 200, 150]);
});

it("supports horizontal orientation", () => {
render(<BarChart data={sampleData} orientation="horizontal" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.xAxis.type).toBe("value");
expect(optionsCall.yAxis.type).toBe("category");
const opts = renderBarOptions({
data: sampleData,
orientation: "horizontal",
});
expect(opts.xAxis.type).toBe("value");
expect(opts.yAxis.type).toBe("category");
});

it("supports stacked bars", () => {
render(<BarChart data={stackedData} stacked />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].stack).toBe("total");
expect(optionsCall.series[1].stack).toBe("total");
const opts = renderBarOptions({ data: stackedData, stacked: true });
expect(opts.series[0].stack).toBe("total");
expect(opts.series[1].stack).toBe("total");
});

it("shows values on bars", () => {
render(<BarChart data={sampleData} showValues />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].label.show).toBe(true);
const opts = renderBarOptions({ data: sampleData, showValues: true });
expect(opts.series[0].label.show).toBe(true);
});

it("shows legend for multiple series", () => {
render(<BarChart data={stackedData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.legend).toBeDefined();
const opts = renderBarOptions({ data: stackedData });
expect(opts.legend).toBeDefined();
});

it("handles empty data", () => {
render(<BarChart data={[]} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.title.text).toBe("No data");
const opts = renderBarOptions({ data: [] });
expect(opts.title.text).toBe("No data");
});

it("shows loading state", () => {
Expand All @@ -101,78 +105,132 @@ describe("BarChart", () => {
// --- New options ---

it("sets barWidth on series when provided and > 0", () => {
render(<BarChart data={sampleData} barWidth={20} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].barWidth).toBe(20);
const opts = renderBarOptions({ data: sampleData, barWidth: 20 });
expect(opts.series[0].barWidth).toBe(20);
});

it("sets barWidth to undefined when barWidth is 0 (auto)", () => {
render(<BarChart data={sampleData} barWidth={0} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].barWidth).toBeUndefined();
const opts = renderBarOptions({ data: sampleData, barWidth: 0 });
expect(opts.series[0].barWidth).toBeUndefined();
});

it("passes barGap to series", () => {
render(<BarChart data={sampleData} barGap="10%" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].barGap).toBe("10%");
const opts = renderBarOptions({ data: sampleData, barGap: "10%" });
expect(opts.series[0].barGap).toBe("10%");
});

it("shows grid lines by default", () => {
render(<BarChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.yAxis.splitLine.show).toBe(true);
const opts = renderBarOptions({ data: sampleData });
expect(opts.yAxis.splitLine.show).toBe(true);
});

it("hides grid lines when showGridLines is false", () => {
render(<BarChart data={sampleData} showGridLines={false} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.yAxis.splitLine.show).toBe(false);
const opts = renderBarOptions({ data: sampleData, showGridLines: false });
expect(opts.yAxis.splitLine.show).toBe(false);
});

it("sets xAxisLabel on the category axis for vertical orientation", () => {
render(<BarChart data={sampleData} xAxisLabel="Product" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.xAxis.name).toBe("Product");
const opts = renderBarOptions({ data: sampleData, xAxisLabel: "Product" });
expect(opts.xAxis.name).toBe("Product");
});

it("sets yAxisLabel on the value axis for vertical orientation", () => {
render(<BarChart data={sampleData} yAxisLabel="Revenue" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.yAxis.name).toBe("Revenue");
const opts = renderBarOptions({ data: sampleData, yAxisLabel: "Revenue" });
expect(opts.yAxis.name).toBe("Revenue");
});

it("swaps axis label targets for horizontal orientation", () => {
render(<BarChart data={sampleData} orientation="horizontal" xAxisLabel="Revenue" yAxisLabel="Product" />);
const optionsCall = mockSetOption.mock.calls[0][0];
const opts = renderBarOptions({
data: sampleData,
orientation: "horizontal",
xAxisLabel: "Revenue",
yAxisLabel: "Product",
});
// xAxisLabel goes to the value axis (xAxis in horizontal), yAxisLabel to category (yAxis)
expect(optionsCall.xAxis.name).toBe("Revenue");
expect(optionsCall.yAxis.name).toBe("Product");
expect(opts.xAxis.name).toBe("Revenue");
expect(opts.yAxis.name).toBe("Product");
});

// --- Reference lines (markLine) ---

it("attaches markLine to the first series when referenceLines is provided", () => {
const refs = JSON.stringify([{ value: 150, label: "Target", color: "#ff0000" }]);
render(<BarChart data={sampleData} referenceLines={refs} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].markLine).toBeDefined();
expect(optionsCall.series[0].markLine.data).toHaveLength(1);
expect(optionsCall.series[0].markLine.data[0].yAxis).toBe(150);
const refs = JSON.stringify([
{ value: 150, label: "Target", color: "#ff0000" },
]);
const opts = renderBarOptions({ data: sampleData, referenceLines: refs });
expect(opts.series[0].markLine).toBeDefined();
expect(opts.series[0].markLine.data).toHaveLength(1);
expect(opts.series[0].markLine.data[0].yAxis).toBe(150);
});

it("does not attach markLine when referenceLines is not provided", () => {
render(<BarChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].markLine).toBeUndefined();
const opts = renderBarOptions({ data: sampleData });
expect(opts.series[0].markLine).toBeUndefined();
});

// --- DataZoom ---

it("passes enableDataZoom to BaseChart", () => {
render(<BarChart data={sampleData} enableDataZoom />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.dataZoom).toBeDefined();
expect(optionsCall.dataZoom.length).toBeGreaterThan(0);
const opts = renderBarOptions({ data: sampleData, enableDataZoom: true });
expect(opts.dataZoom).toBeDefined();
expect(opts.dataZoom.length).toBeGreaterThan(0);
});

// --- Percentage stacked ---

it("normalizes values to percentages when stackMode is percent", () => {
const opts = renderBarOptions({
data: stackedData,
stackMode: "percent",
});
// Q1: sales=100, returns=20 -> total=120 -> sales=83.33%, returns=16.67%
expect(opts.series[0].data[0]).toBeCloseTo(83.33, 1);
expect(opts.series[1].data[0]).toBeCloseTo(16.67, 1);
});

it("sets y-axis max to 100 in percent mode", () => {
const opts = renderBarOptions({
data: stackedData,
stackMode: "percent",
});
expect(opts.yAxis.max).toBe(100);
});

it("stacks series in percent mode", () => {
const opts = renderBarOptions({
data: stackedData,
stackMode: "percent",
});
expect(opts.series[0].stack).toBe("total");
expect(opts.series[1].stack).toBe("total");
});

it("stacks series in stacked mode (backward compat)", () => {
const opts = renderBarOptions({
data: stackedData,
stackMode: "stacked",
});
expect(opts.series[0].stack).toBe("total");
});

it("does not stack in none mode", () => {
const opts = renderBarOptions({ data: stackedData, stackMode: "none" });
expect(opts.series[0].stack).toBeUndefined();
});

it("backward compat: stacked boolean still works", () => {
const opts = renderBarOptions({ data: stackedData, stacked: true });
expect(opts.series[0].stack).toBe("total");
});

it("handles zero total gracefully in percent mode", () => {
const zeroData = [
{ label: "A", x: 0, y: 0 },
{ label: "B", x: 10, y: 20 },
];
const opts = renderBarOptions({ data: zeroData, stackMode: "percent" });
// Zero total row: all values should be 0 (not NaN)
expect(opts.series[0].data[0]).toBe(0);
expect(opts.series[1].data[0]).toBe(0);
});
});
Loading
Loading