Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
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
69 changes: 67 additions & 2 deletions component/src/charts/__tests__/bar-chart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,14 @@ describe("BarChart", () => {
});

it("swaps axis label targets for horizontal orientation", () => {
render(<BarChart data={sampleData} orientation="horizontal" xAxisLabel="Revenue" yAxisLabel="Product" />);
render(
<BarChart
data={sampleData}
orientation="horizontal"
xAxisLabel="Revenue"
yAxisLabel="Product"
/>,
);
const optionsCall = mockSetOption.mock.calls[0][0];
// xAxisLabel goes to the value axis (xAxis in horizontal), yAxisLabel to category (yAxis)
expect(optionsCall.xAxis.name).toBe("Revenue");
Expand All @@ -153,7 +160,9 @@ describe("BarChart", () => {
// --- Reference lines (markLine) ---

it("attaches markLine to the first series when referenceLines is provided", () => {
const refs = JSON.stringify([{ value: 150, label: "Target", color: "#ff0000" }]);
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();
Expand All @@ -175,4 +184,60 @@ describe("BarChart", () => {
expect(optionsCall.dataZoom).toBeDefined();
expect(optionsCall.dataZoom.length).toBeGreaterThan(0);
});

// --- Percentage stacked ---

it("normalizes values to percentages when stackMode is percent", () => {
render(<BarChart data={stackedData} stackMode="percent" />);
const optionsCall = mockSetOption.mock.calls[0][0];
// Q1: sales=100, returns=20 → total=120 → sales=83.33%, returns=16.67%
const salesSeries = optionsCall.series[0];
const returnsSeries = optionsCall.series[1];
expect(salesSeries.data[0]).toBeCloseTo(83.33, 1);
expect(returnsSeries.data[0]).toBeCloseTo(16.67, 1);
});

it("sets y-axis max to 100 and formats as percent in percent mode", () => {
render(<BarChart data={stackedData} stackMode="percent" />);
const optionsCall = mockSetOption.mock.calls[0][0];
const valueAxis = optionsCall.yAxis;
expect(valueAxis.max).toBe(100);
});

it("stacks series in percent mode", () => {
render(<BarChart data={stackedData} stackMode="percent" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].stack).toBe("total");
expect(optionsCall.series[1].stack).toBe("total");
});

it("stacks series in stacked mode (backward compat)", () => {
render(<BarChart data={stackedData} stackMode="stacked" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].stack).toBe("total");
});

it("does not stack in none mode", () => {
render(<BarChart data={stackedData} stackMode="none" />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].stack).toBeUndefined();
});

it("backward compat: stacked boolean still works", () => {
render(<BarChart data={stackedData} stacked />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.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 },
];
render(<BarChart data={zeroData} stackMode="percent" />);
const optionsCall = mockSetOption.mock.calls[0][0];
// Zero total row: all values should be 0 (not NaN)
expect(optionsCall.series[0].data[0]).toBe(0);
expect(optionsCall.series[1].data[0]).toBe(0);
});
});
87 changes: 80 additions & 7 deletions component/src/charts/bar-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@
import { parseColorThresholds } from "./color-threshold";
import type { StylingRule } from "./styling-rule";

export type BarStackMode = "none" | "stacked" | "percent";

