FIX: Rerender Chart component if config changes (#29955)

The chart component was not rerendering if the chart
config passed to it was changed, this commit fixes the issue
by getting the config from `this.args` before trying to
access it inside an async call, so if the args change Ember
correctly rerenders. Also adds tests for this and general
chart rendering.

---------

Co-authored-by: Martin Brennan <martin@discourse.org>
This commit is contained in:
Sam 2024-11-28 14:37:59 +11:00 committed by GitHub
parent 07813ba83c
commit cb819ab49a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 109 additions and 4 deletions

View File

@ -6,11 +6,10 @@ import loadScript from "discourse/lib/load-script";
// chartConfig - object
export default class Chart extends Component {
renderChart = modifier((element) => {
const { chartConfig } = this.args;
loadScript("/javascripts/Chart.min.js").then(() => {
this.chart = new window.Chart(
element.getContext("2d"),
this.args.chartConfig
);
this.chart = new window.Chart(element.getContext("2d"), chartConfig);
});
return () => this.chart?.destroy();

View File

@ -0,0 +1,106 @@
import { tracked } from "@glimmer/tracking";
import { render, settled } from "@ember/test-helpers";
import { module, test } from "qunit";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import Chart from "admin/components/chart";
module("Integration | Component | Chart", function (hooks) {
setupRenderingTest(hooks);
function generateData() {
let data = [];
for (let i = 0; i < 10; i++) {
data.push({ x: i, y: Math.random() * 10 });
}
return data;
}
// We do this because comparing hashes at a pixel level is not reliable,
// even when calling a hash on a canvas that was not changing at all
// I was getting different results. PNG is more solid ground and deterministic.
// No need to hash anything, the data URLs are unique, and we are rendering
// quite small canvases so no need to worry about perf as much.
function renderAsPNGURL(canvas) {
return canvas.toDataURL("image/png");
}
function generateChartConfig(model, options = {}) {
return {
type: "bar",
data: {
labels: model.data.map((r) => r.x),
datasets: [
{
data: model.data.map((r) => r.y),
label: model.title,
backgroundColor: options.backgroundColor || "rgba(200,220,240,1)",
},
],
},
// We don't want anything messing with the canvas rendering by
// moving things around as we are trying to capture it.
options: {
animation: false,
responsive: false,
maintainAspectRatio: false,
},
};
}
test("renders a chart", async function (assert) {
this.model = {
title: "Test Chart",
data: generateData(),
};
const chartConfig = generateChartConfig(this.model);
await render(<template><canvas class="empty-canvas"></canvas></template>);
const emptyCanvasDataURL = await renderAsPNGURL(
document.querySelector("canvas.empty-canvas")
);
await render(<template><Chart @chartConfig={{chartConfig}} /></template>);
const dataURL = await renderAsPNGURL(
document.querySelector("canvas.chart-canvas")
);
assert.notStrictEqual(
emptyCanvasDataURL,
dataURL,
"The canvas was rendered successfully"
);
});
test("rerenders the chart if the config changes", async function (assert) {
this.model = {
title: "Test Chart",
data: generateData(),
};
const testState = new (class {
@tracked chartConfig;
})();
testState.chartConfig = generateChartConfig(this.model);
await render(<template>
<Chart @chartConfig={{testState.chartConfig}} />
</template>);
const firstCanvasDataURL = await renderAsPNGURL(
document.querySelector("canvas.chart-canvas")
);
testState.chartConfig = generateChartConfig(this.model, {
backgroundColor: "red",
});
await settled();
const secondCanvasDataURL = await renderAsPNGURL(
document.querySelector("canvas.chart-canvas")
);
assert.notStrictEqual(
firstCanvasDataURL,
secondCanvasDataURL,
"The canvases are not identical, so the chart rerendered successfully"
);
});
});