fix(chart) :: line series up on a category axis for every chart type

Author: 81reap

CHANGELOG.md

@@ -8,6 +8,7 @@
  - Form `options_source` URLs now preserve existing query parameters when adding the dynamic `search` parameter.
  - Map coordinates that are not a pair of numbers, like a latitude with no longitude, are now reported in the browser console and skipped, instead of breaking the whole map.
  - Stacked charts now stack their series by `x` value instead of by point order, which used to give wrong totals when a series was missing a point.
+ - `line`, `area`, `scatter`, `bubble` and `heatmap` charts with text labels on the x axis now line their series up by label, leaving a gap where a series skips one.
  - `column` charts now display vertical bars instead of nothing at all.
  - `stacked` is now ignored on chart types that cannot stack, instead of displaying an empty chart.
  - Screen readers now announce the title of the modal component instead of an unnamed dialog.

sqlpage/apexcharts.js

@@ -38,6 +38,14 @@ sqlpage_chart = (() => {
 
   const STACKABLE_CHART_TYPES = ["line", "area", "bar"];
   const APEXCHARTS_TYPE_ALIASES = { column: "bar" };
+  const Y_WHEN_A_SERIES_SKIPS_A_LABEL = {
+    bar: 0,
+    line: null,
+    area: null,
+    scatter: null,
+    bubble: null,
+    heatmap: null,
+  };
 
   /** @typedef {number|string|Date} XValue */
   /** @typedef { {name:string, data:{x:XValue,y:number|null,z?:number}[]} } ChartSeries */
@@ -46,6 +54,9 @@ sqlpage_chart = (() => {
   /** @param {XValue} x @returns {number|string} equal x values share a key */
   const x_key = (x) => (x instanceof Date ? x.getTime() : x);
 
+  /** @param {ChartSeries[]} series */
+  const x_is_text = (series) => typeof series[0]?.data[0]?.x === "string";
+
   /**
    * @param {ChartSeries[]} series
    * @returns {XValue[]} every x the series hold, in their own order where they
@@ -66,29 +77,44 @@ sqlpage_chart = (() => {
 
   /**
    * ApexCharts pairs points across series by index rather than by x, so a
-   * series that skips an x stacks onto the wrong one. Give every series the
-   * same x values, counting an x it never measured as zero.
+   * series that skips an x lands on the wrong one. Give every series the same
+   * amount of x values.
    *
    * @param {ChartSeries[]} series
+   * @param {number|null} y_when_missing what a series with no value at an x is
+   *   worth there: zero to add nothing to a stack, null to leave a gap.
    * @returns {ChartSeries[]}
    */
-  function align_series(series) {
+  function align_series(series, y_when_missing) {
     const all_x = merged_x_values(series);
     return series.map(({ name, data }) => {
       const by_x = new Map(data.map((point) => [x_key(point.x), point]));
       return {
         name,
         data: all_x.map((x) => {
           const point = by_x.get(x_key(x));
-          return { ...point, x, y: point?.y || 0 };
+          return { ...point, x, y: point?.y ?? y_when_missing };
         }),
       };
     });
   }
 
+  /**
+   * @param {ChartSeries[]} series
+   * @param {string} chart_type
+   * @param {boolean} is_stacked
+   * @returns {ChartSeries[]}
+   */
+  function align_series_for(series, chart_type, is_stacked) {
+    if (is_stacked) return align_series(series, 0);
+    if (x_is_text(series) && chart_type in Y_WHEN_A_SERIES_SKIPS_A_LABEL)
+      return align_series(series, Y_WHEN_A_SERIES_SKIPS_A_LABEL[chart_type]);
+    return series;
+  }
+
   // The unit tests load this file as a CommonJS module; browsers have no `module`.
   if (typeof module !== "undefined")
-    module.exports = { align_series, merged_x_values };
+    module.exports = { align_series, align_series_for, merged_x_values };
 
   /** @param {HTMLElement} c */
   function build_sqlpage_chart(c) {
@@ -129,16 +155,12 @@ sqlpage_chart = (() => {
     let series = Object.values(series_map);
 
     let labels;
-    const categories =
-      series.length > 0 && typeof series[0].data[0].x === "string";
+    const categories = x_is_text(series);
     if (chart_type === "pie") {
       labels = data.points.map(([name, x, _y]) => x || name);
       series = data.points.map(([_name, _x, y]) => Number.parseFloat(y));
-    } else if (
-      series.length > 1 &&
-      (is_stacked || (categories && chart_type === "bar"))
-    )
-      series = align_series(series);
+    } else if (series.length > 1)
+      series = align_series_for(series, chart_type, is_stacked);
 
     const options = {
       chart: {

tests/end-to-end/chart-component.spec.ts

@@ -106,17 +106,21 @@ async function renderChart(
           p.y,
         ]),
       }));
-      const drawnPerSeries = series.map(({ name }) => ({
-        name,
-        heights: [
+      const drawnPerSeries = series.map(({ name }) => {
+        const markers = [
           ...container.querySelectorAll<SVGGraphicsElement>(
-            `.apexcharts-series[seriesName='${name}'] .apexcharts-marker`,
+            `.apexcharts-series[seriesName='${name}'] .apexcharts-series-markers > .apexcharts-marker`,
           ),
-        ].map((m) => Math.round(m.getBBox().y)),
-      }));
+        ].map((m) => m.getBBox());
+        return {
+          name,
+          lefts: markers.map((b) => Math.round(b.x)),
+          heights: markers.map((b) => Math.round(b.y)),
+        };
+      });
       const shapes = [
         ...container.querySelectorAll<SVGGraphicsElement>(
-          ".apexcharts-bar-area, .apexcharts-rangebar-area",
+          ".apexcharts-bar-area, .apexcharts-rangebar-area, .apexcharts-treemap-rect",
         ),
       ].map((shape) => {
         const { x, y, width, height } = shape.getBBox();
@@ -252,6 +256,102 @@ test("stacks a bar series on the categories it skipped", async ({ page }) => {
   ]);
 });
 
+test("lines an unstacked series up with the categories it skipped", async ({
+  page,
+}) => {
+  const chart = await renderChart(page, { type: "line" }, [
+    ...A_IN_EVERY_QUARTER,
+    ...B_MISSING_THE_FIRST_QUARTER,
+  ]);
+
+  expect(chart.failures).toEqual([]);
+  expect(chart.series[1].points).toEqual([
+    ["Q1", null],
+    ["Q2", 20],
+    ["Q3", 30],
+  ]);
+});
+
+test("draws nothing where an unstacked series has no value", async ({
+  page,
+}) => {
+  const chart = await renderChart(page, { type: "line" }, [
+    ...A_IN_EVERY_QUARTER,
+    ...B_MISSING_THE_FIRST_QUARTER,
+  ]);
+  const [a, b] = chart.drawnPerSeries;
+
+  expect(a.lefts).toHaveLength(3);
+  expect(b.lefts).toEqual(a.lefts.slice(1));
+});
+
+test("keeps a measured zero apart from a missing value", async ({ page }) => {
+  const chart = await renderChart(page, { type: "line" }, [
+    ...A_IN_EVERY_QUARTER,
+    ["B", "Q2", 0],
+    ["B", "Q3", 30],
+  ]);
+  const [a, b] = chart.drawnPerSeries;
+
+  expect(chart.series[1].points).toEqual([
+    ["Q1", null],
+    ["Q2", 0],
+    ["Q3", 30],
+  ]);
+  expect(b.lefts).toEqual(a.lefts.slice(1));
+});
+
+for (const type of ["area", "scatter", "heatmap"]) {
+  test(`lines up the series of a ${type} chart on a category axis`, async ({
+    page,
+  }) => {
+    const chart = await renderChart(page, { type }, [
+      ...A_IN_EVERY_QUARTER,
+      ...B_MISSING_THE_FIRST_QUARTER,
+    ]);
+
+    expect(chart.failures).toEqual([]);
+    expect(chart.series[1].points.map((p) => p[0])).toEqual(["Q1", "Q2", "Q3"]);
+  });
+}
+
+test("keeps the bubble size of the points it lined up", async ({ page }) => {
+  const chart = await renderChart(page, { type: "bubble" }, [
+    ["A", "Q1", 1, 30],
+    ["A", "Q2", 2, 30],
+    ["B", "Q2", 5, 70],
+  ]);
+
+  expect(chart.failures).toEqual([]);
+  expect(chart.series[1].points).toEqual([
+    ["Q1", null],
+    ["Q2", 5],
+  ]);
+});
+
+test("leaves a rangeBar chart on a category axis alone", async ({ page }) => {
+  const chart = await renderChart(
+    page,
+    { type: "rangeBar", time: true },
+    TASKS_OVER_TIME,
+  );
+
+  expect(chart.failures).toEqual([]);
+  expect(chart.shapes).toHaveLength(2);
+});
+
+test("leaves a treemap chart alone", async ({ page }) => {
+  const chart = await renderChart(page, { type: "treemap" }, [
+    ["North America", "United States", 35],
+    ["North America", "Canada", 15],
+    ["Europe", "France", 30],
+    ["Europe", "Germany", 55],
+  ]);
+
+  expect(chart.failures).toEqual([]);
+  expect(chart.shapes).toHaveLength(4);
+});
+
 test("draws a rangeBar chart that asks to be stacked", async ({ page }) => {
   const chart = await renderChart(
     page,