Skip to content

Commit

Permalink
[FIX] chart: trend line legend is wrong
Browse files Browse the repository at this point in the history
The trend line legend is a circle instead of a line in scatter charts.
And it's a rectangle in bar chart.

Also the onClick of the legend was slightly wrong: we used the index
of the legend item to check which dataset to hide. Which didn't
work if for whatever reason ChartJs decided to change the order of
the legend items.

closes #5226

Task: 4342471
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
  • Loading branch information
hokolomopo authored and LucasLefevre committed Dec 3, 2024
1 parent 80f430a commit 873498c
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 17 deletions.
49 changes: 32 additions & 17 deletions src/helpers/figures/charts/runtime/chartjs_legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import { ComboChartDefinition } from "../../../../types/chart/combo_chart";
import { RadarChartDefinition } from "../../../../types/chart/radar_chart";
import { ColorGenerator } from "../../../color";
import { chartFontColor, getPieColors } from "../chart_common";
import { TREND_LINE_XAXIS_ID, chartFontColor, getPieColors } from "../chart_common";

type ChartLegend = DeepPartial<LegendOptions<any>>;

Expand Down Expand Up @@ -96,11 +96,12 @@ export function getScatterChartLegend(
return {
...INTERACTIVE_LEGEND_CONFIG,
...getLegendDisplayOptions(definition, args),
labels: {
color: chartFontColor(definition.background),
boxHeight: 6,
usePointStyle: true,
},
...getCustomLegendLabels(chartFontColor(definition.background), {
pointStyle: "circle",
// the stroke is the border around the circle, so increasing its size with the chart's color reduce the size of the circle
strokeStyle: definition.background || "#ffffff",
lineWidth: 8,
}),
};
}

Expand Down Expand Up @@ -203,10 +204,10 @@ export const INTERACTIVE_LEGEND_CONFIG = {
target.style.cursor = "default";
},
onClick: (event, legendItem, legend) => {
if (!legend.legendItems) {
const index = legendItem.datasetIndex;
if (!legend.legendItems || index === undefined) {
return;
}
const index = legend.legendItems.indexOf(legendItem);
if (legend.chart.isDatasetVisible(index)) {
legend.chart.hide(index);
} else {
Expand All @@ -232,15 +233,29 @@ function getCustomLegendLabels(
color: fontColor,
usePointStyle: true,
generateLabels: (chart: Chart) =>
chart.data.datasets.map((dataset, index) => ({
text: dataset.label ?? "",
fontColor,
strokeStyle: dataset.borderColor as Color,
fillStyle: dataset.backgroundColor as Color,
hidden: !chart.isDatasetVisible(index),
pointStyle: dataset.type === "line" ? "line" : "rect",
...legendLabelConfig,
})),
chart.data.datasets.map((dataset, index) => {
if (dataset["xAxisID"] === TREND_LINE_XAXIS_ID) {
return {
text: dataset.label ?? "",
fontColor,
strokeStyle: dataset.borderColor as Color,
hidden: !chart.isDatasetVisible(index),
pointStyle: "line",
datasetIndex: index,
lineWidth: 3,
};
}
return {
text: dataset.label ?? "",
fontColor,
strokeStyle: dataset.borderColor as Color,
fillStyle: dataset.backgroundColor as Color,
hidden: !chart.isDatasetVisible(index),
pointStyle: dataset.type === "line" ? "line" : "rect",
datasetIndex: index,
...legendLabelConfig,
};
}),
},
};
}
26 changes: 26 additions & 0 deletions tests/figures/chart/bar_chart_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ describe("bar chart", () => {
lineWidth: 3,
pointStyle: "rect",
strokeStyle: "#FFFFFF",
datasetIndex: 0,
},
{
fontColor: "#000000",
Expand All @@ -172,6 +173,7 @@ describe("bar chart", () => {
lineWidth: 3,
pointStyle: "rect",
strokeStyle: "#FFFFFF",
datasetIndex: 1,
},
]);
});
Expand Down Expand Up @@ -225,4 +227,28 @@ describe("bar chart", () => {
runtime = model.getters.getChartRuntime("chartId") as BarChartRuntime;
expect(runtime.chartJsConfig.data.datasets[0].borderColor).toBe("#f00");
});

test("Bar chart trend line legend", () => {
const model = createModelFromGrid({ A1: "1", A2: "2" });
createChart(
model,
{
dataSets: [
{
dataRange: "Sheet1!A1:A2",
backgroundColor: "#f00",
label: "serie_1",
trend: { type: "polynomial", order: 1, color: "#f0f", display: true },
},
],
dataSetsHaveTitle: false,
type: "bar",
},
"1"
);
expect(getChartLegendLabels(model, "1")).toMatchObject([
{ text: "serie_1", fillStyle: "#f00", pointStyle: "rect" },
{ text: "Trend line for serie_1", strokeStyle: "#f0f", pointStyle: "line" },
]);
});
});
2 changes: 2 additions & 0 deletions tests/figures/chart/combo_chart_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ describe("combo chart", () => {
pointStyle: "rect",
strokeStyle: "#f00",
fillStyle: "#f00",
datasetIndex: 0,
},
{
fontColor: "#000000",
Expand All @@ -143,6 +144,7 @@ describe("combo chart", () => {
pointStyle: "line",
strokeStyle: "#00f",
fillStyle: "#00f",
datasetIndex: 1,
},
]);
});
Expand Down
2 changes: 2 additions & 0 deletions tests/figures/chart/line_chart_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ describe("line chart", () => {
lineWidth: 3,
pointStyle: "line",
strokeStyle: "#f00",
datasetIndex: 0,
},
{
fontColor: "#000000",
Expand All @@ -206,6 +207,7 @@ describe("line chart", () => {
lineWidth: 3,
pointStyle: "line",
strokeStyle: "#00f",
datasetIndex: 1,
},
]);
});
Expand Down
2 changes: 2 additions & 0 deletions tests/figures/chart/radar_chart_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe("radar chart", () => {
lineWidth: 3,
pointStyle: "line",
strokeStyle: "#f00",
datasetIndex: 0,
},
{
fontColor: "#000000",
Expand All @@ -97,6 +98,7 @@ describe("radar chart", () => {
lineWidth: 3,
pointStyle: "line",
strokeStyle: "#00f",
datasetIndex: 1,
},
]);
});
Expand Down

0 comments on commit 873498c

Please sign in to comment.