diff --git a/CHANGELOG.md b/CHANGELOG.md index eb4c7032..2f3ef908 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## unreleased - Numeric x values on Cartesian charts now explicitly use a continuous numeric axis, preventing fractional tick positions from being displayed as misleading rounded integers. +- `xticks` now represents the requested number of tick positions on numeric x-axes, rather than the number of intervals between them. ## v0.46.1 diff --git a/examples/official-site/sqlpage/migrations/01_documentation.sql b/examples/official-site/sqlpage/migrations/01_documentation.sql index ce61b247..52c60704 100644 --- a/examples/official-site/sqlpage/migrations/01_documentation.sql +++ b/examples/official-site/sqlpage/migrations/01_documentation.sql @@ -669,7 +669,7 @@ INSERT INTO parameter(component, name, description, type, top_level, optional) S ('xtitle', 'Title of the x axis, displayed below it.', 'TEXT', TRUE, TRUE), ('ytitle', 'Title of the y axis, displayed to its left.', 'TEXT', TRUE, TRUE), ('ztitle', 'Title of the z axis, displayed in tooltips.', 'TEXT', TRUE, TRUE), - ('xticks', 'Number of intervals used to generate a numeric x-axis, normally producing one more tick position. On category and time axes, this is a target for label density, so the visible label count may differ.', 'INTEGER', TRUE, TRUE), + ('xticks', 'Number of tick positions on a numeric x-axis. For example, 5 requests five evenly spaced positions, including the two ends. On category and time axes, it asks ApexCharts to show up to that many labels; it may show fewer to keep labels from overlapping.', 'INTEGER', TRUE, TRUE), ('yticks', 'Number of ticks on the y axis.', 'INTEGER', TRUE, TRUE), ('ystep', 'Step between ticks on the y axis.', 'REAL', TRUE, TRUE), ('marker', 'Marker size', 'REAL', TRUE, TRUE), diff --git a/sqlpage/apexcharts.js b/sqlpage/apexcharts.js index b822f9e1..3627e570 100644 --- a/sqlpage/apexcharts.js +++ b/sqlpage/apexcharts.js @@ -61,8 +61,17 @@ sqlpage_chart = (() => { /** @param {ChartSeries[]} series */ const x_is_text = (series) => typeof series[0]?.data?.[0]?.x === "string"; - /** @param {ChartSeries[]} series @param {string} chart_type */ - function xaxis_type_for(series, chart_type, is_timeseries, is_horizontal) { + /** + * Numeric x values need an explicit axis type to retain their proportional + * spacing; otherwise ApexCharts treats them as evenly spaced categories. + * + * @param {ChartSeries[]} series + * @param {{chart_type:string, is_timeseries:boolean, is_horizontal:boolean}} options + */ + function xaxis_type_for( + series, + { chart_type, is_timeseries, is_horizontal }, + ) { if (is_timeseries) return "datetime"; if (x_is_text(series)) return "category"; if ( @@ -73,6 +82,18 @@ sqlpage_chart = (() => { return "numeric"; } + /** + * ApexCharts expects intervals for numeric axes, while SQLPage exposes the + * more intuitive number of tick positions to users. + * + * @param {number|undefined} xticks + * @param {string|undefined} xaxis_type + */ + function xaxis_tick_amount(xticks, xaxis_type) { + if (!xticks) return; + return xaxis_type === "numeric" ? Math.max(1, xticks - 1) : xticks; + } + /** * @param {ChartSeries[]} series * @returns {XValue[]} every x the series hold, in their own order where they @@ -225,12 +246,11 @@ sqlpage_chart = (() => { let colors = palette; let series = Object.values(series_map); - const xaxis_type = xaxis_type_for( - series, + const xaxis_type = xaxis_type_for(series, { chart_type, is_timeseries, - !!data.horizontal, - ); + is_horizontal: !!data.horizontal, + }); let labels; if (chart_type === "pie") { @@ -386,9 +406,7 @@ sqlpage_chart = (() => { series, }; if (labels) options.labels = labels; - // Numeric axes count intervals; category and time axes use tickAmount as a - // target for label density. - if (data.xticks) options.xaxis.tickAmount = data.xticks; + options.xaxis.tickAmount = xaxis_tick_amount(data.xticks, xaxis_type); const chart = new ApexCharts(chartContainer, options); chart.render(); if (window.charts) window.charts.push(chart); diff --git a/tests/end-to-end/fixtures/chart/numeric-axis-xticks.sql b/tests/end-to-end/fixtures/chart/numeric-axis-xticks.sql index 9848a9a1..667fa36f 100644 --- a/tests/end-to-end/fixtures/chart/numeric-axis-xticks.sql +++ b/tests/end-to-end/fixtures/chart/numeric-axis-xticks.sql @@ -1,9 +1,9 @@ SELECT 'chart' AS component, 'test-chart' AS id, - 'Explicit numeric x intervals' AS title, + 'Explicit numeric x tick count' AS title, 'bar' AS type, - 2 AS xticks; + 3 AS xticks; SELECT 'A' AS series, 1 AS x, 1 AS y UNION ALL SELECT 'A', 4, 4 diff --git a/tests/end-to-end/fixtures/chart/test.ts b/tests/end-to-end/fixtures/chart/test.ts index d20553a9..1d796095 100644 --- a/tests/end-to-end/fixtures/chart/test.ts +++ b/tests/end-to-end/fixtures/chart/test.ts @@ -175,11 +175,12 @@ test("keeps irregular numeric x values proportionately spaced", async ({ ); }); -test("keeps an explicit x interval count", async ({ page }) => { +test("renders the requested number of numeric x ticks", async ({ page }) => { const chart = await renderChart(page, "numeric-axis-xticks"); expect(chart.failures).toEqual([]); expect(chart.xaxis).toEqual({ type: "numeric", tickAmount: 2 }); + expect(chart.axisLabels).toHaveLength(3); }); test("keeps text x values as categories", async ({ page }) => { diff --git a/tests/js/chart_series.spec.ts b/tests/js/chart_series.spec.ts index dc981d77..4bc7cc90 100644 --- a/tests/js/chart_series.spec.ts +++ b/tests/js/chart_series.spec.ts @@ -37,16 +37,31 @@ test("uses a continuous axis for numeric Cartesian x values", () => { const numeric = [series("a", { x: 1, y: 1 }, { x: 12, y: 12 })]; for (const type of ["line", "area", "bar", "scatter", "bubble"]) - assert.equal(xaxis_type_for(numeric, type, false, false), "numeric"); + assert.equal( + xaxis_type_for(numeric, { + chart_type: type, + is_timeseries: false, + is_horizontal: false, + }), + "numeric", + ); }); test("keeps text and time x values on their respective axes", () => { assert.equal( - xaxis_type_for([series("a", { x: "Q1", y: 1 })], "bar", false, false), + xaxis_type_for([series("a", { x: "Q1", y: 1 })], { + chart_type: "bar", + is_timeseries: false, + is_horizontal: false, + }), "category", ); assert.equal( - xaxis_type_for([series("a", { x: 1, y: 1 })], "bar", true, false), + xaxis_type_for([series("a", { x: 1, y: 1 })], { + chart_type: "bar", + is_timeseries: true, + is_horizontal: false, + }), "datetime", ); }); @@ -55,8 +70,22 @@ test("does not turn category-oriented charts into numeric axes", () => { const numeric = [series("a", { x: 1, y: 1 })]; for (const type of ["heatmap", "rangeBar", "pie", "treemap"]) - assert.equal(xaxis_type_for(numeric, type, false, false), undefined); - assert.equal(xaxis_type_for(numeric, "bar", false, true), undefined); + assert.equal( + xaxis_type_for(numeric, { + chart_type: type, + is_timeseries: false, + is_horizontal: false, + }), + undefined, + ); + assert.equal( + xaxis_type_for(numeric, { + chart_type: "bar", + is_timeseries: false, + is_horizontal: true, + }), + undefined, + ); }); test("merged_x_values keeps the order the series agree on", () => {