export interface BarChartProps extends Omit<BaseChartProps, "options"> {
/** Array of data points. Each object has a `label` key and one or more numeric series keys. */
data: BarChartDataPoint[];
/** Bar orientation */
orientation?: "vertical" | "horizontal";
/** Stack bars when multiple series */
/** Stack mode: none (grouped), stacked (absolute), percent (100% stacked) */
stackMode?: BarStackMode;
/** @deprecated Use stackMode instead. Stack bars when multiple series */
stacked?: boolean;
/** Show values on bars */
showValues?: boolean;
Expand Down Expand Up @@ -62,7 +66,8 @@
function BarChart({
data,
orientation = "vertical",
stackMode: stackModeProp,
stacked = false,

Check warning on line 70 in component/src/charts/bar-chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'stacked' is deprecated.

See more on https://sonarcloud.io/project/issues?id=alfredo1996_neoboard&issues=AZ4dDKcdVsBJTA8SUUtZ&open=AZ4dDKcdVsBJTA8SUUtZ&pullRequest=724
showValues = false,
showLegend,
barWidth = 0,
Expand All @@ -80,10 +85,26 @@
const { width, height, containerRef } = useContainerSize();
const { compact, hideLegend } = getCompactState(width, height);

// Resolve stack mode: prefer explicit stackMode, fall back to legacy boolean
const stackMode: BarStackMode =
stackModeProp ?? (stacked ? "stacked" : "none");
const isPercent = stackMode === "percent";
const isStacked = stackMode === "stacked" || isPercent;

const options = useMemo((): EChartsOption => {

Check failure on line 94 in component/src/charts/bar-chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 21 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=alfredo1996_neoboard&issues=AZ4dDKcdVsBJTA8SUUta&open=AZ4dDKcdVsBJTA8SUUta&pullRequest=724
if (!data.length) return buildEmptyDataOption();

const seriesKeys = Object.keys(data[0]).filter((k) => k !== "label");

// Pre-compute row totals for percentage normalization
const rowTotals = isPercent
? data.map((d) =>
seriesKeys.reduce((sum, key) => {
const v = Number(d[key]);
return sum + (Number.isFinite(v) ? Math.abs(v) : 0);
}, 0),
)
: [];
const effectiveShowLegend = resolveShowLegend(
showLegend,
seriesKeys.length,
Expand Down Expand Up @@ -116,18 +137,56 @@
};
const valueAxis = {
type: "value" as const,
axisLabel: { show: !compact },
axisLabel: {
show: !compact,
...(isPercent ? { formatter: "{value}%" } : {}),
},
splitLine: { show: showGridLines },
name: compact ? undefined : isHorizontal ? xAxisLabel : yAxisLabel,
nameLocation: "middle" as const,
nameGap: 50,
...(isPercent ? { max: 100 } : {}),
};

// In percent mode, build a tooltip showing "pct% (absolute)"
const percentTooltipFormatter = isPercent
? (params: unknown) => {
const items = Array.isArray(params)
? (params as {
seriesName?: string;
name?: string;
value?: number;
marker?: string;
dataIndex?: number;
}[])
: [
params as {
seriesName?: string;
name?: string;
value?: number;
marker?: string;
dataIndex?: number;
},
];
const header = items[0]?.name ?? "";
const lines = items.map((p) => {
const pct =
typeof p.value === "number" ? p.value.toFixed(1) : p.value;
const rowIdx = p.dataIndex ?? 0;
const seriesKey =
seriesKeys.find((k) => k === p.seriesName) ?? seriesKeys[0];

Check failure on line 177 in component/src/charts/bar-chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this code to not nest functions more than 4 levels deep.

See more on https://sonarcloud.io/project/issues?id=alfredo1996_neoboard&issues=AZ4dDKcdVsBJTA8SUUtb&open=AZ4dDKcdVsBJTA8SUUtb&pullRequest=724
const absValue = seriesKey ? data[rowIdx]?.[seriesKey] : "";
return `${p.marker ?? ""} ${p.seriesName}: ${pct}% (${absValue})`;
});
return `<strong>${header}</strong><br/>${lines.join("<br/>")}`;
}
: undefined;

return {
tooltip: {
trigger: "axis" as const,
axisPointer: { type: "shadow" as const },
formatter: buildTooltipFormatter(),
formatter: percentTooltipFormatter ?? buildTooltipFormatter(),
},
legend: effectiveShowLegend ? { bottom: 0 } : undefined,
grid: buildCompactGrid(compact, effectiveShowLegend),
Expand All @@ -136,10 +195,21 @@
series: seriesKeys.map((key, idx) => ({
name: key,
type: "bar" as const,
data: data.map((d) => {
data: data.map((d, rowIdx) => {
const rawValue = d[key];
const numericValue =
typeof rawValue === "number" ? rawValue : Number(rawValue);

// In percent mode, normalize to percentage of row total
let displayValue = rawValue as number | string;

Check warning on line 204 in component/src/charts/bar-chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=alfredo1996_neoboard&issues=AZ4dcf-qqzxeJOEKvPu4&open=AZ4dcf-qqzxeJOEKvPu4&pullRequest=724
if (isPercent) {
const total = rowTotals[rowIdx];
displayValue =
total > 0 && Number.isFinite(numericValue)
? Math.round((numericValue / total) * 10000) / 100
: 0;
}

const color = Number.isFinite(numericValue)
? resolveItemColor(
numericValue,
Expand All @@ -148,9 +218,11 @@
thresholds,
)
: undefined;
return color ? { value: rawValue, itemStyle: { color } } : rawValue;
return color
? { value: displayValue, itemStyle: { color } }
: displayValue;
}),
stack: stacked ? "total" : undefined,
stack: isStacked ? "total" : undefined,
barWidth: effectiveBarWidth,
barGap,
label: effectiveShowValues
Expand All @@ -167,7 +239,8 @@
}, [
data,
orientation,
stacked,
isStacked,
isPercent,
showValues,
showLegend,
barWidth,
Expand Down
2 changes: 1 addition & 1 deletion component/src/charts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export {
export { LineChart } from "./line-chart";
export type { LineChartProps } from "./line-chart";
export { BarChart } from "./bar-chart";
export type { BarChartProps } from "./bar-chart";
export type { BarChartProps, BarStackMode } from "./bar-chart";
export { PieChart } from "./pie-chart";
export type { PieChartProps } from "./pie-chart";
export { SingleValueChart } from "./single-value-chart";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe("ChartOptionsPanel", () => {
);
expandAllCategories();
expect(screen.getByText("Orientation")).toBeInTheDocument();
expect(screen.getByText("Stacked")).toBeInTheDocument();
expect(screen.getByText("Stack Mode")).toBeInTheDocument();
expect(screen.getByText("Show Values")).toBeInTheDocument();
expect(screen.getByText("Show Legend")).toBeInTheDocument();
expect(screen.getByText("Bar Width (px, 0=auto)")).toBeInTheDocument();
Expand All @@ -49,14 +49,15 @@ describe("ChartOptionsPanel", () => {
render(
<ChartOptionsPanel
chartType="bar"
settings={{ stacked: false }}
settings={{ showValues: false }}
onSettingsChange={onChange}
/>,
);
const switchEl = screen.getByRole("switch", { name: "Stacked" });
expandAllCategories();
const switchEl = screen.getByRole("switch", { name: "Show Values" });
fireEvent.click(switchEl);
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({ stacked: true }),
expect.objectContaining({ showValues: true }),
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe("getChartOptions", () => {
const options = getChartOptions("bar");
const keys = options.map((o) => o.key);
expect(keys).toContain("orientation");
expect(keys).toContain("stacked");
expect(keys).toContain("stackMode");
expect(keys).toContain("showValues");
expect(keys).toContain("showLegend");
});
Expand Down Expand Up @@ -160,7 +160,7 @@ describe("getDefaultChartSettings", () => {
it("returns correct defaults for bar chart", () => {
const d = getDefaultChartSettings("bar");
expect(d.orientation).toBe("vertical");
expect(d.stacked).toBe(false);
expect(d.stackMode).toBe("none");
expect(d.showValues).toBe(false);
expect(d.showLegend).toBe(true);
});
Expand Down
15 changes: 10 additions & 5 deletions component/src/components/composed/chart-options/bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ export const barOptions: ChartOptionDef[] = [
],
},
{
key: "stacked",
label: "Stacked",
type: "boolean",
default: false,
key: "stackMode",
label: "Stack Mode",
type: "select",
default: "none",
category: "Layout",
description:
"Stack series on top of each other instead of placing them side by side.",
"How to arrange multiple series: side by side, stacked, or 100% stacked (percentage).",
options: [
{ label: "Normal (grouped)", value: "none" },
{ label: "Stacked", value: "stacked" },
{ label: "100% Stacked", value: "percent" },
],
},
{
key: "barWidth",
Expand Down
Loading