Skip to content
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
36 changes: 27 additions & 9 deletions sqlpage/apexcharts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions tests/end-to-end/fixtures/chart/numeric-axis-xticks.sql
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 2 additions & 1 deletion tests/end-to-end/fixtures/chart/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
39 changes: 34 additions & 5 deletions tests/js/chart_series.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
});
Expand All @@ -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", () => {
Expand Down